1457503 - Remove <meta http-equiv=set-cookie> support

Status: RESOLVED FIXED

(Core :: DOM: Security, enhancement, P2)

Description

[Anne (:annevk)]

See cookies/meta-blocked.html in web-platform-tests and https://github.com/whatwg/html/pull/3649

Chrome has succeeded with this.

Comment 1

[Jonathan Kingston [:jkt] he/him]

Seems ok yeah, will see what try says with removing this.

Comment 2

[Jonathan Kingston [:jkt] he/him]

Attachment: Bug 1457503 - Removal of http-equiv cookies.

Review commit: https://reviewboard.mozilla.org/r/240514/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/240514/

Comment 3

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8971743 [details]
Bug 1457503 - Removal of http-equiv cookies.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240514/diff/1-2/

Comment 4

[Jonathan Kingston [:jkt] he/him]

Hey :dveditz,

Are there any blockers on publishing an intent to remove with this default disabled via a preference?

The patch is small and we could allow for enterprise to keep this enabled perhaps. I'm happy also to do a phased rollout too or work on our own telemetry too.

Chrome removed this in 65 which was the previous stable release.

Thanks

Comment 5

[Anne (:annevk)]

Per https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/17255938/ it seems Edge has removed this too.

Comment 6

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8971743 [details]
Bug 1457503 - Removal of http-equiv cookies.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240514/diff/2-3/

Comment 7

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8971743 [details]
Bug 1457503 - Removal of http-equiv cookies.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240514/diff/3-4/

Comment 8

[Daniel Veditz [:dveditz]]

jkt: sounds good, do it!

This missed ESR-60 and hopefully by the next ESR the world will have adjusted to stop using these, but the pref is still a good idea in the short term so we can easily recover if things go unexpectedly badly on release.

Comment 9

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8971743 [details]
Bug 1457503 - Removal of http-equiv cookies.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240514/diff/4-5/

Comment 10

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8971743 [details]
Bug 1457503 - Removal of http-equiv cookies.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240514/diff/5-6/

Comment 11

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8971743 [details]
Bug 1457503 - Removal of http-equiv cookies.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240514/diff/6-7/

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8971743 [details]
Bug 1457503 - Removal of http-equiv cookies.

https://reviewboard.mozilla.org/r/240514/#review249044

::: dom/base/nsContentSink.cpp:193
(Diff revision 7)
>    Preferences::AddIntVarCache(&sInitialPerfTime,
>                                "content.sink.initial_perf_time", 2000000);
>    Preferences::AddIntVarCache(&sEnablePerfMode,
>                                "content.sink.enable_perf_mode", 0);
> +
> +  Preferences::AddBoolVarCache(&sDisableMetaCookie,

I think you may want a StaticPrefList.h entry here instead of this member and call, depending on the response to bug 1436655 comment 42.

::: dom/base/nsContentSink.cpp:843
(Diff revision 7)
>        return NS_OK;
>      }
>  
>      // Don't allow setting cookies in <meta http-equiv> in cookie averse
>      // documents.
> -    if (nsGkAtoms::setcookie->Equals(header) && mDocument->IsCookieAverse()) {
> +    if (nsGkAtoms::setcookie->Equals(header) && mDocument->IsCookieAverse() && !sDisableMetaCookie) {

This check doesn't make sense to me.  Yes, we'll skip doing most of the work in ProcessHeaderData, but we'll still call mDocument->SetHeaderData().  Is that really what we want?

If it is, then shouldn't the cookie-averse check also just happen in ProcessHeaderData after mDocument->SetHeaderData()?

::: testing/web-platform/meta/cookies/meta-blocked.html.ini:2
(Diff revision 7)
>  [meta-blocked.html]
> -  [Cookie is not set from `<meta>`.]
> +  prefs: [content.cookie.meta.disabled:false]

This doesn't make sense to me.  If the pref is forced to false as here, wouldn't the test fail?  Shouldn't you just remove this .ini file altogether?

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 8971743 [details]
Bug 1457503 - Removal of http-equiv cookies.

https://reviewboard.mozilla.org/r/240514/#review251200

I assume we're getting another patch so clearing review.

Comment 14

[Valentin Gosu [:valentin] (he/him)]

Has this stalled? This was an issue in bug 1487085. It would be nice if we removed it.

Comment 15

[Andrea Marchesini [:baku]]

Attachment: Bug 1457503 - Removal of http-equiv cookies. draft, r?jkt

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af207309bcf5
Removal of http-equiv cookies. draft, r=jkt

Comment 17

[Oana Pop-Rus]

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

Comment 18

[Kohei Yoshino]

Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2019/setting-cookies-with-meta-http-equiv-is-no-longer-allowed/

Comment 19

[Eric Shepherd [:sheppy]]

Content updates already done; I have submitted BCD PR 4341 to list set-cookie as being gone in Firefox 68.