tasktiger icon indicating copy to clipboard operation
tasktiger copied to clipboard

Option to allow custom JSONEncoder/JSONDecoder

Open charliewolf opened this issue 9 years ago • 10 comments

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

Your country thanks you.

charliewolf avatar Jul 14 '16 16:07 charliewolf

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

thomasst avatar Jul 14 '16 16:07 thomasst

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

charliewolf avatar Jul 14 '16 16:07 charliewolf

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 avatar Jul 14 '16 17:07 thomasst

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

tkram01 avatar Sep 09 '16 14:09 tkram01

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.

wojcikstefan avatar Sep 09 '16 15:09 wojcikstefan

(See also #22)

philfreo avatar Sep 09 '16 15:09 philfreo

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

thomasst avatar Sep 09 '16 15:09 thomasst

@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.

charliewolf avatar Sep 09 '16 16:09 charliewolf

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).

thomasst avatar Sep 09 '16 17:09 thomasst

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.

alanhamlett avatar Jan 19 '17 05:01 alanhamlett