1559707 - Firefox 66, 67 saves 302 in cache

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect, P3)

Description

[f.kosik]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0

Steps to reproduce:

Some sequence of 302 cause that Firefox use previously cached location from 302 instead of URL which is already available - therefore it should not be redirected. It happens even if "Cache-Control: no-cache, no-store, max-age=0, must-revalidate" is used in header for all 302.

Use case:

1. User cleans Firefox cache.
2. User enters URL for resource (PNG image) occuring on each page.
3. System (using CAS Apereo + Spring security) sends response with Status Code 302 and Location of CAS login page.
4. User enters other URL, e.g. home page. It causes that user is not redirected to resource required in 2nd step. No-caching works all right in Firefox when this step is skipped.
5. System sends response with Status Code 302 and Location of CAS login page (like in 3rd step).
6. User logs in.
7. System sends response with Status Code 302 and Location of CAS ticket. Then redirects to URL required in 4th step using Status Code 302 again.
8. System does not behave normally. Firefox uses cached files (including those from URLs required in 2nd and 4th steps) although it should not be cached

Actual results:

System does not behave normally. Firefox uses cached files (including those from URLs required in 2nd and 4th steps) although it should not be cached. E.g. It causes redirection to home page (URL from 4th step) instead of displaying required page (accessed from menu) as written in RFC: https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.3

Other browsers and older Firefox behaves normally and redirection does not happen. Firefox 66 and 67 behaves normally when its cache is cleared.

See screenshots from Firefox: https://stackoverflow.com/questions/56620774/firefox-66-67-saves-302-in-cache

Expected results:

Firefox should meet RFC: https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.3

Comment 1

[Adrian Florinescu [:aflorinescu]]

This quote from https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.3 seems confusing to me:

Note: RFC 1945 and RFC 2068 specify that the client is not allowed
to change the method on the redirected request. However, most
existing user agent implementations treat 302 as if it were a 303
response, performing a GET on the Location field-value regardless
of the original request method. The status codes 303 and 307 have
been added for servers that wish to make unambiguously clear which
kind of reaction is expected of the client.

Honza, can you please take a look over this report?

Comment 2

[Honza Bambas (:mayhemer)]

Please provide a log:
https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging

Thanks.

Comment 3

[f.kosik]

Attachment: Firefox networking logs

Comment 4

[Junior [inactive]]

2019-06-23 00:54:29.078585 UTC - [Parent 31802: Main Thread]: V/nsHttp nsHttpResponseHead::ParseCachedOriginalHeader [this=0x7f2e97503600]
2019-06-23 00:54:29.078607 UTC - [Parent 31802: Main Thread]: V/nsHttp nsHttpResponseHead::ParseCachedHead [this=0x7f2e97503600]
2019-06-23 00:54:29.078613 UTC - [Parent 31802: Main Thread]: V/nsHttp nsHttpResponseHead::ParseVersion [version=HTTP/1.1 302 
Location: http://casmcc/login?theme=mcc-standard-5.15&service=http%3A%2F%2Flocalhost%3A8090%2Fmcc-launcher%2Flogin%2Fcas
Content-Length: 0
Date: Sun, 23 Jun 2019 00:54:14 GMT
Server: Apache
˄˅]

Looks like we cache 302.

2019-06-23 00:54:19.334383 UTC - [Parent 31802: Main Thread]: D/nsHttp HttpBaseChannel::SetupReplacementChannel [this=0x7f2ea7655000 newChannel=0x7f2e9b2d2040 preserveMethod=1]
2019-06-23 00:54:19.334441 UTC - [Parent 31802: Main Thread]: D/nsHttp nsHttpChannel::GetNavigationStartTimeStamp 0x7f2ea7655000
2019-06-23 00:54:19.334459 UTC - [Parent 31802: Main Thread]: D/nsHttp nsHttpChannel::WaitForRedirectCallback [this=0x7f2ea7655000]
2019-06-23 00:54:19.385665 UTC - [Parent 31802: Main Thread]: D/nsHttp nsHttpChannel::OnRedirectVerifyCallback [this=0x7f2ea7655000] result=0 stack=2 mWaitingForRedirectCallback=1
2019-06-23 00:54:19.385705 UTC - [Parent 31802: Main Thread]: D/nsHttp nsHttpChannel::ContinueProcessRedirection [rv=0,this=0x7f2ea7655000]
2019-06-23 00:54:19.386012 UTC - [Parent 31802: Main Thread]: D/nsHttp nsHttpChannel::Cancel [this=0x7f2ea7655000 status=804b0003]
2019-06-23 00:54:19.386148 UTC - [Parent 31802: Main Thread]: D/nsHttp HttpBaseChannel::RemoveAsNonTailRequest this=0x7f2ea7655000, rc=0x7f2ea133c480, already added=1
2019-06-23 00:54:19.386304 UTC - [Parent 31802: Main Thread]: D/nsHttp nsHttpChannel::InitCacheEntry [this=0x7f2ea7655000 entry=0x7f2e9f131ca0]
2019-06-23 00:54:19.386445 UTC - [Parent 31802: Main Thread]: D/nsHttp nsHttpChannel::AddCacheEntryHeaders [this=0x7f2ea7655000] begin
2019-06-23 00:54:19.387011 UTC - [Parent 31802: Main Thread]: D/cache2 CacheEntry::MetaDataReady [this=0x7f2e992fa700, state=WRITING]

Comment 5

[Junior [inactive]]

(In reply to Adrian Florinescu [:adrian_sv] from comment #1)

This quote from https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.3 seems confusing to me:

Note: RFC 1945 and RFC 2068 specify that the client is not allowed
to change the method on the redirected request. However, most
existing user agent implementations treat 302 as if it were a 303
response, performing a GET on the Location field-value regardless
of the original request method. The status codes 303 and 307 have
been added for servers that wish to make unambiguously clear which
kind of reaction is expected of the client.

AFAIK this is for the method change like POST->GET

Comment 6

[Junior [inactive]]

Hello Anne,
[1] specifies for 302/307 "This response is only cacheable if indicated by a Cache-Control or Expires header field. "
Looks like [2] doesn't mention this. We might need another spec work. What do you think?

[1] https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.3
[2] https://fetch.spec.whatwg.org/

Comment 7

[Junior [inactive]]

ni? for comment 6.

Some analysis

• Firefox cache 302/307 if cc is not presented, Chrome doesn't cache.
  "This response is only cacheable if indicated by a Cache-Control or Expires header field."

• Firefox doesn't cache 303 at all, Chrome cache if a cc is presented with max-age>0.
  We're good here with "The 303 response MUST NOT be cached."

Comment 8

[Anne (:annevk)]

Junior, HTTP cache semantics are defined by https://tools.ietf.org/html/rfc7231#section-6.1 for various status codes and https://tools.ietf.org/html/rfc7234 for the HTTP cache in general. Fetch largely delegates to HTTP for this. I'd recommend against using RFC 2616.

(You might even want to check the latest HTTP drafts on this at https://httpwg.org/http-core/draft-ietf-httpbis-semantics-latest.html and https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html but I don't think they're different on this issue.)

Comment 9

[Junior [inactive]]

Attachment: Bug 1559707 - Not using heuristic expiration time for non-cacheable requests

(a) Parse Cache-Control:public. Intentionally not serializing in IPC::ParamTraits::Write
since HttpChannelChild hasn't used at this moment.
(b) Don't calculate heuristic expiration time for non-cacheable request (e.g., 302, 304, 307 without
CC:public/private)

Comment 10

[Junior [inactive]]

(In reply to Anne (:annevk) from comment #8)

Junior, HTTP cache semantics are defined by https://tools.ietf.org/html/rfc7231#section-6.1 for various status codes and https://tools.ietf.org/html/rfc7234 for the HTTP cache in general. Fetch largely delegates to HTTP for this. I'd recommend against using RFC 2616.

(You might even want to check the latest HTTP drafts on this at https://httpwg.org/http-core/draft-ietf-httpbis-semantics-latest.html and https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html but I don't think they're different on this issue.)

Thanks Anne. RFC 7234 and 7231 are clearer and not conflicted with RFC 2616 for 302/307.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62286ede9f12
Not using heuristic expiration time for non-cacheable requests r=mayhemer

Comment 12

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/62286ede9f12