We keep Http2Session::MaybeReTunnel in a circle DoS'ing the proxy. I need to confirm this happens in an otherwise tuned up environment.

Confirming. When I remove web-id trust from the proxy's CA (or remove exception, if there were any) we shot at the proxy with high cadence. The moment I reestablish the trust, all works as expected. Tested only with my test proxy. I can see we are creating sessions with the proxy - proxy.on('session') is triggered on the proxy, but I don't know the nodejs h2 impl that well to know in what phase it fires this event.

Anyway, easily reproducible.

This is bug in detect portal! Investigating.

The problem is here:
nsHttpChannel.cpp - mozsearch

When the error is a sec error (nss) we immediately retry, creating a nice loop. Introduced in 816866 - Certificate errors frequently caused by captive portals should trigger captive portal detection 3 years ago!

Valentin, you reviewed that patch, do you remember how this has to work? This way it's definitely pretty much broken...

Oh, that's interesting indeed.
What's supposed to happen is that if we encounter a cert error, as it is common in some captive portals, we immediately recheck if there's a captive portal. However, I never considered what would happen if it failed for a http2 proxy :)

I guess what we might want to do here is to put some rate limiting on RecheckCaptivePortal - if we last checked in the last 3 seconds (pref?) maybe we don't recheck at all. We already have mLastChecked that we can use for this.

What do you think?

Ah! If some website has a cert error, we check if there is a captive portal suddenly. OK. There should definitely be rate limiting inside the service ;) But, seriously - if the channel is a plain request, then any cert error must come from a proxy, right? I think we could just disable this check when the channel's uri schema is "http" or when the channel is a detect-portal check (I wonder if that would ever become https uri?)

(In reply to Honza Bambas (:mayhemer) from comment #5)

I think we could just disable this check when the channel's uri schema is "http" or when the channel is a detect-portal check

I think that's a good fix too. Note: I don't know if we have a good way of detecting if the channel is a detect-portal check

(I wonder if that would ever become https uri?)

Users can technically set the URL to be a https one. I assume it rarely happens, as it kinda defeats the purpose. Some captive portals simply time out or block HTTPS connections, so it would completely fail to detect those networks.

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

Might be good to uplift.