Skip to content

Throw "invalid cookie domain" as its definition implies#1955

Open
yezhizhen wants to merge 1 commit intow3c:masterfrom
yezhizhen:update-cookie
Open

Throw "invalid cookie domain" as its definition implies#1955
yezhizhen wants to merge 1 commit intow3c:masterfrom
yezhizhen:update-cookie

Conversation

@yezhizhen
Copy link
Copy Markdown
Member

@yezhizhen yezhizhen commented Mar 26, 2026

See servo/servo#43690 (comment)

The definition of InvalidCookieDomain is: An illegal attempt was made to set a cookie under a different domain than the current page.

But somehow, the spec asks to throw "invalid argument" error for this case.
However, all browsers indeed throw "invalid cookie domain" for this case.

Following test aligns with current spec, yet fails for all browsers.

def test_add_cookie_with_domain_not_matching_current_domain(session, url):
    session.url = url("/common/blank.html")

    new_cookie = {
        "name": "hello",
        "value": "world",
        "domain": "example.com",
        "path": "/",
        "httpOnly": False,
        "secure": False
    }

    response = add_cookie(session, new_cookie)
    assert_error(response, "invalid argument")

This change is Reviewable


Preview | Diff

Signed-off-by: Euclid Ye <euclid.ye@huawei.com>
@yezhizhen
Copy link
Copy Markdown
Member Author

cc @whimboo @xiaochengh

@yezhizhen yezhizhen changed the title Throw "invalid cookie domain" as its name suggests Throw "invalid cookie domain" as its definition implies Mar 26, 2026
github-merge-queue bot pushed a commit to servo/servo that referenced this pull request Mar 26, 2026
Improve the error reporting for [cookie
addition](https://w3c.github.io/webdriver/#add-cookie).
Most notably, when
- expiry time exceeds maximum safe integer
- cookie domain is not equal to session's current browsing context's
active document's domain

There is a bug with spec:
#43690 (comment).
We fix spec in w3c/webdriver#1955 and
make sure Servo behaves consistently with other browsers.

Testing: Added one test for invalid expiry time. New passing with
existing.

---------

Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@whimboo
Copy link
Copy Markdown
Contributor

whimboo commented Mar 26, 2026

This makes sense. Thanks for discovering that discrepancy. The referenced tests can be found here but I have to add that we only test unsupported schemes for invalid cookie domain. We should indeed at least one more test with a valid domain. @yezhizhen mind adding one respectively? Thanks!

@yezhizhen
Copy link
Copy Markdown
Member Author

Adding one more in web-platform-tests/wpt#58801

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants