-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cronvoy: Negotiated Protocol #1546
Comments
Question for @carloseltuerto / @DavidSchinazi / @RyanTheOptimist if no protocol is negotiated (ALPN not attempted) is this still filled in with the protocol used? |
Based on my reading of the code, if ALPN is not use then the negotiated protocol will be unknown.
|
The description of the getNegotiatedProtocol() states that the returned String is empty when using plain https or http. I browsed chromium for tests, and here are the strings I got (maybe some are bogus):
https://source.chromium.org/search?q=getNegotiatedProtocol%20f:test.java&sq=&ss=chromium I guess more efforts are needed to figure out the exhaustive list of possible values. Maybe someone with Cronet experience would know where to look. |
Yes those sound like correct ALPN strings. I would not think that you would need an exhaustive list of values. Instead, I would hope that you could ask envoy for the ALPN string and it would give the correct value? |
Maybe - I'm not 100% sure if Envoy would return http if it fails over to HTTP/1.1, or if it'd return an empty string. I'll make sure to test and loop back if the default is different than Cronets. |
If Envoy Mobile returns "HTTP/1.1", Cronvoy could convert it to an empty String. I guess where that hacking occurs can be discussed, if this is happening. |
The official list of ALPN protocol IDs is here (note that you want to check the Identification Sequence column, not the Protocol column). If Envoy is hardcoded to use a given protocol instead of negotiating it via ALPN, I'd suggest having Envoy return the ALPN protocol ID for the configured protocol (such as Some additional notes:
|
I'd be inclined to look at the code. If when hard-coded to H2 Envoy does ALPN and fails if it doesn't negotiate h2, I'd return H2. In the unlikely event it doesn't negotiate when not configured I'd rather reflect reality. |
Part of envoyproxy/envoy-mobile#1546 Risk Level: Low Testing: new integration test Docs Changes: n/a Release Notes: n/a Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@goaway WDYT about making an explicit accessor for this vs sticking alpn in stream intel and doing string copies on every up call? |
-> charles for header - > cronet API (though the header will be blank if we're not using the ALPN cluster or doing HTTP/3) |
@carloseltuerto could you please close this issue. It's implemented via #2164 |
In order to be able to close this Issue, some more Chromium::Cronet Tests need to be ported. Here are some: https://chromium.googlesource.com/chromium/src/+/HEAD/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamQuicTest.java These tests are also validating the "Negotiated Protocol". |
As part of the Cronet API, the Cornet Engine exposes the "Negotiated Protocol".
/**
* Returns the protocol (for example 'quic/1+spdy/3') negotiated with the server.
* Returns an empty string if no protocol was negotiated, the protocol is
* not known, or when using plain HTTP or HTTPS.
*/
public String getNegotiatedProtocol()
This somehow will need to be resurfaced by the Envoy Mobile Engine. I guess that adding a
x-envoy-negotiated-protocol
response header would suffice.The text was updated successfully, but these errors were encountered: