Closed
Bug 62348
Opened 24 years ago
Closed 22 years ago
empty cookie path treated incorrectly
Categories
(Core :: Networking: Cookies, defect, P3)
Core
Networking: Cookies
Tracking
()
VERIFIED
FIXED
mozilla1.0.1
People
(Reporter: st.n, Assigned: morse)
References
Details
(Whiteboard: [need info][fixed-trunk] [Needs refresh of a=])
Attachments
(1 file, 2 obsolete files)
689 bytes,
patch
|
jesup
:
review+
jesup
:
superreview+
jesup
:
approval+
|
Details | Diff | Splinter Review |
I would expect that a cookie without a path would be treated like a cookie with the path '/' (coming from Set-Cookie: ...; path=/). But Mozilla stores these as two different cookies, which can be verified by opening the cookie manager. Additionally, Mozilla sends this cookie twice to the server (like this: Cookie: max=50; max=200), where the second one is the one with the path '/'. And what's more annoying: it is impossible to remove the cookie without a path with a Set-Cookie:-request, at least not with an expiration time in the past and without a specified path. Unfortunately, IE handles all this like I would expect. :-( I could provide a PHP script as a test case, if that would help. The essential part is that I used to set a cookie with setcookie( 'max', $max, time() + 90*24*60*60 ); and now that I included a path in a newer version like this: setcookie( 'max', $max, time() + 90*24*60*60, '/' ); I would like to remove the old cookie like this: setcookie( 'max', '', 1 ); You can find the description of the setcookie function at http://php.net/setcookie . I'm using nightly build 2000112620 in NT4 at the moment, but I don't think that matters, so I'm setting Platform and OS to all.
Assignee | ||
Comment 1•24 years ago
|
||
A complete testcase (html file containing javascript set-cookies) would be helpful here. How does 4.x treat this?
Reporter | ||
Comment 2•24 years ago
|
||
Unfortunately I'm not very experienced with Javascript yet, so I can't write a test case before next week. Could someone give my some hints on how to do this, or links to documentation? As I said, I wrote this in PHP. And I haven't tried it with Nav4.x yet, but I'm also going to report here as soon as I do. But it's Friday evening here in Germany already, so again: next week. :-)
Comment 4•24 years ago
|
||
Netscape Nav triage team: this is not a Netscape beta stopper.
Keywords: nsbeta1-
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2
Comment 5•22 years ago
|
||
OK, I ran into the same problem and here are more details on this bug. According to the spec at http://www.netscape.com/newsref/std/cookie_spec.html "If the path is not specified, it as assumed to be the same path as the document being described by the header which contains the cookie." The questions is: What is a "path" in this context? The script that i was accessing had an URL like http://www.example.com/332/login.php?target=http://www.example.info/idx_menu.php&username=a&password=b This script's output contained a set-cookie header without a path option. The path of the cookie was set to path=/332/login.php?target=http://www.example.info I believe it should have been set to path=/332/login.php This would be in accordance with what javascript thinks is a path of a URL (location.path). It seems that the code that determines the path looks for the last slash in the URL, but it should stop at the first question mark it encounters. This bug is somewhat complicated to demonstrate with a javascript testcase since it depends on the URL of the page that is served. If necessary, I can however provide a test setup that you can access remotely (with a browser) that shows this bug. For reference: It seems both Netscape 4.x and MS IE 6 don't put the query string (the part after the "?" including the "?" itself) into the cookie's path.
Comment 6•22 years ago
|
||
More info: It seems the RFC 2109 regarding cookies is more recent than Netscape's specification. It states at http://www.cis.ohio-state.edu/cgi-bin/rfc/rfc2109.html#sec-4.3.1 Path Defaults to the path of the request URL that generated the Set-Cookie response, up to, but not including, the right-most /. This is exactly what mozilla does at the moment, but I still think it's a bug because the RFC authors didn't consider the query string (the part following the "?") and the "/"'s it might contain. I am CC'ing them, maybe they can confirm my theory? Once again: for the URL http://example.com/dir/script?param1=abc¶m2=de/fg¶m3=gh I think the path should default to http://example.com/dir and not to http://example.com/dir/script?param1=abc¶m2=de David and Lou, please comment.
Assignee | ||
Comment 7•22 years ago
|
||
If there was a slash in the query string, it would probably need to be escaped.
Comment 8•22 years ago
|
||
This patch is to the 0.9.9 source. I couldn't test it because I don't have a build environment ready. It should strip down everything after the first question mark to get the path. If there is no question mark, the old behaviour is used.
Well, for one the "slash" character is considered a reserved character in the query string and needs to be escaped. THerefore, it should NEVER appear unescaped in the query string. See W3C doc - http://www.w3.org/Addressing/URL/uri-spec.html Section: BNF of generic URI syntax reserved = | ; | / | # | ? | : | space Therefore the spec is correct and the URL that fails does so because it is an inproperly formed URL (and should fail).
Comment 10•22 years ago
|
||
I'm not sure if it needs to be escaped. Does RFC 1630 page 7 "QUERY STRING" or RFC 2396 section 3.4 apply? RFC 2396 does not claim to supersede or update RFC 1630. A page with an unescaped "/" in the query string passes through the "HTML 4.01 strict" validator at the W3C. There are lots of pages that don't encode "&" (as "&") and "/" properly in the query string. It would be good of mozilla would tolerate such errors in this simple case.
Comment 11•22 years ago
|
||
Email to both authors of RFC 2109 bounces. If an unescaped "/" is illegal in the query part of the URL, i think mozilla should ignore it in its attempt to determine the path. This is what my patch does.
Comment 12•22 years ago
|
||
There is an issue with my patch. For the URL http://example.org/a/b?c the path will be http://example.org/a/b but it should be http://example.org/a Could someone come up with a better patch? It should find the last slash in the URL, but not past the first question mark.
Comment 13•22 years ago
|
||
Why not just use this algorithm: 1. Parse the URL on the "?" into an object (like an array) 2. Take the first element of the object, get the position of the last instance of the "/" character 3. Using that position, get the substring from 0 --> position 4. There's your path :)
Comment 14•22 years ago
|
||
This one works like before with query strings that don't have "/" in them. It obsoletes my attachment 74330 [details] [diff] [review] (i'm not authorized to check the box.. bleh).
Comment 15•22 years ago
|
||
Comment on attachment 74330 [details] [diff] [review] Use path up to first "?", then try last "/" buggy. obsoleted by attachment 74346 [details] [diff] [review]
Attachment #74330 -
Attachment is obsolete: true
Assignee | ||
Comment 16•22 years ago
|
||
I'm not convinced there is a problem here. If the query string contains an unescaped /, then the query string is incorrectly formed (comment 7 and comment 9) and we have an evangelism issue. However if IE is tolerant enough to accept this malformed query, then perhaps we should be too. That said, the patch that you are proposing is indeed the correct way to handle this. My only criticism is the use of the variable name "slash" since you are using it when you look for the questionmark too. Change that to a more generic name, such as pathEndChar or something equivalent. With that change, r=morse. Now you need to get yourself an sr.
Assignee | ||
Comment 17•22 years ago
|
||
*** Bug 121355 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 18•22 years ago
|
||
OK, there are starting to be dups on this one, and I'm afraid there are going to be many sites that will fail because of this problem. Even though it's an evangelism issue for those sites, we are the ones that are going to get the blame. Therefore I'm going to recommend that this one be considered for nsbeta1. Especially since we already have a patch for it and it's probably of low risk.
Keywords: nsbeta1
Target Milestone: mozilla1.2alpha → mozilla1.0
Comment 19•22 years ago
|
||
Nav triage team needs info: What is the real impact here? Does this affect any top sites?
Whiteboard: [need info]
Assignee | ||
Comment 20•22 years ago
|
||
I have no idea whether any top sites are affected. We won't know the answer to that until after we release the product and start getting complaints. This is a case where we can avoid the problem completely with a relatively safe fix that already exists (thanks to a web contributor). So why should we not use it?
Comment 21•22 years ago
|
||
Impact in bug 121355: Unable to access paid content on a site where verification uses cookies. Even though the underlying problem lies with the site, this works in Netscape 3,4.x,6.0-6.2, Mozilla <0.9.6, IE 4-6, Opera 5-6, and Konqueror, so users that run into a similar problem will very likely blame Mozilla. Evangelism is of course an option, but the problem with the respective sites can be rather difficult to diagnose. Infact, I tried to look into the source-code of the non-working site, but the incriminated unescaped slash is hidden in a variable that is set by a php-script and normally not seen by the user. Most people lack morse's skills and dedication, so often we won't even know we should evangelize the affected sites. Much more probable, the user will just turn away from Mozilla/Netscape (especially if the browser does not show "my" *paid* content).
Comment 22•22 years ago
|
||
sr=jag provided Morse's comment about the variable name is addressed. A common name for such variables is |iter|.
Comment 23•22 years ago
|
||
Comment on attachment 74346 [details] [diff] [review] Find last "/" but not past first "?" a=asa (on behalf of drivers) for checkin to the 1.0 branch
Attachment #74346 -
Flags: approval+
Assignee | ||
Comment 24•22 years ago
|
||
Checked in on trunk. Can't check in on branch due to lack of adt approval.
Comment 25•22 years ago
|
||
Here's the patch again with the requested name change. Thanks for considering it for 1.0.
Attachment #74346 -
Attachment is obsolete: true
Assignee | ||
Comment 26•22 years ago
|
||
Sven, thanks for the updated patch. It's exactly what I've already checked in on the trunk before you made your latest posting (I had made those same changes based on my comments and jag's).
Comment 27•22 years ago
|
||
Just curious - is there a "contributers" file or something I get added to for fixing this problem? :-)
Comment 28•22 years ago
|
||
Thank you Steve, Sven! This fixed the problem in bug 121355. Hopefully we will get approval for the branch before release.
Comment 29•22 years ago
|
||
tever, please test the bug fix on the trunk and add your comments here. The adt needs to know before the fix is checked into the branch. thanks.
Comment 31•22 years ago
|
||
Samir, what's the reason for the nsbeta-? Please check morse's comment 18 as well as comment 21. Since the patch is checked into the trunk (and fixes the problem with the site), should this be renominated nsbeta after a few days of baking?
Comment 32•22 years ago
|
||
adt1.0.0- on behalf of the adt. Not needed for MachV.
Updated•22 years ago
|
Updated•22 years ago
|
Whiteboard: [need info] → [need info][fixed-trunk]
Comment 33•22 years ago
|
||
This bug is still present in 1.0rc2 it seems. Does this mean it won't make it into 1.0? I can't access a site with mozilla because of this. It works fine in all other major browsers.
Assignee | ||
Comment 34•22 years ago
|
||
Sven, see comments 24, 30, and 32. Unless it gets adt1.0.0+ it can't go into the branch.
Assignee | ||
Comment 35•22 years ago
|
||
*** Bug 144148 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 36•22 years ago
|
||
removing minus to get back on ADT radar.
Assignee | ||
Comment 37•22 years ago
|
||
Being marked fixed since it is checked in on trunk, even though it is still not fixed on the branch.
Assignee | ||
Updated•22 years ago
|
Keywords: mozilla1.0
Comment 38•22 years ago
|
||
Please get an nsbeta1+ from the nav-triage team before seeking ADT approval.
Comment 39•22 years ago
|
||
nsbeta1+ per Nav triage team, since minimal Nav team work is required.
Comment 40•22 years ago
|
||
adt1.0.0+ (on ADT's behalf) for approval to checkin to the 1.0 branch, pending Driver's re-approval (e.g. a= is older than 3 days). After, checking in, please add the fixed1.0 keyword.
Whiteboard: [need info][fixed-trunk] → [need info][fixed-trunk] [Needs refresh of a=]
Updated•22 years ago
|
Assignee | ||
Updated•22 years ago
|
Target Milestone: mozilla1.0 → mozilla1.0.1
Comment 41•22 years ago
|
||
changing to adt1.0.1+ for adt approval for checkin to the 1.01 milestone. Please get drivers approval before checking in.
Comment 42•22 years ago
|
||
Comment on attachment 79405 [details] [diff] [review] Same as attachment 74346 [details] [diff] [review], but with better variable name re-approval for 1.0.1 branch. please check into the 1.0.1 branch ASAP. once landed remove the mozilla1.0.1+ keyword and add the fixed1.0.1 keyword
Attachment #79405 -
Flags: superreview+
Attachment #79405 -
Flags: review+
Attachment #79405 -
Flags: approval+
Updated•22 years ago
|
Keywords: mozilla1.0 → mozilla1.0.1+
Assignee | ||
Updated•22 years ago
|
Keywords: mozilla1.0.1+ → fixed1.0.1
Assignee | ||
Comment 43•22 years ago
|
||
See bug 135932 which is a consequence of this bug.
Assignee | ||
Comment 44•22 years ago
|
||
*** Bug 150953 has been marked as a duplicate of this bug. ***
Comment 45•22 years ago
|
||
I realize I'm wandering into this discussion rather late, and it's good to know that this bug is being fixed. I think that this is a parsing issue more than anything--in this case, how to parse certain invalid URLs. <a href="http://www.w3.org/Addressing/rfc1808.txt">RFC 1808</a> has some good guidelines on how to parse URLs. Obviously, mozilla itself doesn't mistake a URL of the form "http://foo.com/bar?moby/quux" as referring to the path "/bar?moby", because then it wouldn't be parsing the query correctly. Whether or not the query is invalid or contains a '/' is really a separate issue--the path for the cookie is only supposed to concern the URL's path, not its query. Sure, the query is incorrectly formed, but that shouldn't be relevant to the path at all, any more than an incorrectly formed hostname or fragment would be. And if these URLs really are invalid, why doesn't Mozilla give me an error? My guess would be because its URL parsing works something like the method specified in RFC 1808, and therefore is rather forgiving in what it accepts. The problem here is that Mozilla silently sets the wrong path for the cookie; a warning or an error would have been much easier to diagnose. :) But it's great to hear that this has been fixed; I just wish it could have made it into Netscape 7.0PR1, since people are starting to use it now. I'll download a nightly build of Mozilla and test it again...
Comment 47•22 years ago
|
||
verified galeon 1.2.6 + Mozilla 1.1
You need to log in
before you can comment on or make changes to this bug.
Description
•