-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
plistlib unable to read json and binary plist files #58660
Comments
Hi, Plist files have actually three flavors : XML ones, binary ones, and now (starting from Mac OS X 10.7 Lion) json one. The plistlib.readPlist function can only read XML plist files and thus cannot read binary and json ones. The binary format is open and described by Apple (http://opensource.apple.com/source/CF/CF-550/CFBinaryPList.c). Here is the diff (from Python 2.7 implementation of plistlib) to transparently read both binary and json formats. API of plistlib remains unchanged, since format detection is done by plistlib.readPlist. 57,58c57
64d62
195,285d183 |
Thanks for the patch. Could you upload it as a context diff? |
Here is the new patch. I assumed that you meant to use diff -c instead of the raw diff command. |
Hmm. Apparently what I meant was -u instead of -c (unified diff). I just use the 'hg diff' command myself, which does the right thing :) Of course, to do that you need to have a checkout. (We can probably use the context diff.) |
This patch is for Python 2. New features are accepted only for Python 3.3+. I ported the patch, but since I have no Mac, I can't check. To date code was specified incorrectly. The length of integers was calculated incorrectly. To convert integers, you can use int.from_bytes. Objects identity was not preserved. I'm not sure that the recognition of XML done enough. Should consider UTF-16 and UTF-32 with the BOM and without. Need tests. Also I'm a bit cleaned up and modernizing the code. I believe that it should be rewritten in a more object-oriented style. It is also worth to implement writer. |
storchaka > I'm trying to take care of your remarks. |
Keep it simple: if a few functions work, there is no need at all to add classes. Before doing more work though I suggest you wait for the feedback of the Mac maintainers. |
I (as one of the Mac maintainers) like the new functionality, but would like to see some changes:
Don't worry about rearchitecturing plistlib, it might need work in that regard but that need not be part of this issue and makes it harder to review the changes. I'm also far from convinced that a redesign of the code is needed. |
I'm working on a class, BinaryPlistParser, which allow to both read and write binary files. I've also added a parameter fmt to writePlist and readPlist, to specify the format ('json', 'xml1' or 'binary1', using XML by default). These constants are used by Apple for its plutil program. I'm now working on integrating these three formats to the test_plistlib.py. However, the json is less expressive than the other two, since it cannot handle dates. |
Here is the new patch, allowing read and write binary, json and xml plist files. It includes both the plistlib.py and test/test_plistlib.py patches. Similarly, my binary writer does not write the same binary files as the Apple library: my library writes the content of compound objects (dicts, lists and sets) before the object itself, while Apple writes the object before its content. Copying the Apple behavior results in some additional weird lines of code, for little benefit. Thus, I also removed the test_appleformattingfromliteral test for binary files. Other tests are made for all the three formats. |
Hi, I noticed in the latest message that d9pounces posted that "JSON format does not allow dates and data, so XML is used by default to write files.". Rthe XML version of plists also do not really 'support' those types, and they are converted as follows: NSData -> Base64 encoded data (from http://en.wikipedia.org/wiki/Property_list#Mac_OS_X) So really it should be the same thing when converting to json no? |
The plutil (Apple's command-line tool to convert plist files from a format to another) returns an error if you try to convert a XML plist with dates to JSON. |
Where are you even seeing these json property lists? I just checked the most recent documentation for NSPropertyListSerialization, and they have not updated the enum for NSPropertyListFormat. It seems that if even Apple doesn't support writing json property lists with their own apis then we shouldn't worry about supporting it? enum { |
plutil(1) supports writing json format. That written, the opensource parts of CoreFoundation on opensource.apple.com don't support reading or writing json files. I'm therefore -1 w.r.t. adding support for json formatted plist files, support for json can be added when Apple actually supports that it the system libraries and hence the format is stable. |
are any more changes needed to the code that is already posted as a patch in this bug report? or are the changes you wanted to see happen in msg157669 not happen yet? |
d9pouces: are you willing to sign a contributor agreement? The agreement is needed before we can add these changes to the stdlib, and I'd like to that for the 3.4 release. More information on the contributor agreement: http://www.python.org/psf/contrib/contrib-form/ |
I just signed this agreement. Thanks for accepting this patch! |
I've started work on integrating the latest patch. Some notes (primarily for my own use):
|
I've attached bpo-14455-v2.txt with an updated patch. The patch is still very much a work in progress, I haven't had as much time to work on this as I'd like. This version:
|
v3 is still a work in progress, and still fails some tests
The data generated by the binary plist generator does't match the data generated by Cocoa in OSX 10.8 (and generated by the helper script), I haven't fully debugged that problem yet. The generated binary plist |
See also: bpo-18168: request for the sort_keys option |
The test failure I'm getting is caused by a difference in the order in which items are written to the archive. I'm working on a fix. |
v4 passes the included tests. The testsuite isn't finished yet. |
The fifth version of the patch should be much cleaner. Changes:
Open issues:
|
I intend to commit my latest version of the patch during the europython sprints, with a minor change: don't include dump(s) and load(s), that change (and the other items on "open issues" in my last post) can be addressed later. |
Let me review your patch. |
Updated patch after next round of reviews. |
New changeset 673ca119dbd0 by Ronald Oussoren in branch 'default': |
New changeset 602e0a0ec67e by Ned Deily in branch 'default': |
These changes are worth to mention in What's News. "versionchanged" below writePlistToBytes() is wrong. Perhaps below dump() too. "versionadded" is needed for new functions: dump(), dumps(), load(), loads(). |
Currently negative integers are not supported in binary format. Here is a patch which adds support for negative integers and large integers up to 128 bit. |
oops... thanks for the patch. I'll review later this week, in particular the 128 bit integer support because I don't know if Apple's libraries support those. |
According to [1] Apple's libraries write any signed 128-bit integers, but read only integers from -2**63 to 2**64-1 (e.g. signed and unsigned 64-bit integers). [1] http://opensource.apple.com/source/CF/CF-550/CFBinaryPList.c |
Yet one nitpick. Perhaps _write_object() should raise TypeError instead of InvalidFileException. |
I'm working on an update for your patch that addresses these comments:
BTW. What about out-of-range integer values? Those currently raise struct.error, I'd prefer to raise TypeError instead because the use of the And a final question: integers with '2 ** 63 <= value < 2 ** 64' (e.g. values that are in the range of uint64_t but not in the range of int64_t) can be written to a binary plist, but will be read back as a negative value (which is the same behavior as in Apple's code). Should we warn about this in the documentation? I'll post an updated patch later today. |
The attached patch should fix the open issues:
|
At least we should support integers from -2**63 to 2**64-1 (signed and
Indeed.
I have copied this from test_bytes(). I suppose that pl2 can be int subclass.
http://hg.python.org/cpython/file/673ca119dbd0/Doc/library/plistlib.rst#l165
Agree. Especially if OSX SDK doesn't support deserialization of integers
These values should be written as 128-bit integers (token b'\x14'). |
Attached a script (using PyObjC) that demonstrates the behavior of Apple's Foundation framework with large integers. The same behavior should occur when the script is rewritten in Objective-C. |
Updated patch. |
I can't test on OSX, but I see that Apple's code can write any 128-bit integers and read signed and unsigned 64-bit integers. Can Apple's utilities read this file? What is a result? |
Conversion to XML results in: $ plutil -convert xml1 -o - 18446744073709551615.plist
<?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>a</key>
<integer>18446744073709551615</integer>
</dict>
</plist> This is the same as what I get with my latest patch: >>> import plistlib
>>> plistlib.load(open('18446744073709551615.plist', 'rb'))
__main__:1: ResourceWarning: unclosed file <_io.BufferedReader name='18446744073709551615.plist'>
{'a': 18446744073709551615} (and I have check that I can create a binary plist with a negative integer in this shell session) |
New changeset 1a8149ba3000 by Ronald Oussoren in branch 'default': |
I see that plistlib incorrectly writes large ints from 2**63 to 2**64-1 as negative values. >>> d = plistlib.dumps({'a': 18446744073709551615}, fmt=plistlib.FMT_BINARY)
>>> plistlib.loads(d)
{'a': -1} My patch did this correct (as 128-bit integer), and as you can see the produced file is accepted by Apple's plutil. |
However, I have no idea how to write that file using Apple's APIs. I'd prefer to either be compatible with Apple's API (current behavior), or just outright reject values that cannot be represented as a 64-bit signed integer. The file you generated happens to work, but as there is no way to create such as file using a public API there is little reason to expect that this will keep functioning in the future. The CFBinaryPlist code appears to be shared between support for binary plists and keyed archiving (more or less Cocoa's equivalent for pickle) and supports other values that cannot be put in plist files, such as sets. The original patch supported sets in the binary plist reader and writer, I ripped that out because such objects cannot be serialised using Apple's plist APIs. Keep in mind that this module is intended for interop with Apple's data format. |
Look in CFBinaryPList.c. It have a code for creating 128-bit integers:
And I suppose that you have at least one way to create such file -- just
Apple's tool can read and write integers from 2**63 to 2**64-1. Here is a patch against current sources. |
kCFNumberSInt128Type is not public API, see the list of number types in <https://developer.apple.com/library/mac/documentation/corefoundation/Reference/CFNumberRef/Reference/reference.html\>. I agree that CFBinaryPlist.c contains support for those, and for writing binary plists that contain sets, but you cannot create a 128 bit CFNumber object using a public API, and the public API for writing plists won't accept data structures containing sets. |
You have at least one way to create a 128 bit CFNumber. Read plist file (and In any case it is not good to produce incorrect plist for big integers. If you |
Reopening because Cocoa behaves differently that I had noticed earlier... The (Objective-C) code below serialises an NSDictionary with an unsigned long of value ULLONG_MAX and then reads it back. I had expected that restored value contained a negative number, but it actually reads back the correct value. I'm going to do some more spelunking to find out what's going on here, and will adjust the plistlib code to fully represent all values of unsigned 64-bit integers (likely based on your code for supporting 128-bit integers) Output (on a 64-bit system running OSX 10.9): $ ./demo
2014-01-15 15:34:18.196 demo[77580:507] input dictionary: {
key = 18446744073709551615;
} value 18446744073709551615
2014-01-15 15:34:18.198 demo[77580:507] as binary plist: <62706c69 73743030 d1010253 6b657914 00000000 00000000 ffffffff ffffffff 080b0f00 00000000 00010100 00000000 00000300 00000000 00000000 00000000 000020>
2014-01-15 15:34:18.198 demo[77580:507] Restored as {
key = 18446744073709551615;
} Code: /*
* To use:
* $ cc -o demo demo.c -framework Cocoa
* $ ./demo
*/
#import <Cocoa/Cocoa.h>
int main(void)
{
NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
NSNumber* value = [NSNumber numberWithUnsignedLongLong:ULLONG_MAX];
NSDictionary* restored = [NSPropertyListSerialization
propertyListWithData:serialized
options:0
format:nil
error:nil];
NSLog(@"Restored as %@", restored);
return 0;
} |
My last patch supports only values up to 2**64-1. Perhaps you will want to add new test case in |
New changeset 0121c2b7dcce by Ronald Oussoren in branch 'default': |
Serhiy: the issue should now be fixed. I finally understand why I was so sure that Apple's code serialised large positive numbers as negative numbers: due to a bug in PyObjC large positive numbers end up as NSNumber values that are interpreted as negative values. The patch tweaks the test generator to do the right thing by explicitly creating the NSNumber value instead of relying on PyObjC's automatic conversion. Now I just have to hunt down this bug in PyObjC :-) |
New changeset 728f626ee337 by R David Murray in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: