From f52b3da721b11d735d59566e22f22e504ee5faf7 Mon Sep 17 00:00:00 2001 From: Johannes Neumeier Date: Fri, 24 Feb 2023 15:58:53 +0200 Subject: [PATCH 1/2] Make NameRecord comparison not fail on encoding errors #3006 --- Lib/fontTools/ttLib/tables/_n_a_m_e.py | 28 ++++++++++++++++---------- Tests/ttLib/tables/_n_a_m_e_test.py | 25 ++++++++++++++++++++--- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/Lib/fontTools/ttLib/tables/_n_a_m_e.py b/Lib/fontTools/ttLib/tables/_n_a_m_e.py index 21b797f3b..6eb2a4fa2 100644 --- a/Lib/fontTools/ttLib/tables/_n_a_m_e.py +++ b/Lib/fontTools/ttLib/tables/_n_a_m_e.py @@ -607,34 +607,40 @@ class NameRecord(object): def __lt__(self, other): if type(self) != type(other): - return NotImplemented + return NotImplemented() try: - # implemented so that list.sort() sorts according to the spec. selfTuple = ( self.platformID, self.platEncID, self.langID, self.nameID, - self.toBytes(), ) otherTuple = ( other.platformID, other.platEncID, other.langID, other.nameID, - other.toBytes(), ) - return selfTuple < otherTuple - except (UnicodeEncodeError, AttributeError): + except (AttributeError): # This can only happen for # 1) an object that is not a NameRecord, or # 2) an unlikely incomplete NameRecord object which has not been - # fully populated, or - # 3) when all IDs are identical but the strings can't be encoded - # for their platform encoding. - # In all cases it is best to return NotImplemented. - return NotImplemented + # fully populated + return NotImplemented() + + try: + # Include the actual NameRecord string in the comparison tuples + selfTuple = selfTuple + (self.toBytes(),) + otherTuple = otherTuple + (other.toBytes(),) + except (UnicodeEncodeError) as e: + # toBytes caused an encoding error in either of the two, so content + # to sorting based on IDs only + log.warning("NameRecord sorting failed to decode: %s" % e) + + # Implemented so that list.sort() sorts according to the spec by using + # the order of the tuple items and their comparison + return selfTuple < otherTuple def __repr__(self): return "" % ( diff --git a/Tests/ttLib/tables/_n_a_m_e_test.py b/Tests/ttLib/tables/_n_a_m_e_test.py index 43001b2fc..d3e41022f 100644 --- a/Tests/ttLib/tables/_n_a_m_e_test.py +++ b/Tests/ttLib/tables/_n_a_m_e_test.py @@ -72,15 +72,34 @@ class NameTableTest(unittest.TestCase): ] table.compile(None) - def test_names_sort_bytes_str_encoding_error(self): + def test_names_sort_attributes(self): table = table__n_a_m_e() + # Create an actual invalid NameRecord object + broken = makeName("Test", 25, 3, 1, 0x409) + delattr(broken, "platformID") table.names = [ - makeName("Test寬", 25, 1, 0, 0), - makeName("Test鬆鬆", 25, 1, 0, 0), + makeName("Test", 25, 3, 1, 0x409), + broken, ] + # Sorting these two is impossible, expect an error to be raised with self.assertRaises(TypeError): table.names.sort() + def test_names_sort_encoding(self): + """ + Confirm that encoding errors in name table strings do not prevent at + least sorting by other IDs + """ + table = table__n_a_m_e() + table.names = [ + makeName("Mac Unicode 寬 encodes ok", 25, 3, 0, 0x409), + makeName("Win Latin 寬 fails to encode", 25, 1, 0, 0), + ] + table.names.sort() + # Encoding errors or not, sort based on other IDs nonetheless + self.assertEqual(table.names[0].platformID, 1) + self.assertEqual(table.names[1].platformID, 3) + def test_addName(self): table = table__n_a_m_e() nameIDs = [] From 29f980135aeb0451fcd07a2f3cdb399ceee858a1 Mon Sep 17 00:00:00 2001 From: Johannes Neumeier Date: Mon, 27 Feb 2023 09:31:13 +0200 Subject: [PATCH 2/2] Clean up and NotImplemented as proper return value --- Lib/fontTools/ttLib/tables/_n_a_m_e.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/fontTools/ttLib/tables/_n_a_m_e.py b/Lib/fontTools/ttLib/tables/_n_a_m_e.py index 6eb2a4fa2..320cfa820 100644 --- a/Lib/fontTools/ttLib/tables/_n_a_m_e.py +++ b/Lib/fontTools/ttLib/tables/_n_a_m_e.py @@ -607,7 +607,7 @@ class NameRecord(object): def __lt__(self, other): if type(self) != type(other): - return NotImplemented() + return NotImplemented try: selfTuple = ( @@ -622,21 +622,21 @@ class NameRecord(object): other.langID, other.nameID, ) - except (AttributeError): + except AttributeError: # This can only happen for # 1) an object that is not a NameRecord, or # 2) an unlikely incomplete NameRecord object which has not been # fully populated - return NotImplemented() + return NotImplemented try: # Include the actual NameRecord string in the comparison tuples selfTuple = selfTuple + (self.toBytes(),) otherTuple = otherTuple + (other.toBytes(),) - except (UnicodeEncodeError) as e: + except UnicodeEncodeError as e: # toBytes caused an encoding error in either of the two, so content # to sorting based on IDs only - log.warning("NameRecord sorting failed to decode: %s" % e) + log.error("NameRecord sorting failed to encode: %s" % e) # Implemented so that list.sort() sorts according to the spec by using # the order of the tuple items and their comparison