Closed
Bug 826798
Opened 11 years ago
Closed 11 years ago
Network inspector does not correctly render cookie
Categories
(DevTools :: Console, defect, P2)
DevTools
Console
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 21
People
(Reporter: st3fan, Assigned: msucan)
Details
Attachments
(1 file, 1 obsolete file)
7.40 KB,
patch
|
akeybl
:
approval-mozilla-aurora-
|
Details | Diff | Splinter Review |
The stackoverflow.com site sets the following cookie: Name: usr Value: t=KxxxxqBWXEmn&s=n7IxxxwwU2A This is rendered in the Inspect Network Request panel as: usr=t It probably incorrectly splits on the '=' somewhere.
Assignee | ||
Updated•11 years ago
|
OS: Mac OS X → All
Priority: -- → P2
Hardware: x86 → All
Assignee | ||
Comment 1•11 years ago
|
||
Surprisingly I had this bug fix for parsing Set-Cookie, but I forgot to fix it for Cookie. Fixed it now and I added a test. Please review. Thanks! We might also want to land this in aurora.
Comment 2•11 years ago
|
||
Comment on attachment 698752 [details] [diff] [review] proposed fix Review of attachment 698752 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. parseSetCookieHeader could also use the |equal| local variable. ::: toolkit/devtools/webconsole/test/test_network_longstring.html @@ +178,5 @@ > > checkHeadersOrCookies(aResponse.cookies, { > foobar: "fooval", > omgfoo: "bug768096", > }); Shouldn't you add a 3rd cookie here?
Attachment #698752 -
Flags: review?(past) → review+
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Panos Astithas [:past] from comment #2) > Comment on attachment 698752 [details] [diff] [review] > proposed fix > > Review of attachment 698752 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks good. parseSetCookieHeader could also use the |equal| local variable. Sure. > ::: toolkit/devtools/webconsole/test/test_network_longstring.html > @@ +178,5 @@ > > > > checkHeadersOrCookies(aResponse.cookies, { > > foobar: "fooval", > > omgfoo: "bug768096", > > }); > > Shouldn't you add a 3rd cookie here? I didn't bother adding the 3rd cookies to all tests - as long as there's at least one test does what it needs to do. Will fix. Thank you for the quick review!
Assignee | ||
Comment 4•11 years ago
|
||
Attachment #698752 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Whiteboard: [land-in-fx-team]
Comment 5•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/d9a8cd7317d8
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d9a8cd7317d8
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 21
Assignee | ||
Comment 7•11 years ago
|
||
Comment on attachment 699403 [details] [diff] [review] updated patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 673148 User impact if declined: the web console network panel will continue to show incorrect values for some cookies that contain unescaped '='. Testing completed (on m-c, etc.): landed in fx-team and m-c. Risk to taking this patch (and alternatives if risky): close to no risks. alternatives: not really. String or UUID changes made by this patch: no such changes. Thank you!
Attachment #699403 -
Flags: approval-mozilla-aurora?
Comment 8•11 years ago
|
||
Comment on attachment 699403 [details] [diff] [review] updated patch The minimal user impact here means this change can ride the trains.
Attachment #699403 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•