1976197 - Reject invalid cookies in the webExt cookie API - nightly only

Status: RESOLVED FIXED

(WebExtensions :: General, task)

Description

[Andrea Marchesini [:baku]]

The webExt.cookies API is currently the only entry point where we allow invalid cookies, as nsICookieValidation is not enforced.
This bug aims to enable cookie validation in Nightly only, to monitor the level of breakage it may cause.
If the level of breakage is acceptable, in a few cycles, we will enable the validation in all the channels.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: Bug 1976197 - Reject invalid cookies in the webExt cookie API - nightly only, r?robwu

Comment 2

[Rob Wu [:robwu]]

The validation landed in 141, off-by-default in https://hg-edge.mozilla.org/mozilla-central/rev/aa54edb433a0
back then the use of invalid cookie values resulted in the following message being logged in the console:

Extension [extension id] tried to create an invalid cookie: [error message]

With the patch here, this validation becomes enabled by default, and browser.cookies.set() will reject when an invalid value is encountered.

Ideally, we should only enable this by default if all cookies that an extension can observe through the cookies API are also valid. Bug 1974987 is still open, which suggests that it is possible for the cookies DB to return invalid cookies. Are there other unaccounted cases that still need a migration?

Comment 3

[Andrea Marchesini [:baku]]

Ideally, we should only enable this by default if all cookies that an extension can observe through the cookies API are also valid. Bug 1974987 is still open, which suggests that it is possible for the cookies DB to return invalid cookies. Are there other unaccounted cases that still need a migration?

Telemetry says that the majority of cookie errors from DB are:

• eRejectedNoneRequiresSecure: 1.21% - Migration done (bug 1973651)
• eRejectedAttributeExpiryOversize: 0.00% - Migration in review (Bug 1974987)
• eRejectedEmptyNameAndValue: 0.00% - Migration in review (Bug 1974987)
• eRejectedInvalidCharValue: 0.00% - Migration in review (Bug 1974987)
• eRejectedInvalidCharName: 0.00% - Migration in review (Bug 1974987)
• eRejectedNameValueOversize: 0.00% - not included. Too low.

Query: https://sql.telemetry.mozilla.org/queries/109167/source

As soon as we land bug 1974987, we will be ready to proceed.

Comment 4

[Rob Wu [:robwu]]

(moving to WebExtensions::General since this change only affects extension API behavior)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/488b54fd9df7
https://hg.mozilla.org/integration/autoland/rev/8e83b3296c86
Reject invalid cookies in the webExt cookie API - nightly only, r=robwu,cookie-reviewers,edgul

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6cb2ee6b7a4b
https://hg.mozilla.org/integration/autoland/rev/34b03e551372
Revert "Bug 1976197 - Reject invalid cookies in the webExt cookie API - nightly only, r=robwu,cookie-reviewers,edgul" for causing mochitest failures @ test_ext_cookies.html

Comment 7

[amarc]

Backed out for causing mochitest failures @ test_ext_cookies.html

• Backout link
• Push with failures
• Failure Log

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/82d6f931d58c
https://hg.mozilla.org/integration/autoland/rev/a0dcff3b0556
Reject invalid cookies in the webExt cookie API - nightly only, r=robwu,cookie-reviewers,edgul

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/a0dcff3b0556

Comment 10

[rbloor]

Release note added in Bug 1976197 Reject invalid cookies in the webExt cookie API in nightly only #40351