beaker icon indicating copy to clipboard operation
beaker copied to clipboard

Session json data_serializer is ignored

Open mohierf opened this issue 8 years ago • 7 comments

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 ?

mohierf avatar Feb 25 '17 08:02 mohierf

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 https://github.com/bbangert/beaker/blob/f377072eb1fb99bce73734eaff1350fbcd6514b7/beaker/session.py#L464 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 :)

amol- avatar Feb 25 '17 09:02 amol-

Thanks. I will give a try and keep you informed

mohierf avatar Feb 25 '17 11:02 mohierf

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

mohierf avatar Feb 26 '17 07:02 mohierf

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 avatar Dec 30 '19 00:12 dialtone

I have submitted a PR https://github.com/bbangert/beaker/pull/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 avatar Dec 30 '19 14:12 dialtone

Looks like all tests pass.

dialtone avatar Dec 30 '19 15:12 dialtone

Anyone here?

dialtone avatar Jan 05 '20 23:01 dialtone