bpo-32072: Fix issues with binary plists.#4455
bpo-32072: Fix issues with binary plists.#4455serhiy-storchaka merged 3 commits intopython:masterfrom
Conversation
* Fixed saving bytearrays. * Identical objects will be saved only once. * Equal references will be load as identical objects. * Added support for saving and loading recursive data structures.
ronaldoussoren
left a comment
There was a problem hiding this comment.
Looks good to me.
I have two small comments, but the code is fine even if those are not addressed.
| result = [] | ||
| self._objects[ref] = result | ||
| result.extend(self._read_object(x) for x in obj_refs) | ||
| return result |
There was a problem hiding this comment.
Why the return statement? This saves a duplicate store to self._objects[ref] at the end of the method, but makes handling this token different from he previous ones.
There was a problem hiding this comment.
The tokens "array" and "dict" are special. Since they are containers and can contain a cyclic reference to itself, self._objects[ref] should be set before reading the content. These tokens already handled different from tokens for scalar types, and the difference will be kept even if remove the return statement here.
| result[self._read_object(self._object_offsets[k]) | ||
| ] = self._read_object(self._object_offsets[o]) | ||
| result[self._read_object(k)] = self._read_object(o) | ||
| return result |
There was a problem hiding this comment.
I guess the same applies to this (existing) return statement.
|
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6. |
* Fixed saving bytearrays. * Identical objects will be saved only once. * Equal references will be load as identical objects. * Added support for saving and loading recursive data structures. (cherry picked from commit a897aee)
|
GH-4654 is a backport of this pull request to the 3.6 branch. |
|
Thank you for your review @ronaldoussoren. |
* Fixed saving bytearrays. * Identical objects will be saved only once. * Equal references will be load as identical objects. * Added support for saving and loading recursive data structures. (cherry picked from commit a897aee)
* Fixed saving bytearrays. * Identical objects will be saved only once. * Equal references will be load as identical objects. * Added support for saving and loading recursive data structures.. (cherry picked from commit a897aee)
* [3.4] bpo-32072: Fix issues with binary plists. (GH-4455) * Fixed saving bytearrays. * Identical objects will be saved only once. * Equal references will be load as identical objects. * Added support for saving and loading recursive data structures.. (cherry picked from commit a897aee) * Fix implementation dependent assertion in test_plistlib. (#4813) It is failed with an advanced optimizer.
* [3.5] bpo-32072: Fix issues with binary plists. (GH-4455) * Fixed saving bytearrays. * Identical objects will be saved only once. * Equal references will be load as identical objects. * Added support for saving and loading recursive data structures. (cherry picked from commit a897aee) * Fix implementation dependent assertion in test_plistlib. (#4813) It is failed with an advanced optimizer.
* Add missing test class (mistake in GH-4455) * Increase coverage with 4 more test cases * Rename neg_uid to huge_uid in test_modified_uid_huge * Replace test_main() with unittest.main() * Update plistlib docs
* Add missing test class (mistake in GH-4455) * Increase coverage with 4 more test cases * Rename neg_uid to huge_uid in test_modified_uid_huge * Replace test_main() with unittest.main() * Update plistlib docs. (cherry picked from commit d0d9f7c) Co-authored-by: Jon Janzen <[email protected]>
* Add missing test class (mistake in pythonGH-4455) * Increase coverage with 4 more test cases * Rename neg_uid to huge_uid in test_modified_uid_huge * Replace test_main() with unittest.main() * Update plistlib docs
* Add missing test class (mistake in pythonGH-4455) * Increase coverage with 4 more test cases * Rename neg_uid to huge_uid in test_modified_uid_huge * Replace test_main() with unittest.main() * Update plistlib docs
https://bugs.python.org/issue32072