Ticket #5636 (closed defect: duplicate)
Opened 5 years ago
Last modified 12 months ago
unicode encoding error in cookie
| Reported by: | Dave Abrahams <dave@…> | Owned by: | cboos |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | web frontend | Version: | devel |
| Severity: | normal | Keywords: | unicode cookie |
| Cc: | |||
| Release Notes: | |||
| API Changes: | |||
Description
I ran into a situation where the cookie was unicode but the rest of the code expected a str; the following made it go away:
-
trac/web/api.py
505 505 506 506 cookies = self.outcookie.output(header='') 507 507 for cookie in cookies.splitlines(): 508 self._outheaders.append(('Set-Cookie', cookie.strip()))508 self._outheaders.append(('Set-Cookie', str(cookie.strip()))) 509 509 510 510 511 511 class IAuthenticator(Interface):
Attachments
Change History
comment:1 Changed 5 years ago by Dave Abrahams <dave@…>
comment:2 follow-up: ↓ 3 Changed 5 years ago by cboos
- Keywords unicode cookie added
- Owner changed from jonas to cboos
It would probably be worth investigating how a unicode value could get in.
Also, str() conversion is not robust in case that unicode value contains anything else than 'ascii' characters, on most systems.
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 4 Changed 5 years ago by Dave Abrahams <dave@…>
Replying to cboos:
It would probably be worth investigating how a unicode value could get in.
Definitely. But I don't know where to start. Frankly I'm not even 100% positive that it was unicode and not, say, an int. If you have specific tests you'd like me to run, I can try to do that.
Also, str() conversion is not robust in case that unicode value contains anything else than 'ascii' characters, on most systems.
I know. Maybe I should be using something from urlencode?
comment:4 in reply to: ↑ 3 Changed 5 years ago by cboos
Replying to Dave Abrahams <dave@boost-consulting.com>:
Replying to cboos:
It would probably be worth investigating how a unicode value could get in.
Definitely. But I don't know where to start. Frankly I'm not even 100% positive that it was unicode and not, say, an int. If you have specific tests you'd like me to run, I can try to do that.
Me neither ;-) Add some more debugging info?
Also, str() conversion is not robust in case that unicode value contains anything else than 'ascii' characters, on most systems.
I know. Maybe I should be using something from urlencode?
The brute force way would be to_unicode(...).encode('utf-8').
comment:5 Changed 4 years ago by ThurnerRupert
- Milestone changed from 0.11 to 0.11.1
nobody else noticed it ... maybe its save to leave it until 0.11.1 ... also seems to be no degression.
Changed 4 years ago by Dave Abrahams <dave@…>
- Attachment assertion.html added
Assertion error screen produced by fastcgi when this patch is needed
Changed 4 years ago by Dave Abrahams <dave@…>
- Attachment assertion.2.html added
Correction: Assertion error screen produced by fastcgi when this patch is needed
comment:6 follow-up: ↓ 7 Changed 4 years ago by cboos
Can you try this patch?
-
trac/web/api.py
122 122 self._strict_set(key, real_value, coded_value) 123 123 except CookieError: 124 124 self.bad_cookies.append(key) 125 dict.__setitem__(self, key, None)125 dict.__setitem__(self, key, '') 126 126 127 127 128 128 class Request(object):
comment:7 in reply to: ↑ 6 ; follow-up: ↓ 8 Changed 4 years ago by Dave Abrahams <dave@…>
comment:8 in reply to: ↑ 7 Changed 4 years ago by Dave Abrahams <dave@…>
Replying to Dave Abrahams <dave@boost-consulting.com>:
Replying to cboos:
Can you try this patch?
Seems to work!
I take it back; still broken. See login.html and logout.html
Changed 4 years ago by Dave Abrahams <dave@…>
- Attachment logout.html added
still more assertion errors
comment:9 follow-up: ↓ 10 Changed 4 years ago by cboos
Well, perhaps could you add some tracing in that Cookie class in order to see what's really going on?
comment:10 in reply to: ↑ 9 Changed 4 years ago by Dave Abrahams <dave@…>
Replying to cboos:
Well, perhaps could you add some tracing in that Cookie class in order to see what's really going on?
I'm happy to try specific patches you suggest and let you know the output, but I can't afford to try to figure out what to trace by myself.
comment:11 Changed 12 months ago by cboos
- Component changed from general to web frontend
- Milestone next-minor-0.12.x deleted
- Resolution set to duplicate
- Status changed from new to closed



The specific error was: