Edgewall Software
Modify

Opened 17 years ago

Closed 14 years ago

#5636 closed defect (duplicate)

unicode encoding error in cookie

Reported by: Dave Abrahams <dave@…> Owned by: Christian Boos
Priority: normal Milestone:
Component: web frontend Version: devel
Severity: normal Keywords: unicode cookie
Cc: Branch:
Release Notes:
API Changes:
Internal 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 (4)

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

Download all attachments as: .zip

Change History (15)

comment:1 by Dave Abrahams <dave@…>, 17 years ago

The specific error was:

AssertionError: Header values must be strings

comment:2 by Christian Boos, 17 years ago

Keywords: unicode cookie added
Owner: changed from Jonas Borgström to Christian Boos

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.

in reply to:  2 ; comment:3 by Dave Abrahams <dave@…>, 17 years ago

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?

in reply to:  3 comment:4 by Christian Boos, 17 years ago

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 by ThurnerRupert, 17 years ago

Milestone: 0.110.11.1

nobody else noticed it … maybe its save to leave it until 0.11.1 … also seems to be no degression.

by Dave Abrahams <dave@…>, 17 years ago

Attachment: assertion.html added

Assertion error screen produced by fastcgi when this patch is needed

by Dave Abrahams <dave@…>, 17 years ago

Attachment: assertion.2.html added

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

comment:6 by Christian Boos, 17 years ago

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):

in reply to:  6 ; comment:7 by Dave Abrahams <dave@…>, 17 years ago

Replying to cboos:

Can you try this patch?

Seems to work!

in reply to:  7 comment:8 by Dave Abrahams <dave@…>, 17 years ago

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

by Dave Abrahams <dave@…>, 17 years ago

Attachment: login.html added

more assertion errors

by Dave Abrahams <dave@…>, 17 years ago

Attachment: logout.html added

still more assertion errors

comment:9 by Christian Boos, 17 years ago

Well, perhaps could you add some tracing in that Cookie class in order to see what's really going on?

in reply to:  9 comment:10 by Dave Abrahams <dave@…>, 17 years ago

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 by Christian Boos, 14 years ago

Component: generalweb frontend
Milestone: next-minor-0.12.x
Resolution: duplicate
Status: newclosed

Seems this was the same issue as #9176.

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.