[HTTP/3] Alt-Svc logic has many issues

[ManickaP]

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.

Originally posted by @MihaZupan in #83775 (comment)

[ghost]

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

Issue Details

---

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.

Originally posted by @MihaZupan in #83775 (comment)

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

[ManickaP]

Triage: we've lived this until now, the only thing that seems more urgent is the potential for race with h3 connection. Putting to future for now.