Closed Bug 1636118 Opened 4 years ago Closed 4 years ago

3 /mixed-content/gen/*/link-prefetch-tag.https.html tests are expected timeout

Categories

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

Product:
Core
Component:
DOM: Security
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla78
Tracking Flags:
Tracking Status
firefox78 --- fixed

People

(Reporter: jmaher, Assigned: jgraham)

References

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 file)

Bug 1636118 - Use agressive prefetch in mixed-content tests,
47 bytes, text/x-phabricator-request
Details | Review

I am looking into tests that are timeout and trying to get them fixed. I noticed that we have these 3 tests failing:
/mixed-content/gen/top.http-rp/opt-in/link-prefetch-tag.https.html
/mixed-content/gen/top.meta/opt-in/link-prefetch-tag.https.html
/mixed-content/gen/top.meta/unset/link-prefetch-tag.https.html

and they fail on the first test:
Mixed-Content: Expects allowed for link-prefetch-tag to same-https origin and keep-scheme redirection from https context.

I reproduced this locally on windows 10 and didn't see any additional information in the console.

:ckerschb, here are 3 tests that seem to be related, could you take a look at these or redirect to someone who can? There are many other tests in these directories, so I assume there is something specific in these tests that isn't supported or a pref we need, or an issue with the test itself.

Flags: needinfo?(ckerschb)

We are doing a lot of refactoring in MCB right now, let me dig into what's going on here. Keeping my ni? until then.

I am clueless at this point. I only see 2 requests happening after the test is fired up:

There is nothing in the console and nothing related to the mixed content blocker.

The only thing I see is

  • WARNING: NS_ENSURE_TRUE(mRequest) failed: file /home/ckerschb/source/mc/netwerk/base/nsBaseChannel.cpp, line 919
    which links to here

I can't tell if that is related.

I'll keep digging.

James, have you encountered any harness issues related to link-prefetch-tag that could potentially bring some insights to this problem here?

Flags: needinfo?(ckerschb) → needinfo?(james)

So, adding some logging in https://searchfox.org/mozilla-central/source/testing/web-platform/tests/common/security-features/resources/common.sub.js#207 I see handlers being added to the following objects with the given event name:

<link rel="prefetch" href="https://web-platform.tes…c3&path=%2Fmixed-content"> load 
<link rel="prefetch" href="https://web-platform.tes…c3&path=%2Fmixed-content"> error

But I don't see resolve or reject are ever called. So I think that for some reason in the initial load we don't do the prefetch at all and so don't end up firing these events. Per the spec that seems to be allowed [1]. But it would be nice if we could find a way to make this test actually work (on reload it seems to pass).

Oh and while looking for someone to needinfo I discovered the network.prefetch-next.aggressive pref, which seems to make this test pass.

[1] https://w3c.github.io/resource-hints/#load-and-error-events

Flags: needinfo?(james)

I am slightly confused. Honza can you help me out understanding prefs. Last time we chatted we figured that the pref network.prefetch-next is set to true here which should enable prefetch, right?

I have not seen where we can set network.prefetch-next.aggressive as per comment 5. If that pref makes the test pass then I guess we can obviously flip it but I also wanted to understand what the difference is here.

Flags: needinfo?(honzab.moz)

This makes the tests which expect link rel=prefetch to always occur pass

Assignee: nobody → james
Status: NEW → ASSIGNED

Yeah, it's definitely also possible that the prefetch behaviour is a bug. I posted a patch with smaug as a reviewer since he seemed to have reviewed a lot of recent prefetch changes, but if Honza or someone wants to steal that's obviously OK.

Priority: -- → P2
Whiteboard: [domsecurity-active]
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/f405e817df72
Use agressive prefetch in mixed-content tests, r=smaug

https://hg.mozilla.org/mozilla-central/rev/f405e817df72

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox78: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/3f7932b4c557
Use agressive prefetch in mixed-content tests, r=smaug

network.prefetch-next.aggressive described here:

  // In usual case prefetch does not start until all normal loads are done.
  // Aggressive mode ignores normal loads and just start prefetch ASAP.
  // It's mainly for testing purpose and discoraged for normal use;
  // see https://bugzilla.mozilla.org/show_bug.cgi?id=1281415 for details.

This comment could be related too: https://bugzilla.mozilla.org/show_bug.cgi?id=1281415#c3

To sum, the tests are simply doing something (an infinite load or some race condition) that will never allow a prefetch to start. Flipping this pref is a workaround. If that is also the right fix is a question to look at the tests more closely.

Flags: needinfo?(honzab.moz)

Note that aiui these tests are mostly checking security properties applied to all load types; they aren't specifically written to test prefetch semantics. So if the pref just means that the load happens, and doesn't change the interaction with mixed content blocking, I don't think it will adversely affect the correctness.

Flags: needinfo?(james)

https://hg.mozilla.org/mozilla-central/rev/3f7932b4c557

Status: REOPENED → RESOLVED
Closed: 4 years ago → 4 years ago
status-firefox78: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: