Skip to content
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

Option to allow custom JSONEncoder/JSONDecoder #21

Open
charliewolf opened this issue Jul 14, 2016 · 10 comments
Open

Option to allow custom JSONEncoder/JSONDecoder #21

charliewolf opened this issue Jul 14, 2016 · 10 comments

Comments

@charliewolf
Copy link
Contributor

charliewolf commented Jul 14, 2016

This would work better for some types, i.e. datetime, etc.

Your country thanks you.

@thomasst
Copy link
Member

Instead of pickle, how about being able to specify a custom JSONEncoder/JSONDecoder?

@charliewolf
Copy link
Contributor Author

That would probably be good to have too. That said, pickle would be easier out-of-the-box.

@thomasst
Copy link
Member

I'm not a fan of implementing pickle since it allows code execution (Warning The pickle module is not secure against erroneous or maliciously constructed data. Never unpickle data received from an untrusted or unauthenticated source.), but I'm fine with custom JSON encoders/decoders. Patch welcome :)

@thomasst thomasst changed the title Option to use pickle instead of json for [de]serialization Option to allow custom JSONEncoder/JSONDecoder Jul 14, 2016
@tkram01
Copy link

tkram01 commented Sep 9, 2016

We would like the option to use pickle as well. Some swig wrappers support pickle serialization and not JSON.

@wojcikstefan
Copy link
Member

Personally, I don't think there's a point in implementing an insecure serialization/de-serialization option, especially given how often tasks are called with params containing user-generated data.

@tkram01 you could probably write a very simple middleware that unpickles an object you get from SWIG and then serializes it into JSON before passing it to a TaskTiger task.

@philfreo
Copy link
Member

philfreo commented Sep 9, 2016

(See also #22)

@thomasst
Copy link
Member

thomasst commented Sep 9, 2016

Yeah, I'll review this later, but we can probably do something like #22.

@charliewolf
Copy link
Contributor Author

@wojcikstefan @thomasst I'm not seeing how Pickle is a security issue if the python app is control of pickling/unpickling. My understanding was that the only secufity issue is if you were to accept arbitrary pickled data from users but Tasktiger doesn't do that since it does the serialization/deserialization itself.

@thomasst
Copy link
Member

thomasst commented Sep 9, 2016

That's correct, but it's bad design and unnecessarily opens up a potential point of attack. It's like saying you can use gets() in C because the input size is known.

That being said, I don't mind having an option for a customer (de)serializer, as long as we don't ship pickle with TaskTiger (but the user is free to implement/configure it).

@alanhamlett
Copy link
Contributor

That being said, I don't mind having an option for a customer (de)serializer, as long as we don't ship pickle with TaskTiger (but the user is free to implement/configure it).

And just point people in the right direction with a custom JSONEncoder/Decoder example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants