Alt-Svc header parser doesn't accept empty ALPN

[ManickaP]

This is a root cause for failing outerloop test: Public_Interop_Upgrade_Request3OrLower_Success for https://www.litespeedtech.com/

The server sends this Alt-Svc header:

Alt-Svc: =":443"; ma=2592000, h3=":443"; ma=2592000, h3-29=":443"; ma=2592000, h3-Q050=":443"; ma=2592000, h3-Q046=":443"; ma=2592000, h3-Q043=":443"; ma=2592000, quic=":443"; ma=2592000; v="43,46"

Note the first item is missing ALPN. Our header parsing then discards the whole header. RFC says that empty ALPN is not allowed as it specifies it as token

Firefox has no issues with this and is able to process the next request via h3.

Question is whether we should allow this? We could just ignore the unparsable value or allow empty ALPN.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This is a root cause for failing outerloop test: Public_Interop_Upgrade_Request3OrLower_Success for https://www.litespeedtech.com/

The server sends this Alt-Svc header:

Alt-Svc: =":443"; ma=2592000, h3=":443"; ma=2592000, h3-29=":443"; ma=2592000, h3-Q050=":443"; ma=2592000, h3-Q046=":443"; ma=2592000, h3-Q043=":443"; ma=2592000, quic=":443"; ma=2592000; v="43,46"

Note the first item missing ALPN. Our header parsing then discards the whole header. RFC says that empty ALPN is not allowed as it specifies it as token

Firefox has no issues with this and is able to process the next request via h3.

Question is whether we should allow this? We could just ignore the unparsable value or allow empty ALPN.

Author:	ManickaP
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	-

[wfurt]

I would be in favor of ignoring fragments where practical instead of ignoring whole header. I agree that the example seems to violate RFC but it may be good for improving interior. Upgrading to H/3 should be optional so I don't see it is critical.
Perhaps low-priority ?

[MihaZupan]

Looking at the current Alt-Svc logic, there are other weird things going on:

• It's likely that the current logic for clear is unreachable
  • We are reading the list of values via HttpHeaders.TryGetValues here. This involves validation, so it goes through the AltSvcHeaderParser, which creates a list of AltSvcHeaderValues. Those values are then ToStringed and returned from TryGetValues. Then the loop feeds those strings back to AltSvcHeaderParser and looks at parsed values.
  • AltSvcHeaderValue.ToString is broken for clear and it will return clear=":0"; ma=0, which is an invalid value and won't roundtrip through the parser.
  • As an example, the following code throws on the second Add

    var headers = new HttpResponseMessage().Headers;
    headers.Add("Alt-Svc", "clear");
    string value = headers.GetValues("Alt-Svc").Single();
    headers.Clear();
    headers.Add("Alt-Svc", value); // The format of value 'clear=":0"; ma=0' is invalid

  • Similarly, the second call to the AltSvcHeaderParser will fail, and so that loop can never observe the clear value
• Even if the loop could see clear, we're still going to use the first authority we saw, even though clear should have been the only value present. (We break here, but maybe we should be returning?)
• We're comparing authorities against each other in a few places with == and !=, but the HttpAuthority class doesn't override these operators, so this is doing reference equality. This is suspicious at best and potentially an error in some places.
• GetHttp3ConnectionAsync could end up returning null here in a race, leading to a NRE here.

[ManickaP]

Triage: our behavior follows RFC, the impact is minimal as we only do not upgrade to h3 which is on its own voluntary, moving to future.

[ManickaP]

Looking at the current Alt-Svc logic, there are other weird things going on:

I've extracted your notes to a separate issue. They might be tackled together, but that's not necessary.