Skip to content

Commit

Permalink
Wrap XML library exceptions with glifLib types when parsing glifs
Browse files Browse the repository at this point in the history
This allows dependent projects to catch errors parsing glifs without
requiring logic to account for which XML library fonttools is using
internally (e.g. for implementing fonttools/ufoLib2#264).

This commit also adds tests to ensure that the exception we expose when
glifs have invalid syntax remains stable across future releases.
  • Loading branch information
Hoolean committed Mar 9, 2023
1 parent 27758ee commit 1f33803
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 4 deletions.
12 changes: 8 additions & 4 deletions Lib/fontTools/ufoLib/glifLib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1082,10 +1082,14 @@ def _glifTreeFromFile(aFile):

def _glifTreeFromString(aString):
data = tobytes(aString, encoding="utf-8")
if etree._have_lxml:
root = etree.fromstring(data, parser=etree.XMLParser(remove_comments=True))
else:
root = etree.fromstring(data)
try:
if etree._have_lxml:
root = etree.fromstring(data, parser=etree.XMLParser(remove_comments=True))
else:
root = etree.fromstring(data)
except Exception as etree_exception:
raise GlifLibError("GLIF contains invalid XML.") from etree_exception

if root.tag != "glyph":
raise GlifLibError("The GLIF is not properly formatted.")
if root.text and root.text.strip() != "":
Expand Down
33 changes: 33 additions & 0 deletions Tests/ufoLib/glifLib_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import tempfile
import shutil
import unittest
from pathlib import Path
from io import open
from .testSupport import getDemoFontGlyphSetPath
from fontTools.ufoLib.glifLib import (
Expand Down Expand Up @@ -134,6 +135,27 @@ def testGetUnicodes(self):
else:
self.assertEqual(g.unicodes, unicodes[glyphName])

def testReadGlyphInvalidXml(self):
"""Test that calling readGlyph() to read a .glif with invalid XML raises
a library error, instead of an exception from the XML dependency that is
used internally."""

# Create a glyph set with three empty glyphs.
glyph_set = GlyphSet(self.dstDir)
glyph_set.writeGlyph("a", _Glyph())
glyph_set.writeGlyph("b", _Glyph())
glyph_set.writeGlyph("c", _Glyph())

# Corrupt the XML of /c.
invalid_xml = b"<abc></def>"
Path(self.dstDir, glyph_set.contents["c"]).write_bytes(invalid_xml)

# Confirm that reading /c raises an appropriate error.
glyph_set.readGlyph("a", _Glyph())
glyph_set.readGlyph("b", _Glyph())
with pytest.raises(GlifLibError, match="GLIF contains invalid XML"):
glyph_set.readGlyph("c", _Glyph())


class FileNameTest:
def test_default_file_name_scheme(self):
Expand Down Expand Up @@ -228,6 +250,17 @@ def test_parse_xml_remove_comments(self):
assert g.width == 1290
assert g.unicodes == [0x0041]

def test_read_invalid_xml(self):
"""Test that calling readGlyphFromString() with invalid XML raises a
library error, instead of an exception from the XML dependency that is
used internally."""

invalid_xml = b"<abc></def>"
empty_glyph = _Glyph()

with pytest.raises(GlifLibError, match="GLIF contains invalid XML"):
readGlyphFromString(invalid_xml, empty_glyph)

def test_read_unsupported_format_version(self, caplog):
s = """<?xml version='1.0' encoding='utf-8'?>
<glyph name="A" format="0" formatMinor="0">
Expand Down

0 comments on commit 1f33803

Please sign in to comment.