Opened 11 years ago

Closed 11 years ago

#19664 closed Bug (fixed)

Illegal Characters In Session Key Give Fatal Error On File Backend Only

Reported by: Simon Blanchard Owned by: Sasha Romijn
Component: contrib.sessions Version: dev
Severity: Normal Keywords: cookies sessions
Cc: bnomis@…, eromijn@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The file backend for sessions checks for illegal characters in the key. If there are any illegal characters it throws a SuspiciousOperation exception. It is the only backend that check for this and throws an error. Shouldn't every backend either check or none of them?

This is an issue because I occasionally get http clients accessing my sites with a comma separating cookies rather than a semicolon in the HTTP_COOKIE variable. Python parses the cookie string wrongly (according to the spec.) and I end up with a comma at the end of the first cookie. I've reported this in the Python issue tracker http://bugs.python.org/issue16362 It's known behaviour and will not be fixed.

I suspect this has not been noticed by many since not many use the file backend.

So do we really need to throw an error here. Or could we just return a new session?

Attachments (1)

[Django] ERROR (EXTERNAL IP)_ Internal Server Error_ _en_.eml (3.9 KB ) - added by Simon Blanchard 11 years ago.
Error traceback

Download all attachments as: .zip

Change History (10)

comment:1 by Simon Blanchard, 11 years ago

Summary: Illegal Characters Im Session Key Give Fatal Error On File Backend OnlyIllegal Characters In Session Key Give Fatal Error On File Backend Only

comment:2 by supersteve9219, 11 years ago

Triage Stage: UnreviewedDesign decision needed
Type: UncategorizedBug

comment:3 by Aymeric Augustin, 11 years ago

Resolution: needsinfo
Status: newclosed

I cannot locate any specific check in the file backend; it just calls validate_key from the base class.

Could you clarify which check you're referring to, and provide a complete traceback?

by Simon Blanchard, 11 years ago

Error traceback

comment:4 by Simon Blanchard, 11 years ago

It's the _key_to_file method See attachment just uploaded

comment:5 by Aymeric Augustin, 11 years ago

Resolution: needsinfo
Status: closednew

Oops, I was looking at the cache backend and not at the session backend, sorry!

comment:6 by Aymeric Augustin, 11 years ago

Triage Stage: Design decision neededAccepted

comment:7 by Sasha Romijn, 11 years ago

Cc: eromijn@… added
Owner: changed from nobody to Sasha Romijn
Status: newassigned
Version: 1.5-beta-1master

comment:8 by Sasha Romijn, 11 years ago

All other session backends simply create a new session if there is some kind of problem with the provided session key. I think it absolutely makes sense to do this for the file backend too.

PR: https://github.com/django/django/pull/1172

comment:9 by Sasha Romijn, 11 years ago

Has patch: set
Resolution: fixed
Status: assignedclosed
Triage Stage: AcceptedReady for checkin
Note: See TracTickets for help on using tickets.
Back to Top