Skip to content

Commit 60a6209

Browse files
committed
Drop non-plist glyph libs instead of failing
These exist in a number of old real-world UFO files, and it doesn't seem particularly useful to just fail to read those files at all. This also brings in the log crate, for logging these dropped errors.
1 parent 01d4c2e commit 60a6209

3 files changed

Lines changed: 19 additions & 8 deletions

File tree

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ thiserror = "2.0"
4141
indexmap = { version = "2.0.0", features = ["serde"] }
4242
base64 = "0.22"
4343
close_already = "0.3"
44+
log = "0.4"
4445

4546
[dev-dependencies]
4647
failure = "0.1.6"

src/glyph/parse.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -308,12 +308,20 @@ impl<'names> GlifParser<'names> {
308308
}
309309

310310
let plist_slice = &raw_xml[start..end];
311-
let dict = plist::Value::from_reader_xml(plist_slice)
312-
.map_err(|_| GlifLoadError::Parse(ErrorKind::BadLib))?
313-
.into_dictionary()
314-
.ok_or(GlifLoadError::Parse(ErrorKind::LibMustBeDictionary))?;
311+
match plist::Value::from_reader_xml(plist_slice)
312+
.map_err(|_| GlifLoadError::Parse(ErrorKind::BadLib))
313+
.and_then(|x| {
314+
x.into_dictionary().ok_or(GlifLoadError::Parse(ErrorKind::LibMustBeDictionary))
315+
}) {
316+
Ok(dict) => {
317+
// we used to error if this was malformed but there are a number of early UFO files
318+
// in the wild that store arbitrary xml in the lib, which doesn't parse as a plist.
319+
// Instead of failing to parse these files, we prefer to just skip the dicts.
320+
self.glyph.lib = dict;
321+
}
322+
Err(e) => log::info!("glyph {} contains invalid lib: '{e}'", self.glyph.name),
323+
}
315324

316-
self.glyph.lib = dict;
317325
Ok(())
318326
}
319327

src/glyph/tests.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,8 +318,9 @@ fn bad_angle() {
318318
}
319319

320320
#[test]
321-
#[should_panic(expected = "LibMustBeDictionary")]
322-
fn lib_must_be_dict() {
321+
// in a number of older UFO files in the wild glyphs can contain dictionaries
322+
// with arbitrary non-plsit XML:
323+
fn skip_non_lib_dictionary() {
323324
let data = r#"
324325
<?xml version="1.0" encoding="UTF-8"?>
325326
<glyph name="period" format="2">
@@ -328,7 +329,8 @@ fn lib_must_be_dict() {
328329
</lib>
329330
</glyph>
330331
"#;
331-
let _ = parse_glyph(data.as_bytes()).unwrap();
332+
let glyph = parse_glyph(data.as_bytes()).unwrap();
333+
assert!(glyph.lib.is_empty());
332334
}
333335

334336
#[test]

0 commit comments

Comments
 (0)