Open Bug 1691113 Opened 27 days ago Updated 21 days ago

Commented out block relating to ancient cookie Path requirements should be removed

Categories

(Core :: Networking: Cookies, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: miketaylr, Assigned: miketaylr)

References

Details

(Whiteboard: [necko-triaged])

Attachments

(1 obsolete file)

https://searchfox.org/mozilla-central/source/netwerk/cookie/CookieService.cpp#1791-1806 has the following commented out code:

#if 0
  } else {
    /**
     * The following test is part of the RFC2109 spec.  Loosely speaking, it says that a site
     * cannot set a cookie for a path that it is not on.  See bug 155083.  However this patch
     * broke several sites -- nordea (bug 155768) and citibank (bug 156725).  So this test has
     * been disabled, unless we can evangelize these sites.
     */
    // get path from aHostURI
    nsAutoCString pathFromURI;
    if (NS_FAILED(aHostURI->GetPathQueryRef(pathFromURI)) ||
        !StringBeginsWith(pathFromURI, aCookieData.path())) {
      return false;
    }
#endif

Chromium has a similar comment at https://source.chromium.org/chromium/chromium/src/+/master:net/cookies/canonical_cookie.cc;l=310-314;drc=c6259cb544738517e7f1e9400ac8ce9372f8c9a1

  // The RFC says the path should be a prefix of the current URL path.
  // However, Mozilla allows you to set any path for compatibility with
  // broken websites.  We unfortunately will mimic this behavior.  We try
  // to be generous and accept cookies with an invalid path attribute, and
  // default the path to something reasonable.

The relevant spec text at the time stated:

(From https://tools.ietf.org/html/rfc2109:)

4.3.2  Rejecting Cookies

   To prevent possible security or privacy violations, a user agent
   rejects a cookie (shall not store its information) if any of the
   following is true:

   * The value for the Path attribute is not a prefix of the request-
     URI.

But rfc6265 states:

the Path attribute does not provide any integrity protection because the
user agent will accept an arbitrary Path attribute in a Set-Cookie header.

So this block can be removed.

Assignee: nobody → miketaylr

Awkwardly, if I try to log into Phabricator with my Bugzilla account, it says my account has been disabled.

Comment on attachment 9201691 [details] [diff] [review]
0001-Bug-1691113-Remove-obsolete-and-commented-out-cookie.patch

Review of attachment 9201691 [details] [diff] [review]:
-----------------------------------------------------------------

r? :dragana

(I have no idea if code is even landable via splinter anymore)
Attachment #9201691 - Flags: review?(dd.mozilla)
Severity: -- → S4
Priority: -- → P3
Whiteboard: [necko-triaged]

(I have no idea if code is even landable via splinter anymore)

It isn't; I've re-enabled your Phabricator account, please resubmit there.

Attachment #9201691 - Attachment is obsolete: true
Attachment #9201691 - Flags: review?(dd.mozilla)

Thanks glob!

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