Skip to content

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

@ManickaP

Description

@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)

Metadata

Metadata

Assignees

Labels

area-System.Net.Httpbugin-prThere is an active PR which will close this issue when it is merged

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions