ebarnard / rust-plist

A rusty plist parser.
MIT License
71 stars 42 forks source link

Should EmitterConfig::pad_self_closing be set false for XmlWriter? #65

Closed MLKrisJohnson closed 3 years ago

MLKrisJohnson commented 3 years ago

I've been using the plist crate to generate files used for code-signing iOS applications. One of the necessary files is an "entitlements" plist. When I use plist::to_writer_xml() to serialize one of these structures to a file, the result is this:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>application-identifier</key>
    <string>XXXYYYZZZZ.com.mycompany.MyApp</string>
    <key>get-task-allow</key>
    <true />
    <key>com.apple.developer.team-identifier</key>
    <string>XXXYYYZZZZ</string>
    <key>keychain-access-groups</key>
    <array>
        <string>XXXYYYZZZZ.com.mycompany.MyApp</string>
    </array>
</dict>
</plist>

This is valid XML. However, the XML parser used in Apple's codesign utility does not accept the space in <true /> as valid, so attempts to use this as-is fail.

I can workaround this in my code by writing the serialized XML to a buffer and then substituting a space-less <true/> before writing to disk. However, I think this problem could be fixed by adding .pad_self_closing(false) to the EmitterConfig that is used in stream::XmlWriter::new(), or by adding a variant of to_writer_xml() that accepts a custom EmitterConfig, or by adding another option to XmlWriteOptions.

I'm willing to submit a PR with the necessary changes. But I'd like to know which fix is preferred: change the default EmitterConfig, or add an alternate method that allows configuration. The "one-line fix" of adding .pad_self_closing(false) requires a corresponding change to the stream::xml_writer::tests::streaming_parser unit test.

ebarnard commented 3 years ago

This a bug as this library aims to match the output of Apple's tools for XML plists.

Let's add .pad_self_closing(false), there are unreleased changes that require a minor version bump so now would be a good time to change this.

ebarnard commented 3 years ago

Fixed in 1a9bf1e9.