Closed Bug 206022 Opened 23 years ago Closed 23 years ago

[cookies] parser doesn't allow spaces in NAME token

Categories

(Core :: Networking: Cookies, defect)

Product:
Core
Component:
Networking: Cookies
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.4final

People

(Reporter: ted.schwind, Assigned: dwitte)

References

(
URL
)

Details

Attachments

(1 file)

proposed fix v1
7.83 KB, patch
mvl
: review+
alecf
: superreview+
asa
: approval1.4+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6 My company uses Kronos for timetracking. I have no problems using IE with Kronos. But I can not use MozillaFirebird. The first page always gives a cookie error. popup window title: [Javascript Application] Message: Your browser has cookies disabled. Cookies must be enabled to continue. The javascript console is empty. I have tried the following with no luck Cookie setting are to allow anything and don't ask for permission user_pref("dom.disable_cookie_get", false); and user_pref("dom.disable_cookie_set", false); A fresh install that starts with no existing config files. Reproducible: Always Steps to Reproduce: 1.Go to https://www.kronos.utah.edu/tkweb/html/ 2.This site also shows that javascript cookies are not recognized: http://gemal.dk/browserspy/cookie.html 3. Actual Results: Javascript cookie error Expected Results: No error message
-> me the site in 2) works fine for me on a trunk mozilla build (haven't tested firebird). confirmed site 1) is broken looks like more parser fun... the site tries to set a string containing spaces as the token name. this will break things... the site clearly violates spec by doing this, but since IE apparently works, it's probably time for me to get around to evaluating it. i'll try to test IE; and we'll probably want this for 1.4 :/ re site 2), that should be working. can you explicitly list your cookie prefs, and possibly try a mozilla build rather than firebird, to confirm that it's a firebird-specific problem?
Assignee: darin → dwitte
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → mozilla1.4final
The problem seems to be in the name of the cookie. kronos tries to set a cookie with "Kronos: WorkForce Central Suite Verification=success". But the confirmation dialog show only "Kronos:" as the name. So the cookie that wil be returned onle has "Kronos:", and the javascript sees that as not having cookies. The question: Is a colon in the name of a cookie allowed? And it would be interresting to know how IE handle it.
Same problem occurs at https://www.kronos.utah.edu/tkweb/html/ with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030517 ------------------------------- On this site "http://gemal.dk/browserspy/cookie.html" In Firebird I see: Variable Setting Cookies enabled? Yes JavaScript Cookies No or unable to detect... Meta Tag Cookies Yes Current Cookies browserspy_meta=test In Mozilla I see: Variable Setting Cookies enabled? Yes JavaScript Cookies Yes Meta Tag Cookies Yes Current Cookies browserspy_meta=test
thanks, the firebird issue is separate. filed bug 206149 for that.
Summary: Javascript cookies are either not enabled or undetectable → [cookies] parser doesn't allow spaces in NAME token
Can anybody please confirm if whitespaces are allowed by rfc in cookie names? For instance: <html><head> <script language=javascript> document.cookie = "cookie Name = cookieValue"; </script> </head> <body> </body> </html> works in 1.3.1 and older, but in 1.4x it doesn't. Somebody (not me) has a propietary application that sends this kind of cookies and updating to mozilla 1.4b has stopped working for this reason (IE works). Is this supposed to be fixed or should he stay in 1.3.1?
RFC2109 does _not_ allow spaces in cookie names. However, consistency with IE is more important, so I consider this a regression in 1.4 and I'll be fixing it shortly. so, your "somebody" will be able to upgrade to 1.4 no problem.
Attached patch proposed fix v1 — — Splinter Review
this patch changes the parser semantics to allow whitespace in token names (the same as token values). this makes the parsing rules more symmetric, although they aren't identical yet - we don't allow quoted-strings for token names. i'd like to leave that for a more thorough investigation of IE's behavior, which I don't have time for right now - it'd also be a higher-risk fix (with little reward) so it should probably wait for post-1.4. this fix broke one of the cookie testcases (an unimportant side-effect), so this changes the testcase to be compliant with the parser change. fyi, this patch does fix the site in question. ;)
Comment on attachment 123872 [details] [diff] [review] proposed fix v1 alecf, mvl: any chance you could review this for 1.4? the fix is pretty straightforward, and we've had two reports of site breakage so far, so it'd be nice to have this in.
Attachment #123872 - Flags: superreview?(alecf)
Attachment #123872 - Flags: review?(mvl)
Comment on attachment 123872 [details] [diff] [review] proposed fix v1 sr=alecf it continues to make me happy how you keep that BNF up to date. I love it!
Attachment #123872 - Flags: superreview?(alecf) → superreview+
Attachment #123872 - Flags: review?(mvl) → review+
Comment on attachment 123872 [details] [diff] [review] proposed fix v1 requesting approval for 1.4 - pretty simple change that fixes cookie bustage on a couple of reported sites. this shouldn't affect cookie handling for sites that currently work, so it's pretty low risk.
Attachment #123872 - Flags: approval1.4?
Comment on attachment 123872 [details] [diff] [review] proposed fix v1 You might want to add a test to TestCookie, to make sue you don't regress this in the future.
Comment on attachment 123872 [details] [diff] [review] proposed fix v1 a=asa (on behalf of drivers) for checkin to 1.4
Attachment #123872 - Flags: approval1.4? → approval1.4+
checked in thanks mvl, i forgot to do that here. i'll make a note to put it in with the next testcookie update.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
dwitte: re #8, what is the second site w/ the problem? re #13, is this in the branch+trunk?
>re #8, what is the second site w/ the problem? hmm, can't really remember now - perhaps it was the proprietary thing mentioned in comment 5? >re #13, is this in the branch+trunk? the fix is (obviously), but I haven't updated the cookie testsuite with a testcase for this bug yet.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: