Closed Bug 1407675 Opened 6 years ago Closed 6 years ago

Unable to add cookie with an IP address IP


(Remote Protocol :: Marionette, defect)

Not set


(firefox-esr52 unaffected, firefox56 unaffected, firefox57 fixed, firefox58 fixed)

Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed
firefox58 --- fixed


(Reporter: lucast1533, Assigned: majorpetya)



(Keywords: regression)


(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36

Steps to reproduce:

Attempt to add a cookie using an IP address for domain. More specifically, a cookie dictionary of:
{'path': '/', 'domain': '', 'name': 'foo', 'value': 'bar', 'secure': False}

Deleting the 'domain' key works as intended and using a regular domain name seems to work fine.

This was discovered in the selenium python tests, starting on 04/10/2017. I believe it is due to the fix for

Link to travis tests where it first started failing:

Actual results:

Error is returned

"Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsICookieManager2.add]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: chrome://marionette/content/cookie.js :: cookie.add :: line 135"  data: no]

Trying to set the cookie with the port added results in:

Cookies may only be set for the current domain (

As expected.

Expected results:

Should be able to add a cookie for the IP address as the domain.
The actual bug Lukas is talking about here and which regressed this is bug 1402978.

Peter, could you have a look at it please?
Blocks: 1402978
Component: Untriaged → Marionette
Flags: needinfo?(majorpetya)
Keywords: regression
Product: Firefox → Testing
Ever confirmed: true
Hmm, not quite sure how to fix this just yet, I'll take a more closer look..
This happens because an "." is prefixed to restrictToHost in [1]:

>   if (restrictToHost) {
>     if (!restrictToHost.endsWith(newCookie.domain) &&
>         ("." + restrictToHost) !== newCookie.domain) {
>       throw new InvalidCookieDomainError(
>           `Cookies may only be set ` +
>           `for the current domain (${restrictToHost})`);
>     }
>   }

restrictToHost is new URL(GeckoDriver#currentURL).hostname from
GeckoDriver#addCookie [2].

OS: Unspecified → All
Hardware: Unspecified → All
It seems a prerequisite here to first determine if the provided
domain is an IP or a TLD before checking if the cookie can be set.
We might be able to make use of the nsIEffectiveTLDService for this.

There appears to be code in nsCookieService::CanSetCookie [1] that
is useful for understanding how to implement the correct check.

I had a look at this today and fired up a review for a potential fix.
Flags: needinfo?(majorpetya)
Fantastic. Thank you a lot! Andreas or myself will have a look at the patch later.
Assignee: nobody → majorpetya
Just a head’s up before I look at the patch: I believe you have
commit access level 1 now?  This allows you to trigger try runs
on Treeherder.  You find a dialogue for doing this directly from
MozReview under ‘Automation’ → ‘Trigger a Try Build’.

A good try syntax for Marionette/geckodriver is:

> try: -b do -p linux64,macosx64,win64,android-api-16 -u marionette-e10s,marionette-headless-e10s,xpcshell,web-platform-tests,firefox-ui-functional-local-e10s,firefox-ui-functional-remote-e10s -t none

It’s generally a good idea to trigger a try run for your own build
because it reduces the turnaround time on getting patches through

I triggered the try run for you now.
Comment on attachment 8917855 [details]
Bug 1407675 - Fix cookie creation via Marionette using IP address

Thanks for addressing the regression, this looks good to me.
Attachment #8917855 - Flags: review?(ato) → review+
Pushed by
Fix cookie creation via Marionette using IP address r=ato
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Whiteboard: [checkin-needed-beta]
Flags: in-testsuite+
Whiteboard: [checkin-needed-beta]
Attachment #8917855 - Flags: review?(hskupin)
Peter, thanks a lot for fixing this regression!
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.