Opened 4 years ago

Closed 3 years ago

#19664 closed Bug (fixed)

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

Reported by: Simon Blanchard Owned by: Erik Romijn
Component: contrib.sessions Version: master
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 4 years ago.
Error traceback

Download all attachments as: .zip

Change History (10)

comment:1 Changed 4 years ago by Simon Blanchard

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
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 Changed 4 years ago by supersteve9219

Triage Stage: UnreviewedDesign decision needed
Type: UncategorizedBug

comment:3 Changed 4 years ago by Aymeric Augustin

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?

Changed 4 years ago by Simon Blanchard

Error traceback

comment:4 Changed 4 years ago by Simon Blanchard

It's the _key_to_file method See attachment just uploaded

comment:5 Changed 4 years ago by Aymeric Augustin

Resolution: needsinfo
Status: closednew

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

comment:6 Changed 4 years ago by Aymeric Augustin

Triage Stage: Design decision neededAccepted

comment:7 Changed 3 years ago by Erik Romijn

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

comment:8 Changed 3 years ago by Erik Romijn

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 Changed 3 years ago by Erik Romijn

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