Added json as an option for cookie serialization #56
Conversation
|
Btw, should it actually default to json? |
| return pickle.dumps(data, 2) | ||
|
|
||
| def deserialize(data_string, method): | ||
| try: |
There was a problem hiding this comment.
this try block, try to unpickle first, then if that fails use json... defeats the whole purpose of this patch, it's still going to run pickle...
why not just check 'if method == "json"' first, rather than attempting to unpickle first.
There was a problem hiding this comment.
That was my first intention, indeed, but then I realized you're losing existing, pickled, data.
There was a problem hiding this comment.
true, but at the expense of security, which I personally think is more important.
There was a problem hiding this comment.
What expense of security?
Projects that are based on beaker and are currently live, probably use pickle. If you decide to switch from pickle to json (and vice versa), that try block will make sure you don't lose that data. When it reads the data, it tries both formats. When it writes back, it uses the selected format. That means existing pickled data will be json serialized. So, basically, it's for compatibility.
If you think that data might be compromised, then you should probably delete it yourself. My concern, as a developer, is not [to work around] your [compromised] data, but the behavior of the library.
Addresses issue #36 (which in turn addressed issue #35)