[Python-Dev] Re: [Python-checkins] CVS: python/dist/src/Lib pickle.py,1.38,1.39

[Jeremy Hylton]

>>>>> "GvR" == Guido van Rossum <guido@beopen.com> writes:

  GvR> Hm...  This seems to add a lot of work to a very common item in
  GvR> pickles.

  GvR> I had a different idea on how to make this safe from abuse:
  GvR> pass eval a globals dict with an empty __builtins__ dict, as
  GvR> follows: {'__builtins__': {}}.

  GvR> Have you timed it?

I just timed it with a few test cases, using strings from
/dev/urandom. 

1. pickle dictionary with 25 items, 10-byte keys, 20-bytes values
   0.1% slowdown

2. pickle dictionary with 25 items, 15-byte keys, 100-byte values
   1.5% slowdown

3. pickle 8k string
   0.6% slowdown

The performance impact seems minimal.  And, of course, pickle is
already incredibly slow compared to cPickle.

So it isn't slow, but is it necessary?  I didn't give it much thought;
merely saw the cPickle did these checks in addition to calling eval
with an empty builtins dict.

Jim-- Is there a reason you added the "insecure string pickle"
feature?

I can't think of anything in particular that would go wrong other than
bizarre exceptions, e.g. OverflowError, SyntaxError, etc.  It would be
possible to construct pickles that produced unexpected objects, like
an instance with an attribute that is an integer

    >>> x
    <__main__.Foo instance at 0x8140acc>
    >>> dir(x)
    [3, 'attr']

But there are so many other ways to produce weird objects using pickle
that this particular one does not seem to matter.

The only arguments I'm left with, which don't seem particularly
compelling, are:

1. Simplifies error checking for client, which can catch ValueError
   instead of multiplicity of errors
2. Compatibility with cPickle interface

Barring better ideas from Jim Fulton, it sounds like we should
probably remove the checks from both picklers.

Jeremy