1569715 - CORS preflight requests are cached when 'Disable cache' is checked.

Status: VERIFIED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

[David Boles]

Attachment: Screenshot from 2019-07-29 11-33-19.png

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

Steps to reproduce:

Close and reopen Firefox.
Open the network developer tools and check 'Disable cache'
Trigger a CORS request that will be preflighted and usually cached (Access-Control-Max-Age set in the response) twice.

Actual results:

The first request shows a preceding OPTIONS preflight in the network tools, the second does not.

Expected results:

Both requests should have triggered preflights as caching was disabled.

Comment 1

[David Sacal]

Hi,
i'm not able to reproduce the reported behavior. Would you please attach a minimal test case to trigger the request you've mentioned? In the meantime i will assign a component to make one of our devs have a look. Feel free to change it if you consider it wrong

Regards
David

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@David, we need a reproducible test case to make progress on this bug. Can you please provide one?
Otherwise we'll close the report as invalid (non reproducible) :-(

Honza

Comment 3

[David Boles]

Sorry, totally missed the first replies. The behavior I observed is still happening (on a different computer) if I follow the above instructions using this page (e.g. edit the first pi := 3.14159 text box to trigger a request). I'm pretty busy now and no longer have access to that backend's source code to be able to quickly strip it down, but if there's a specific format of test case you want I can try to make one.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to David Boles from comment #3)

Sorry, totally missed the first replies. The behavior I observed is still happening (on a different computer) if I follow the above instructions using this page (e.g. edit the first pi := 3.14159 text box to trigger a request). I'm pretty busy now and no longer have access to that backend's source code to be able to quickly strip it down,

I am still unsure how to see the issue.

The first request shows a preceding OPTIONS preflight in the network tools, the second does not.

I am not seeing a request with OPTIONS method in the Net panel.

but if there's a specific format of test case you want I can try to make one.

There is no specific format. We just need a simple page (online or attached to this bug) that can be used to see the problem. You can use PHP or NodeJS, if that helps.

Honza

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I could finally repro this issue.

STR:

1. Load https://www.openpolicyagent.org/docs/latest/policy-language/
2. Open DevTools and select the Network panel
3. Check Disable Cache
4. Modify the first input box in The Basics section (default value is: pi := 3.14159)
5. Observe two requests in the Network panel
   • OPTIONS preflight
   • POST
6. Edit again, observe just one request
   • POST
     The preflight request is missing, seems like it's cached, which is wrong since the cache is disabled -> BUG

Honza

Comment 6

[:Harald Kirschner :digitarald]

(Clearing ni? as we have an STR)

P2, as this makes it harder to debug and fix CORS rules efficiently.

Comment 7

[:Harald Kirschner :digitarald]

Nhi, given how important CORS is for a secure web, not having a proper Disable Cache support in DevTools is biting developers. Who do you think could help on the Necko side to unblock this and at least give some guidance on scope this fix entails?

Comment 8

[Nhi Nguyen (:nhi)]

Michal, could you outline the steps needed to fix this? Thanks!

Comment 9

[Michal Novotny [:michal]]

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #5)

I could finally repro this issue.

STR:

1. Load https://www.openpolicyagent.org/docs/latest/policy-language/
2. Open DevTools and select the Network panel
3. Check Disable Cache

What exactly this check box should mean? Should Firefox behave like HTTP cache was disabled (the hover text of the checkbox says so)? Preflight responses are not cached in HTTP cache, so if HTTP cache is disabled preflight responses are still returned from sPreflightCache (https://searchfox.org/mozilla-central/rev/7fd1c1c34923ece7ad8c822bee062dd0491d64dc/netwerk/protocol/http/nsCORSListenerProxy.cpp#181). If you really want to skip the cache, we should check the load flags of aRequestChannel in nsCORSListenerProxy::StartCORSPreflight() and don't lookup the entry if nsIRequest::LOAD_BYPASS_CACHE was specified.

Comment 10

[:Harald Kirschner :digitarald]

What exactly this check box should mean?

It should disable any network-related caching in Firefox for this tab (DNS, CORS preflight). This is especially important for developers as cached HTTP, CORS, DNS, etc. information is can be subject to iterative changes in a development environment and caching gets in the way.

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Just clearing NI since Harald already replied.

Comment 12

[Kershaw Chang [:kershaw]]

Michal, could you take a look?
Thanks.

Comment 13

[:Harald Kirschner :digitarald]

Honza, could you clarify if we have all we need for the Devtools side, or would this depend on the Necko team to implement/fix?

Comment 14

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Summary:

1. Valid test case is in comment #5 and I can easily use it to reproduce this issue

2. The Disable Cache checkbox should disabled any network-related caching in Firefox for this tab (DNS, CORS preflight) as stated in comment #10

3. This needs to be fixed on the platform as stated in comment #9

Preflight responses are not cached in HTTP cache, so if HTTP cache is disabled preflight responses are still returned from sPreflightCache (https://searchfox.org/mozilla-central/rev/7fd1c1c34923ece7ad8c822bee062dd0491d64dc/netwerk/protocol/http/nsCORSListenerProxy.cpp#181). If you really want to skip the cache, we should check the load flags of aRequestChannel in nsCORSListenerProxy::StartCORSPreflight() and don't lookup the entry if nsIRequest::LOAD_BYPASS_CACHE was specified.

4. There is also bug 1376253 (with a test case available) that reports missing preflight request. Worth to include it in the investigation (to see whether it's related).

5. There is also bug 1528603 (reported in Networking) component that is very related. It's about viewing and clearing CORS cache. It also contains a few more links to the platform code base.

Nhi, supporting Firefox security features (including CORS) is increasingly important. Could someone please look into this bug?

Thanks,
Jan

Comment 15

[Michal Novotny [:michal]]

Attachment: Bug 1569715 - Don't lookup CORS preflight cache when request bypasses HTTP cache, r=mayhemer

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29d1be2b96f9
Don't lookup CORS preflight cache when request bypasses HTTP cache, r=mayhemer,necko-reviewers,valentin

Comment 17

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/29d1be2b96f9

Comment 18

[Kershaw Chang [:kershaw]]

It seems we need another way to bypass preflight cache for devtools.
If I read the code correctly, it seems we simple change the pref devtools.cache.disabled when Disable Cache is toggled. Maybe we can simply check if devtools.cache.disabled is true instead checking load flags at here.

Honza, what do you think?

Comment 19

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

Attachment: Bug 1569715 - Don't lookup CORS preflight cache when devtools 'Disable cache' is checked r=Honza

Comment 20

[Kershaw Chang [:kershaw]]

Clear ni to Honza, since he is the reviewer.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1754415059f5
Don't lookup CORS preflight cache when devtools 'Disable cache' is checked r=necko-reviewers,kershaw

Comment 22

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1754415059f5

Comment 23

[Oana Botisan, Desktop QA]

I managed to reproduce the issue using an older version of Nightly on Windows 10 x64. I retested everything on macOS 10.13, Ubuntu 18.04 and Windows 10 x64 using Firefox 84.0 and Nightly 85.0a1. The issue is not reproducing anymore.