Opened 2 years ago

Closed 2 years ago

#19664 closed Bug (fixed)

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

Reported by: simonb Owned by: erikr
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 simonb 2 years ago.
Error traceback

Download all attachments as: .zip

Change History (10)

comment:1 Changed 2 years ago by simonb

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from Illegal Characters Im Session Key Give Fatal Error On File Backend Only to Illegal Characters In Session Key Give Fatal Error On File Backend Only

comment:2 Changed 2 years ago by supersteve9219

  • Triage Stage changed from Unreviewed to Design decision needed
  • Type changed from Uncategorized to Bug

comment:3 Changed 2 years ago by aaugustin

  • Resolution set to needsinfo
  • Status changed from new to 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?

Changed 2 years ago by simonb

Error traceback

comment:4 Changed 2 years ago by simonb

It's the _key_to_file method See attachment just uploaded

comment:5 Changed 2 years ago by aaugustin

  • Resolution needsinfo deleted
  • Status changed from closed to new

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

comment:6 Changed 2 years ago by aaugustin

  • Triage Stage changed from Design decision needed to Accepted

comment:7 Changed 2 years ago by erikr

  • Cc eromijn@… added
  • Owner changed from nobody to erikr
  • Status changed from new to assigned
  • Version changed from 1.5-beta-1 to master

comment:8 Changed 2 years ago by erikr

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 2 years ago by erikr

  • Has patch set
  • Resolution set to fixed
  • Status changed from assigned to closed
  • Triage Stage changed from Accepted to Ready for checkin
Note: See TracTickets for help on using tickets.
Back to Top