search

zopefoundation / persistent

automatic persistence for Python objects
https://pypi.org/project/persistent/
Other
46 stars 28 forks source link

Should `_p_oid` allow arbitrary types? #24

Closed tseaver closed 8 years ago

tseaver commented 9 years ago

The docstring for IPersistent._p_oid claims that it must be bytes. Historically, the C implementation has allowed any type (although values which are not length 8 bytes blow up if used with a standard pickle cache).

See: https://github.com/zopefoundation/persistent/pull/20#discussion-diff-28083537 for pros and cons.

jamadden commented 9 years ago

There's additional discussion on that same PR here.

jamadden commented 9 years ago

I just discovered another inconsistency in https://github.com/NextThought/ZODB/commit/5d38a086be4bcf20558d8bcec33cd95d065a7082.

It turns out that the cPickleCache implementation only enforces the 8-length byte constraint in the __setitem__ implementation. If you use new_ghost, it accepts any arbitrary value for the oid. The Python implementation enforces it both places.

ZODB initially creates and populates the picklecache using new_ghost (Connection.get), so in practice it might seem like if you're using the cPickleCache the restriction is easily bypassed. However, once you commit the transaction, if you modified any of those objects, then you'll find that it calls __setitem__ and things blow up with a TypeError simply because cPickleCache checks the type of the key before checking to see if the key already exists.

strichter commented 9 years ago

I'll note that when I use a non-bytes _p_oid, I do not use pickle at all. Whenever pickle is used, you might as well use the ZODB facilities, because it takes care of most things you would ever want.

For me I specifically went down to the persistent layer for pjpersist because it does not deal with serialization and only provides the messaging framework.

tseaver commented 8 years ago

I'll close this with the note that the consensus answer is "No, OIDs do not have to be bytes of length 8" for implementations which use jars other than those supplied by ZODB.