1823877 - Network error on fetch() with partially implemented ORB

Status: VERIFIED FIXED

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

Description

[B.J. Herbison]

On Nightly before bug 1800658 landed fetch("https://google.com/some", {mode: "no-cors"}) doesn't generate a network error. After the land a network error is generated.

https://developer.mozilla.org/en-US/docs/Web/API/fetch
"A fetch() promise does not reject on HTTP errors (404, etc.)."

Steps to reproduce:

• Open the console and run this command: fetch("https://google.com/some", {mode: "no-cors"})

Expected:

• No network error.

Actual:

• Network error.

Based on a comment in https://chat.mozilla.org/#/room/#firefox:mozilla.org :
https://matrix.to/#/!OjiTSQTpPWGpfDenKT:mozilla.org/$hyjOR9HFAEhAsaeJeB_VE5Ymwi6_enF_vTLqXDrMybs?via=mozilla.org&via=matrix.org&via=privacytools.io

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1800658

:sefeng, since you are the author of the regressor, bug 1800658, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 3

[Sean Feng [:sefeng211]]

Yeah, It raised error because you made an no-cors request that was blocked by https://github.com/annevk/orb.

So this behaviour is expected. One can open the browser console to see which requests were blocked and reason of blockage.

This is currently enabled in Early Beta, so it was not mentioned in the release note. I'll make it sure to mention it when it goes further.

We are still evaluating the web compatibility of ORB, so if there is something that used to work but not working anymore, please let me know.

Comment 4

[Sean Feng [:sefeng211]]

I figured this should be marked as WONTFIX. Please feel free to reopen it.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

I'd like to reopen this. We don't know if the current behavior is web compatible.
Chrome doesn't have this behavior.
Right farre?

Comment 6

[Andreas Farre [:farre]]

I now think we need to prepare for this at least. Thanks for filing the spec issue.

I think the wisest approach is to add a check in HttpBaseChannel::PerformOpaqueResponseSafelistCheckBeforeSniff to see if we're doing a Window.fetch (and I guess Cache.add), and behind a pref add the behavior to allow the response, but filter it. This way we get a slightly less restrictive ORB, but no data that would become filtered will ever reach the content child process. This also turns out to actually be better for all the cases that ORB allows, but that would become filtered anyway for Window.fetch

Comment 7

[Andreas Farre [:farre]]

Attachment: Bug 1823877 - Part 1: Filter opaque results from fetch() in the parent for ORB. r=sefeng!,smaug!

We make sure to not send any data to the content process in case of
fetching an opaque resource.

Comment 8

[Intermittent Failures Robot]

1 failures in 4161 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 1

Platform and build breakdown:

• fetch: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1823877&startday=2023-03-20&endday=2023-03-26&tree=all

Comment 9

[Andreas Farre [:farre]]

Attachment: Bug 1823877 - Part 2: Adjust tests to account for parent filtered responses. r=sefeng!,smaug!

Depends on D173454

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1800658

Comment 11

[Andreas Farre [:farre]]

Turns out this didn't really depend on 1825373, it got through ServiceWorkerGlobalScope.fetch just fine. It was that the AudioContext assumes that it would always get a MIME type.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

For posterity, ORB is now enabled through early beta by way of bug 1821682.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/680c502179c6
Part 1: Filter opaque results from fetch() in the parent for ORB. r=sefeng,smaug,necko-reviewers,edenchuang,valentin
https://hg.mozilla.org/integration/autoland/rev/c1a4692a05e3
Part 2: Adjust tests to account for parent filtered responses. r=sefeng,smaug

Comment 14

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/680c502179c6
https://hg.mozilla.org/mozilla-central/rev/c1a4692a05e3

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:farre, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox114 to wontfix.

For more information, please visit BugBot documentation.

Comment 16

[Andreas Farre [:farre]]

This is behind the early beta flag, so uplifting it to beta isn't necessary to prevent it from reaching release. We want to let this patch ride the trains in the normal tempo.

Comment 17

[Kagami Rosylight [:saschanaz] (they/them)]

wontfix then!

Comment 18

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Kagami [:saschanaz] from comment #17)

wontfix then!

Ha, actually it's disabled as atm we don't plan to let ORB ride to release 114. Sorry for the confusing status!
I make it disabled for firefox114 status so it's aligned with how we set status for previous versions, too. :)

Comment 19

[Kagami Rosylight [:saschanaz] (they/them)]

Oops, of course 👍

Comment 20

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2023-05-07 Nightly build on macOS 12.
Verified as fixed on Firefox 115.0b4(build ID: 20230611180300) and Nightly 116.0a1(build ID: 20230611214645) on macOS 12, Windows 10, Ubuntu 22.