1982916 - Fix test_cors_preflight_dns_cache.js to not rely on using `triggeringPrincipal` to make its requests be treated as cross-origin

Status: NEW

(Core :: Networking, task, P3)

Description

[Daniel Holbert [:dholbert]]

This test...
https://searchfox.org/mozilla-central/rev/68a6b1a19cc76683c8c96430f091ba7218728bb4/netwerk/test/unit/test_cors_preflight_dns_cache.js
...uses a manually-modified triggeringPrincipal to explicitly cause some network requests to be treated as cross-origin, in order to generate a CORS preflight.

Specifically, the test is setting triggeringPrincipal in order to make us fail the mRequestingPrincipal->CheckMayLoad calls here (and skip the return NS_OK early-return), so that we'll reach the further-down preflight code:
https://searchfox.org/mozilla-central/rev/ace4c0612207f29c0d0530dec82e15aeb5fe17d5/netwerk/protocol/http/nsCORSListenerProxy.cpp#1156-1161,1183-1185

if (!mHasBeenCrossSite &&
    NS_SUCCEEDED(mRequestingPrincipal->CheckMayLoad(uri, false)) &&
    (originalURI == uri ||
     NS_SUCCEEDED(mRequestingPrincipal->CheckMayLoad(originalURI, false)))) {
  return NS_OK;
}
...
// Check if we need to do a preflight, and if so set one up. This must be
// called once we know that the request is going, or has gone, cross-origin.
rv = CheckPreflightNeeded(aChannel, aUpdateType, aStripAuthHeader);

Right now, in that code, mRequestingPrincipal is the custom triggeringPrincipal (which is different from uri), so we fail the check and we make it to CheckPreflightNeeded.

But as of bug 1970490's landing, we won't be using the triggeringPrincipal to do CORS checks by default anymore, which means this hack in the test won't work anymore -- we'll pass the mRequestingPrincipal in the code quoted above will now exactly match uri (in at least some of the test's cases) and so we'll just allow the load and won't make it to CheckPreflightNeeded, which means we won't be testing the situation that the test is trying to exercise (and we fail the test because it's expecting some preflights to be cached, etc).

For now I'll keep the test passing by toggling content.cors.use_triggering_principal to true just for that test, to restore the old behavior; but ultimately we should modify the test to avoid relying on triggeringPrincipal to convert same-origin tests to be CORS tests, so that it's not reliant on that pref. Filing this bug to do that.

Comment 1

[Daniel Holbert [:dholbert]]

(kershaw, would you mind looking into this when you get a chance?)

Comment 2

[Sunil Mayya]

let me take a stab at it.

Comment 3

[Daniel Holbert [:dholbert]]

Thank you! You should be able to repro by removing this prefs line:
https://searchfox.org/firefox-main/rev/a353242aeafd56b2c21a2c0672ecb51ea1b81142/netwerk/test/unit/xpcshell.toml#593,598

["test_cors_preflight_dns_cache.js"]
...
prefs = ["content.cors.use_triggering_principal=true"] # See bug 1982916.

Assuming you're able to remove the dependency on that pref, we can get rid of the pref entirely (it only exists for the benefit of keeping this test passing, in its current form).