Closed
Bug 1415828
Opened 7 years ago
Closed 7 years ago
Marionette fails to add new cookie if cookie domain has a preceding dot character
Categories
(Remote Protocol :: Marionette, defect)
Remote Protocol
Marionette
Tracking
(firefox59 fixed)
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: majorpetya, Assigned: majorpetya)
References
(Blocks 1 open bug, )
Details
Attachments
(1 file)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36 Steps to reproduce: With a selenium client try to add a new cookie and make sure that the cookie domain has a preceding dot character. The simplest way to test this is to change testing/web-platform/tests/webdriver/tests/cookies/add_cookie.py to have a cookie domain of .web-platform.test Actual results: The cookie wasn't created and an InvalidCookieDomainError was returned Expected results: The cookie should have been created as a domain cookie.
Assignee | ||
Comment 1•7 years ago
|
||
FYI I'm happy to pick this up in the next few days.
Comment 2•7 years ago
|
||
Great to hear that, Peter. I'm already assigning the bug to you then. Thanks.
Assignee: nobody → majorpetya
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 3•7 years ago
|
||
Whereby if that is not clear by the spec yet (https://github.com/w3c/webdriver/issues/1143), then we might want to wait with this implementation.
Assignee | ||
Comment 4•7 years ago
|
||
Andreas, is the assumption correct that the cookie domain with a preceding dot character should work with Firefox or do you think that further clarification from w3c is needed before we can progress this issue?
Flags: needinfo?(ato)
Comment 5•7 years ago
|
||
(In reply to Peter Major from comment #4) > Andreas, is the assumption correct that the cookie domain with > a preceding dot character should work with Firefox or do you > think that further clarification from w3c is needed before we can > progress this issue? I’m not sure what you’re really asking here. Should a domain prefixed by a dot be possible to add as a cookie to Firefox’ cookie service? Or should Marionette/WebDriver deem that acceptable input?
Flags: needinfo?(ato)
Assignee | ||
Comment 6•7 years ago
|
||
Firefox's cookie service appears to work this way: * if the domain value is prefixed with a dot, the cookie is created as a domain cookie * if the domain value does not start with a dot, the cookie is created as a host only cookie So I think the question really is whether marionette/webdriver should accept domain with leading dot and handle it as a domain cookie?
Flags: needinfo?(ato)
Comment 7•7 years ago
|
||
Sorry for my late reply here. I have been on holiday. (In reply to Peter Major from comment #6) > So I think the question really is whether marionette/webdriver should accept > domain with leading dot and handle it as a domain cookie? Judging my Simon’s reply to https://github.com/w3c/webdriver/issues/1143, I think the answer to this is yes: we should treat a domain string with preceding dots as a domain cookie. It does also seem like the specification could use some tidying up make this clearer.
Flags: needinfo?(ato)
Comment hidden (mozreview-request) |
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8931398 [details] Bug 1415828 - Accept cookie domains with leading dot character https://reviewboard.mozilla.org/r/202526/#review207902 ::: testing/web-platform/tests/webdriver/tests/cookies/add_cookie.py:153 (Diff revision 1) > + clear_all_cookies(session) > + create_cookie_request = { > + "cookie": { > + "name": "hello", > + "value": "world", > + "domain": ".web-platform.test" We shouldn’t really hard code this domain name into the test. Can you extract this from the configuration["host"] fixture, please? I see I’ve accepted a few cases of this in the past, so it would be excellent if you could fix those too (-:
Attachment #8931398 -
Flags: review?(ato) → review-
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8931398 [details] Bug 1415828 - Accept cookie domains with leading dot character https://reviewboard.mozilla.org/r/202526/#review207902 > We shouldn’t really hard code this domain name into the test. Can > you extract this from the configuration["host"] fixture, please? > > I see I’ve accepted a few cases of this in the past, so it would > be excellent if you could fix those too (-: configuration["host"] returns 127.0.0.1 instead of web-platform.test, should I use something else instead?
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8931398 [details] Bug 1415828 - Accept cookie domains with leading dot character https://reviewboard.mozilla.org/r/202526/#review207902 > configuration["host"] returns 127.0.0.1 instead of web-platform.test, should I use something else instead? Used server_config["domains"][""] instead, wondering if I should have iterate through all the available domains instead
Comment 13•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8931398 [details] Bug 1415828 - Accept cookie domains with leading dot character https://reviewboard.mozilla.org/r/202526/#review207902 > Used server_config["domains"][""] instead, wondering if I should have iterate through all the available domains instead I think just picking one of them should be fine.
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8931398 [details] Bug 1415828 - Accept cookie domains with leading dot character https://reviewboard.mozilla.org/r/202526/#review209016 ::: testing/web-platform/tests/webdriver/tests/cookies/add_cookie.py:177 (Diff revision 2) > + assert "domain" in cookie > + assert isinstance(cookie["domain"], basestring) > + > + assert cookie["name"] == "hello" > + assert cookie["value"] == "world" > + assert cookie["domain"] == ".web-platform.test" You probably want to assert that it is the same as you set originally.
Attachment #8931398 -
Flags: review?(ato) → review-
Comment hidden (mozreview-request) |
Assignee | ||
Comment 16•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8931398 [details] Bug 1415828 - Accept cookie domains with leading dot character https://reviewboard.mozilla.org/r/202526/#review209016 > You probably want to assert that it is the same as you set originally. d'oh, sorry about that.
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8931398 [details] Bug 1415828 - Accept cookie domains with leading dot character https://reviewboard.mozilla.org/r/202526/#review209162
Attachment #8931398 -
Flags: review?(ato) → review+
Comment 18•7 years ago
|
||
Pushed by atolfsen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f5e24faf36fc Accept cookie domains with leading dot character r=ato
Comment 19•7 years ago
|
||
Thanks for the patch and also thanks for spending the time digging into this. I appreciate it is not fun to drill down into what the correct/expected behaviour is when all the sources are contradicting. But I believe users will be happy that this is finally fixed!
Comment 20•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f5e24faf36fc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•