Opened 12 years ago

Closed 12 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 12 years ago.
Error traceback

Download all attachments as: .zip

Change History (10)

comment:1 by Simon Blanchard, 12 years ago

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

comment:2 by supersteve9219, 12 years ago

Triage Stage: Unreviewed → Design decision needed
Type: Uncategorized → Bug

comment:3 by Aymeric Augustin, 12 years ago

Resolution: → needsinfo
Status: new → closed

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, 12 years ago

Attachment: [Django] ERROR (EXTERNAL IP)_ Internal Server Error_ _en_.eml​ added

Error traceback

comment:4 by Simon Blanchard, 12 years ago

It's the _key_to_file method See attachment just uploaded

comment:5 by Aymeric Augustin, 12 years ago

Resolution: needsinfo
Status: closed → new

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

comment:6 by Aymeric Augustin, 12 years ago

Triage Stage: Design decision needed → Accepted

comment:7 by Sasha Romijn, 12 years ago

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

comment:8 by Sasha Romijn, 12 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, 12 years ago

Has patch: set
Resolution: → fixed
Status: assigned → closed
Triage Stage: Accepted → Ready for checkin

Fixed by aaugustin in f88700d610d69ba5b44ab7b0692f8792aab3b54b ​https://github.com/django/django/commit/f88700d610d69ba5b44ab7b0692f8792aab3b54b

Note: See TracTickets for help on using tickets.
Back to Top