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

Session json data_serializer is ignored #122

Open
mohierf opened this issue Feb 25, 2017 · 7 comments
Open

Session json data_serializer is ignored #122

mohierf opened this issue Feb 25, 2017 · 7 comments

Comments

@mohierf
Copy link

mohierf commented Feb 25, 2017

Using the 1.8.1 version, with those parameters:

{
  'session.timeout': None, 
  'session.cookie_expires': 'True', 
  'session.key': u'Alignak-WebUI', 
  'session.data_dir': u'/tmp/Alignak-WebUI/sessions', 
  'sesssion.webtest_varname': u'Alignak-WebUI', 
  'session.save_accessed_time': True, 
  'session.data_serializer': 'json', 
  'session.type': 'file', 
  'session.auto': True}

I expected the session files to be serialized with a JSON format, but the resulting file is pickled:

(dp1
S'session'
p2
(dp3
S'_accessed_time'
p4
F1488010180.6782351
sS'current_realm'
p5
ccopy_reg
_reconstructor
p6
(calignak_webui.objects.item_realm
Realm
p7
c__builtin__
object
...
...

Am I missing something ?

@amol-
Copy link
Collaborator

amol- commented Feb 25, 2017

JSON serialiser was introduced mostly for security reason in sending/receiving cookies (as pickle might lead to code injection through the sent cookie) but I don't think it was ever extended to the on-server storage as in that case the session is stored & loaded locally so there is no risk of receiving malicious data (to craft the data user needs access to your server/db and in that case you already have worse problems).

Probably removing this check

if self.encrypt_key:
is already enough to extend the marshaling to locally saved session too.

If you want to try and submit a pull request with an associated test to prevent future regressions I would gladly review and merge it. Feel free to ask if you need any guidance in setting up a local development environment for beaker :)

@mohierf
Copy link
Author

mohierf commented Feb 25, 2017

Thanks. I will give a try and keep you informed

@mohierf mohierf changed the title Session json daa_serializer is ignored Session json data_serializer is ignored Feb 26, 2017
@mohierf
Copy link
Author

mohierf commented Feb 26, 2017

I tried your suggestion but more than 20 unit tests are broken :/ At the moment I do not have enough time to investigate more for this feature, sorry. Perharps in some days ...

@dialtone
Copy link
Contributor

Adding to the +1 here... Given the number of unittests that break I'll probably just go with a patched version of beaker with this changed and no particular care for the broken tests.

@dialtone
Copy link
Contributor

I have submitted a PR #186 to deal with this. A lot of tests break for unrelated reasons (like missing AES library or missing client of a particular backend) but the PR should work nonetheless given how encapsulated it is in the Session object.

@dialtone
Copy link
Contributor

Looks like all tests pass.

@dialtone
Copy link
Contributor

dialtone commented Jan 5, 2020

Anyone here?

amol- pushed a commit that referenced this issue May 20, 2020
* fix ignoring json data_serializer for non-encrypted cookies
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

3 participants