Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing a null-string and re-building that file results in incorrect structure #79

Closed
taknuti opened this issue Sep 14, 2016 · 8 comments

Comments

@taknuti
Copy link

taknuti commented Sep 14, 2016

When parsing a file containing an empty string and re-building that JSON to a plist, the empty string will not be added, resulting in a key without a value (see example below). The fix might be simple on the 'parse' side, as there is an explicit null-check on parse.js@131: if(isEmptyNode(node)) return null;. This can be changed to if(isEmptyNode(node)) return ''; as empty strings behave correctly.
However, this probably means null-values in JSON generated from other sources are affected as well, so perhaps this needs a fix on the build-side as well.

Example code:

var plist = require('plist');
var xml = 
  '<?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">' +
    '<key>metadata</key>' +
    '<dict>' +
      '<key>ExampleKey</key>' +
      '<string></string>' +
      '<key>AnotherKey</key>' +
      '<string>Avalue</string>' +
    '</dict>' +
  '</plist>';

console.log(plist.build(plist.parse(xml)));

Output:

<?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">
  <array>
    <string>metadata</string>
    <dict>
      <key>ExampleKey</key>
      <key>AnotherKey</key>
      <string>Avalue</string>
    </dict>
  </array>
</plist>
@rnarian
Copy link

rnarian commented Sep 16, 2016

👍

Just ran into the same problem:

<key>First Key</key>
</string>
<key>Second Key</key>
<string>Value</string>

... results in:

<key>First Key</key>
<key>Second Key</key>
<string>Value</string>

pcantrell added a commit to pcantrell/plist.js that referenced this issue Sep 20, 2016
akofman added a commit to akofman/plist.js that referenced this issue Sep 21, 2016
akofman added a commit to akofman/plist.js that referenced this issue Sep 21, 2016
@kim3er
Copy link

kim3er commented Sep 21, 2016

@akofman's changes sort the problem for me.

@otmezger
Copy link

otmezger commented Oct 9, 2016

Can confirm, @akofman's fix worked for me on OS Sierra with Xcode 8.0

@njleonzhang
Copy link

Issue is still there in my environment:

    <key>NSMainNibFile</key>
    <string/>
    <key>NSMainNibFile~ipad</key>
    <string/>

to

    <key>NSMainNibFile</key>
    <key>NSMainNibFile~ipad</key>

xcode 8.1
plist 2.0.1

luissilvaos pushed a commit to OutSystems/plist.js that referenced this issue Jan 12, 2017
luissilvaos pushed a commit to OutSystems/plist.js that referenced this issue Jan 12, 2017
luissilvaos pushed a commit to OutSystems/plist.js that referenced this issue Jan 12, 2017
@cmaart
Copy link

cmaart commented Mar 29, 2017

Still a problem

dpa99c referenced this issue in dpa99c/cordova-custom-config Dec 20, 2017
Set plist dependency to use fork which fixes bug causing #136 and #90.
Set xcode to @1.0.0 which is published from the new apache/cordova repo.
dpa99c added a commit to dpa99c/cordova-custom-config that referenced this issue Jan 16, 2019
@mreinstein
Copy link
Collaborator

Hey sorry for the super long reply delay. Can we get a PR for this? I'm happy to review/merge. Would be nice to get this in for the upcoming v4 release.

@taknuti
Copy link
Author

taknuti commented Jan 16, 2023

Hey sorry for the super long reply delay. Can we get a PR for this? I'm happy to review/merge. Would be nice to get this in for the upcoming v4 release.

It has indeed been a while ;) I just reread the comments and unless I'm mistaken this was fixed in #84. We are not using this package anymore and I'm not able to verify just now, but I think this issue van be closed.

@taknuti taknuti closed this as completed Jan 16, 2023
@julien-c
Copy link

i have changed jobs 3 times since i subscribed to this issue so I can't verify either

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants