Ticket #1164 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

turbogears.flash() doesn't work if string contains a comma

Reported by: snej Assigned to: jorge.vargas
Priority: low Milestone: 1.5
Component: TurboGears Version: 1.0b1
Severity: trivial Keywords:
Cc:

Description

The turbogears.flash() function doesn't work properly (usually has no visible effect) if the message string parameter contains a comma (",") character. I just spent about 20 minutes tearing out my hair trying to figure out why the message wouldn't show up.

I believe this happens because the comma is a delimiter in HTTP cookie syntax. TG would need to strip or escape comma characters when setting cookies to avoid this problem. Alternatively, it could store the flash message in the session dictionary instead of using a raw cookie for it.

Attachments

test.patch (1.5 kB) - added by Joost on 12/16/06 13:02:28.
test

Change History

12/16/06 13:02:28 changed by Joost

  • attachment test.patch added.

test

(follow-up: ↓ 5 ) 12/16/06 13:55:16 changed by Joost

I can't reproduce this problem in quickstarted project nor does the included regression test fail.

01/30/07 15:29:38 changed by jorge.vargas

  • owner changed from anonymous to jorge.vargas.
  • priority changed from normal to low.
  • severity changed from normal to trivial.
  • milestone set to 1.0.2.

this seems something simple to test.

02/02/07 21:48:24 changed by jorge.vargas

using your patch against trunk in r2527 the test passed here as well.

02/05/07 22:32:39 changed by plewis

tests in patch pass for me as well (1.0 branch, r2549)

(in reply to: ↑ 1 ) 04/04/07 16:41:35 changed by eaganj

Replying to Joost:

I can't reproduce this problem in quickstarted project nor does the included regression test fail.

This bug just cost me several hours and locks of hair. It appears to only happen in Safari (2.0.4/419.3) on the Mac, but not in Firefox. It is most definitely a real bug (whether in Safari or in TurboGears).

04/08/07 23:08:33 changed by jonypawks

I just tested this as well with [r2858]. The test added by the patch worked fine and the flash message was displayed properly in Safari (2.0.4/419.3) with a quickstarted app.

05/01/07 12:57:01 changed by alberto

  • milestone changed from 1.0.2 to 1.0.3.

07/03/07 10:35:11 changed by faide

  • milestone changed from 1.0.3 to 1.1.

Can someone with Safari confirm if he can display a flash including commas ?

04/06/08 06:53:58 changed by chrisz

This has come up on the mailing list again and I can confirm that this is still a problem with TG 1.0.4.4 and the current Safari 3.1 on Windows.

I think the problem is that flash messages are stored as raw text strings in a cookie, but cookie values should actually not contain commas, semicolons or even whitespace (http://wp.netscape.com/newsref/std/cookie_spec.html). The unit test won't show the problem because the CherryPy? and httptools client handle commas gracefully, as well as other browsers.

To fix this problem in TG 1.0.x I suggest either using urrlib.quote/unquote for escaping the flash message, or using a special function that only escapes commas, semicolons and whitespace.

And TG 1.1 and up should get a more sophisticated flash function using the CherryPy? session anyway.

04/06/08 09:43:22 changed by chrisz

  • status changed from new to closed.
  • resolution set to fixed.

Fixed in r4343 now, using urllib.quote/unquote, but only encoding the few troubled chars with quote, not all.

Btw, another reason why the attached unit test never detected any problem was that it did not do a redirect, so no cookie was involved at all. I have checked in a modified test with redirect that also checks the cookie directly for illegal chars.