Skip to content

Conversation

@ManickaP
Copy link
Member

Fixes #83775

  • This change is not strictly necessary, it's making us being more lenient by accepting Alt-Svc with empty ALPN (RFC is more strict) and ignoring it.

Fixes #83874

  • Fixed ToString for "Alt-Svc: clear".
  • Fixed handling of "clear" - clears out Alt-Svc including anything from the current response.
  • Fixed HttpAuthority comparison by overloading == operator (class is internal, not public).
  • The last point about NRE is irrelevant since the H/3 connection pooling was refactored to support multiple connections.

@ManickaP ManickaP requested review from a team and Copilot April 11, 2025 09:24
@ghost ghost added the area-System.Net.Http label Apr 11, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpRequestHeadersTest.cs:277

  • After re-adding the 'Alt-Svc' header with the value 'clear', consider adding an assertion to verify that the header was processed as expected, ensuring the roundtrip behavior is fully validated.
headers.Add("Alt-Svc", value);

Debug.Assert(_authorityExpireTimer != null || _disposed);
_authorityExpireTimer?.Change(Timeout.Infinite, Timeout.Infinite);
break;
return;
Copy link

Copilot AI Apr 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a clarifying comment above the 'return' statement to document that returning immediately upon handling a 'clear' Alt-Svc header is an intentional control flow decision to prevent further header processing.

Copilot uses AI. Check for mistakes.
Copy link
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

…vcHeaderValue.cs

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
@ManickaP ManickaP merged commit d6cd577 into dotnet:main Apr 14, 2025
85 of 87 checks passed
@ManickaP ManickaP deleted the h3-alt-svc branch April 14, 2025 13:02
@github-actions github-actions bot locked and limited conversation to collaborators May 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[HTTP/3] Alt-Svc logic has many issues Alt-Svc header parser doesn't accept empty ALPN

2 participants