Closed Bug 1545421 Opened 8 months ago Closed 6 months ago

Propagate useful proxy errors to WebExtensions

Categories

(Core :: Networking: HTTP, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox68 --- fixed
firefox69 --- fixed

People

(Reporter: dragana, Assigned: mayhemer)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged][secure-proxy-mvp])

Attachments

(1 file)

No description provided.
Priority: -- → P2
Whiteboard: [necko-triaged]
Summary: Propagate proxy auth error to WebExtensions. → Propagate proxy auth error to WebExtensions

We want to trigger proxy.onError

Blocks: secure-proxy
Priority: P2 → P1
Summary: Propagate proxy auth error to WebExtensions → Propagate useful proxy errors to WebExtensions
Assignee: nobody → honzab.moz
See Also: → 637619
See Also: → 493699

Turning to defect, because we really lack here.

My proposal:

  • this bug will only help to distinguish errors of CONNECT requests
  • I intend to add new NS_ERROR_PROXY_* error codes for some of the common scenarios like 407 (to work around bug 1554218), 502, 504 and some other codes we may find relevant
  • the response codes will convert to error codes somewhere in the h2 tunneling code and be propagated to the connection object to end up on the channel as the error status
  • appropriate error pages for top level loads has to be implemented; this means localization is needed

Test from bug 1554190 has several TODOs that will be updated in this bug.

Plain connections problem:

  • When we are connecting to a plain server through an h2 proxy, we don't use CONNECT, because from the privacy point of view it's just one round trip added and most proxies will fail to connect port 80 this way (it's usually forbidden). But, this brings one major problem: how do we recognize what went wrong? 502/504/5XX are general HTTP response codes that the end server may return well enough as the proxy that simply can't reach the server (from legit reasons.) It is not possible/reliable to recognize if our configured forward h2 proxy can't reach the end server server or a remote reverse proxy can't reach it's back-end server. If our proxy can't reach out, we have to fail the load and show an error page; if the response comes from the end server (rev proxy) we /should/ show the response as the content.

For consideration:

  • tl;dr: special MIME type for proxies to describe their errors
  • long version: there is a proposal (not adopted and normalized) for application/proxy-explanation+json content type that proxies may return (with an appropriate content) to let clients safely and reliably recognize the error code is coming from the proxy and not from the server. (If the server misuse this mime type, it would only lead to blocking of the content, on our side it would only convert to an internal failure code. It could only disrupt our telemetry, when widely abused - unlikely.)
Status: NEW → ASSIGNED
Type: task → defect
Duplicate of this bug: 1554190
Duplicate of this bug: 1554673

As I was afraid, our test harness doesn't provide newer (8+) nodejs. I filed Update nodejs for testharness to at least 8.4 to support newer http2 APIs · Issue #2150 · mozilla/release-services in tooltool to change that.

I will update the patch to comment out the proxy code from moz-http2.js and disable the h2 proxy test and land it like that. That test can still be run locally if you have nodejs 8.4+.

Pushed by honzab.moz@firemni.cz:
https://hg.mozilla.org/integration/autoland/rev/684c934ad7c5
New nsresult error codes for 407, 502 and 504 http response codes returned by proxies + test, r=dragana
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Comment on attachment 9067995 [details]
Bug 1545421 - New nsresult error codes for 407, 502 and 504 http response codes returned by proxies + test, r=dragana!

Beta/Release Uplift Approval Request

  • User impact if declined: Needed for the secure proxy project
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Just propagates few error codes we didn't propagate before, doesn't change/have impact on the internal connection logic.
  • String changes made/needed: none
Attachment #9067995 - Flags: approval-mozilla-beta?

Comment on attachment 9067995 [details]
Bug 1545421 - New nsresult error codes for 407, 502 and 504 http response codes returned by proxies + test, r=dragana!

this doesn't seem appropriate for the last week of beta

Attachment #9067995 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

Julien, this is critical for the secure proxy project that is planned for 68. This patch MUST be there.

Flags: needinfo?(jcristau)

Why did the uplift request not come earlier then? Are you and others in the necko team going to be available in the next two weeks if there are issues?

Flags: needinfo?(jcristau)

(In reply to Julien Cristau [:jcristau] from comment #12)

Why did the uplift request not come earlier then?

Because I was unclear on dates, perhaps.

Are you and others in the necko team going to be available in the next two weeks if there are issues?

We definitely will be.

Flags: needinfo?(jcristau)

Comment on attachment 9067995 [details]
Bug 1545421 - New nsresult error codes for 407, 502 and 504 http response codes returned by proxies + test, r=dragana!

approved for 68.0b14

Flags: needinfo?(jcristau)
Attachment #9067995 - Flags: approval-mozilla-beta- → approval-mozilla-beta+

Could this cause visible changes depending on the user's proxy configuration? Anything that's worth testing before going to release?

Flags: needinfo?(honzab.moz)

(In reply to Julien Cristau [:jcristau] from comment #15)

Could this cause visible changes depending on the user's proxy configuration? Anything that's worth testing before going to release?

This will only effect error pages that we show when a proxy fails to connect an end server - actually will make them more accurate. Instead of a general "proxy connect error" we may show more proper "server connect error" instead.

Flags: needinfo?(honzab.moz)
Whiteboard: [necko-triaged] → [necko-triaged][secure-proxy-mvp]
You need to log in before you can comment on or make changes to this bug.