Edgewall Software
Modify

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

     
    505505                                                                                                                                                                                                     
    506506        cookies = self.outcookie.output(header='')                                                                                                                                                   
    507507        for cookie in cookies.splitlines():                                                                                                                                                          
    508             self._outheaders.append(('Set-Cookie', cookie.strip()))                                                                                                                                  
     508            self._outheaders.append(('Set-Cookie', str(cookie.strip())))                                                                                                                             
    509509                                                                                                                                                                                                     
    510510                                                                                                                                                                                                     
    511511class IAuthenticator(Interface):                                                                                                                                                                     

Attachments

assertion.html (6.6 KB) - added by Dave Abrahams <dave@…> 4 years ago.
Assertion error screen produced by fastcgi when this patch is needed
assertion.2.html (20.3 KB) - added by Dave Abrahams <dave@…> 4 years ago.
Correction: Assertion error screen produced by fastcgi when this patch is needed
login.html (20.4 KB) - added by Dave Abrahams <dave@…> 4 years ago.
more assertion errors
logout.html (20.1 KB) - added by Dave Abrahams <dave@…> 4 years ago.
still more assertion errors

Download all attachments as: .zip

Change History

comment:1 Changed 5 years ago by Dave Abrahams <dave@…>

The specific error was:

AssertionError?: Header values must be strings

comment:2 follow-up: 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: 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@…>

Assertion error screen produced by fastcgi when this patch is needed

Changed 4 years ago by Dave Abrahams <dave@…>

Correction: Assertion error screen produced by fastcgi when this patch is needed

comment:6 follow-up: Changed 4 years ago by cboos

Can you try this patch?

  • trac/web/api.py

     
    122122            self._strict_set(key, real_value, coded_value) 
    123123        except CookieError: 
    124124            self.bad_cookies.append(key) 
    125             dict.__setitem__(self, key, None) 
     125            dict.__setitem__(self, key, '') 
    126126 
    127127 
    128128class Request(object): 

comment:7 in reply to: ↑ 6 ; follow-up: Changed 4 years ago by Dave Abrahams <dave@…>

Replying to cboos:

Can you try this patch?

Seems to work!

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@…>

more assertion errors

Changed 4 years ago by Dave Abrahams <dave@…>

still more assertion errors

comment:9 follow-up: 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

Seems this was the same issue as #9176.

In any case, the fix r9409 would work also for this issue.

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from cboos. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.