Closed Bug 1457503 Opened 6 years ago Closed 4 years ago

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


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




Tracking Status
firefox68 --- fixed


(Reporter: annevk, Assigned: jkt)



(Keywords: dev-doc-complete, site-compat, Whiteboard: [domsecurity-active])


(2 files)

See cookies/meta-blocked.html in web-platform-tests and

Chrome has succeeded with this.
Seems ok yeah, will see what try says with removing this.
Assignee: nobody → jkt
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.

Flags: needinfo?(dveditz)
Priority: -- → P2
Whiteboard: [domsecurity-active]
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.
Flags: needinfo?(dveditz)
Attachment #8971743 - Flags: review?(dveditz)
Attachment #8971743 - Flags: review?(bzbarsky)
Keywords: site-compat
Comment on attachment 8971743 [details]
Bug 1457503 - Removal of http-equiv cookies.

::: 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?
Attachment #8971743 - Flags: review?(bzbarsky)
Comment on attachment 8971743 [details]
Bug 1457503 - Removal of http-equiv cookies.

I assume we're getting another patch so clearing review.
Attachment #8971743 - Flags: review?(dveditz)
Has this stalled? This was an issue in bug 1487085. It would be nice if we removed it.
Pushed by
Removal of http-equiv cookies. draft, r=jkt
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Regressions: 1550050

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

You need to log in before you can comment on or make changes to this bug.