Be more strict when parsing plists

[trufae]

No description provided.

[trufae]

related to #87

[trufae]

test case:

var p = require('./');

var o = p.parse(`
<plist>
<dict>
  <key>test</key>
  <string>Testing</string>
  <key>bar</key>
  <string></string>
</dict>
</plist>
`);

console.log(p.build(o));

[trufae]

Can i have a review on this at least? i would love to use the official module but it's broken for my usecases so i need to keep maintaining my fork

[mreinstein]

technically this is a breaking change, right? meaning this would have to be included with a major semver bump to v4, unless I misunderstand

[mreinstein]

I appreciate the PRs. Please add at least 1 test to validate this behavior

[trufae]

Should be good to review now

[mreinstein]

Isn't an empty string still considered valid?

[trufae]

added a test for the empty string case

[mreinstein]

maybe I misunderstood the test case. Which part of the plist is considered the invalid part? the <strong>... section?

[trufae]

yes. <strong> is not a valid token for a plist. so it shouldnt permit deserializing it because it results on an invalid xml that fails later on when parsing it.

i'm using this library on millions of iOS apps and i think it's fine to consider invalid what's not correct to ease catching bugs in early stages of the problem.

the output of this test without the patch results on this:

<plist>
<dict>
  <key>test</key>
  <key>bar</key>
  <string></string>
</dict>
</plist>

[mreinstein]

so basically this library switches it's philosophy from permissive to a more strict interpretation in v4, right?

I'm not opposed to this, I think it's probably better to be strict, but I want to ensure this is communicated when the v4 migration doc is written.

[trufae]

That's correct. maybe we can have an option disabled by default for this behaviour, so it doesnt breaks. will this way be merged?

also i added the missing line to run the CI on pullreqsts in here

[trufae]

good! :D thanks