Skip to content

Commit d6cd577

Browse files
ManickaPMihaZupan
ManickaP
and
MihaZupan
authored
[H/3] Clean up and fix Alt-Svc handling (#114528)
* Alt-Svc allow empty ALPN * Fix Alt-Svc handling of clear and authority equality. * Test alt-svc clear * Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/AltSvcHeaderValue.cs Co-authored-by: Miha Zupan <[email protected]> --------- Co-authored-by: Miha Zupan <[email protected]>
1 parent 4daefae commit d6cd577

File tree

8 files changed

+106
-27
lines changed

8 files changed

+106
-27
lines changed

‎src/libraries/System.Net.Http/src/System/Net/Http/Headers/AltSvcHeaderParser.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ protected override int GetParsedValueLength(string value, int startIndex, object
4343

4444
idx += alpnProtocolNameLength;
4545

46-
if (alpnProtocolName == "clear")
46+
if (alpnProtocolName == AltSvcHeaderValue.ClearString)
4747
{
4848
if (idx != value.Length)
4949
{
@@ -190,9 +190,9 @@ private static bool TryReadPercentEncodedAlpnProtocolName(string value, int star
190190

191191
if (tokenLength == 0)
192192
{
193-
result = null;
193+
result = "";
194194
readLength = 0;
195-
return false;
195+
return true;
196196
}
197197

198198
ReadOnlySpan<char> span = value.AsSpan(startIndex, tokenLength);

‎src/libraries/System.Net.Http/src/System/Net/Http/Headers/AltSvcHeaderValue.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ namespace System.Net.Http.Headers
1313
/// </remarks>
1414
internal sealed class AltSvcHeaderValue
1515
{
16-
public static AltSvcHeaderValue Clear { get; } = new AltSvcHeaderValue("clear", host: null, port: 0, maxAge: TimeSpan.Zero, persist: false);
16+
public const string ClearString = "clear";
17+
18+
public static AltSvcHeaderValue Clear { get; } = new AltSvcHeaderValue(ClearString, host: null, port: 0, maxAge: TimeSpan.Zero, persist: false);
1719

1820
public AltSvcHeaderValue(string alpnProtocolName, string? host, int port, TimeSpan maxAge, bool persist)
1921
{
@@ -50,6 +52,11 @@ public AltSvcHeaderValue(string alpnProtocolName, string? host, int port, TimeSp
5052

5153
public override string ToString()
5254
{
55+
if (ReferenceEquals(Clear, this))
56+
{
57+
return ClearString;
58+
}
59+
5360
var sb = new ValueStringBuilder(stackalloc char[256]);
5461

5562
sb.Append(AlpnProtocolName);

‎src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs‎

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -663,43 +663,54 @@ internal void HandleAltSvc(IEnumerable<string> altSvcHeaderValues, TimeSpan? res
663663

664664
if (AltSvcHeaderParser.Parser.TryParseValue(altSvcHeaderValue, null, ref parseIdx, out object? parsedValue))
665665
{
666-
var value = (AltSvcHeaderValue?)parsedValue;
666+
Debug.Assert(parsedValue is not null);
667+
668+
var value = (AltSvcHeaderValue)parsedValue;
667669

668670
// 'clear' should be the only value present.
669-
if (value == AltSvcHeaderValue.Clear)
671+
if (ReferenceEquals(AltSvcHeaderValue.Clear, value))
670672
{
671673
lock (SyncObj)
672674
{
675+
// Clear invalidates all Alt-Svc including the current response ones.
676+
// https://httpwg.org/specs/rfc7838.html#alt-svc
673677
ExpireAltSvcAuthority();
674678
Debug.Assert(_authorityExpireTimer != null || _disposed);
675679
_authorityExpireTimer?.Change(Timeout.Infinite, Timeout.Infinite);
676-
break;
680+
return;
677681
}
678682
}
679683

680-
if (nextAuthority == null && value != null && value.AlpnProtocolName == "h3")
684+
// Do not process the Alt-Svc header if we've already found a valid h3 authority before, but continue looking for potential "clear".
685+
if (nextAuthority is not null || value.AlpnProtocolName != "h3")
681686
{
682-
var authority = new HttpAuthority(value.Host ?? _originAuthority.IdnHost, value.Port);
683-
if (IsAltSvcBlocked(authority, out _))
684-
{
685-
// Skip authorities in our blocklist.
686-
continue;
687-
}
687+
continue;
688+
}
689+
690+
var authority = new HttpAuthority(value.Host ?? _originAuthority.IdnHost, value.Port);
691+
if (IsAltSvcBlocked(authority, out _))
692+
{
693+
// Skip authorities in our blocklist.
694+
continue;
695+
}
688696

689-
TimeSpan authorityMaxAge = value.MaxAge;
697+
TimeSpan authorityMaxAge = value.MaxAge;
690698

691-
if (responseAge != null)
692-
{
693-
authorityMaxAge -= responseAge.GetValueOrDefault();
694-
}
699+
if (responseAge != null)
700+
{
701+
authorityMaxAge -= responseAge.GetValueOrDefault();
702+
}
695703

696-
if (authorityMaxAge > TimeSpan.Zero)
697-
{
698-
nextAuthority = authority;
699-
nextAuthorityMaxAge = authorityMaxAge;
700-
nextAuthorityPersist = value.Persist;
701-
}
704+
// It's already out of date, skip it.
705+
if (authorityMaxAge <= TimeSpan.Zero)
706+
{
707+
continue;
702708
}
709+
710+
// We found an h3 authority that is not blocked and has not aged out.
711+
nextAuthority = authority;
712+
nextAuthorityMaxAge = authorityMaxAge;
713+
nextAuthorityPersist = value.Persist;
703714
}
704715
}
705716

‎src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpAuthority.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,12 @@ public override string ToString()
5959
{
6060
return IdnHost != null ? $"{IdnHost}:{Port}" : "<empty>";
6161
}
62+
63+
public static bool operator ==(HttpAuthority? left, HttpAuthority? right)
64+
{
65+
return left is null ? right is null : left.Equals(right);
66+
}
67+
public static bool operator !=(HttpAuthority? left, HttpAuthority? right)
68+
=> !(left == right);
6269
}
6370
}

‎src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.AltSvc.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,40 @@ public async Task AltSvc_ConnectionFrame_UpgradeFrom20_Success()
9696
await AltSvc_Upgrade_Success(firstServer, secondServer, client);
9797
}
9898

99+
[Fact]
100+
public async Task AltSvc_UpgradeThenClear_Success()
101+
{
102+
using Http2LoopbackServer firstServer = Http2LoopbackServer.CreateServer();
103+
using Http3LoopbackServer secondServer = CreateHttp3LoopbackServer();
104+
using HttpClient client = CreateHttpClient(HttpVersion.Version20);
105+
106+
Task<HttpResponseMessage> responseTask = client.GetAsync(firstServer.Address);
107+
Task<HttpRequestData> serverTask = firstServer.HandleRequestAsync(headers: new[]
108+
{
109+
new HttpHeaderData("Alt-Svc", $"h3=\"{secondServer.Address.IdnHost}:{secondServer.Address.Port}\"")
110+
});
111+
await new Task[] { responseTask, serverTask }.WhenAllOrAnyFailed(30_000);
112+
using HttpResponseMessage response1 = responseTask.Result;
113+
Assert.True(response1.IsSuccessStatusCode);
114+
115+
responseTask = client.GetAsync(firstServer.Address);
116+
serverTask = secondServer.HandleRequestAsync(headers: new[]
117+
{
118+
new HttpHeaderData("Alt-Svc", $"clear")
119+
});
120+
await new Task[] { responseTask, serverTask }.WhenAllOrAnyFailed(30_000);
121+
using HttpResponseMessage response2 = responseTask.Result;
122+
string altUsed = serverTask.Result.GetSingleHeaderValue("Alt-Used");
123+
Assert.Equal($"{secondServer.Address.IdnHost}:{secondServer.Address.Port}", altUsed);
124+
Assert.True(response2.IsSuccessStatusCode);
125+
126+
responseTask = client.GetAsync(firstServer.Address);
127+
serverTask = firstServer.HandleRequestAsync();
128+
await new Task[] { responseTask, serverTask }.WhenAllOrAnyFailed(30_000);
129+
using HttpResponseMessage response3 = responseTask.Result;
130+
Assert.True(response3.IsSuccessStatusCode);
131+
}
132+
99133
[Fact]
100134
public async Task AltSvc_ResponseFrame_UpgradeFrom20_Success()
101135
{

‎src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1896,8 +1896,7 @@ public static TheoryData<HttpStatusCode, bool> StatusCodesTestData()
18961896
public static TheoryData<string> InteropUris() =>
18971897
new TheoryData<string>
18981898
{
1899-
// [ActiveIssue("https://github.com/dotnet/runtime/issues/83775")]
1900-
//{ "https://www.litespeedtech.com/" }, // LiteSpeed
1899+
{ "https://www.litespeedtech.com/" }, // LiteSpeed
19011900
{ "https://quic.tech:8443/" }, // Cloudflare
19021901
{ "https://quic.aiortc.org:443/" }, // aioquic
19031902
{ "https://h2o.examp1e.net/" } // h2o/quicly

‎src/libraries/System.Net.Http/tests/UnitTests/Headers/AltSvcHeaderParserTest.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,17 @@ public static IEnumerable<object[]> SuccessfulParseData()
9898
}
9999
};
100100

101+
yield return new object[]
102+
{
103+
"=\":443\"; ma=2592000, h3=\":443\"; ma=2592000, h3-29=\":443\"; ma=2592000, quic=\":443\"; ma=2592000; v=\"43,46\"", new[]
104+
{
105+
new AltSvcHeaderValue("", host: null, port: 443, TimeSpan.FromTicks(TimeSpan.TicksPerSecond * 2592000), persist: false),
106+
new AltSvcHeaderValue("h3", host: null, port: 443, TimeSpan.FromTicks(TimeSpan.TicksPerSecond * 2592000), persist: false),
107+
new AltSvcHeaderValue("h3-29", host: null, port: 443, TimeSpan.FromTicks(TimeSpan.TicksPerSecond * 2592000), persist: false),
108+
new AltSvcHeaderValue("quic", host: null, port: 443, TimeSpan.FromTicks(TimeSpan.TicksPerSecond * 2592000), persist: false),
109+
}
110+
};
111+
101112
// "clear".
102113
yield return new object[]
103114
{

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,16 @@ public void AcceptLanguage_UseAddMethodWithInvalidValue_InvalidValueRecognized()
267267
Assert.Equal("en; q=0.4,[", headers.GetValues("Accept-Language").First());
268268
}
269269

270+
[Fact]
271+
public void AltSvc_Clear_RoundTrip_Success()
272+
{
273+
headers.Add("Alt-Svc", "clear");
274+
string value = headers.GetValues("Alt-Svc").Single();
275+
Assert.Equal("clear", value);
276+
headers.Clear();
277+
headers.Add("Alt-Svc", value);
278+
}
279+
270280
[Fact]
271281
public void Expect_Add100Continue_Success()
272282
{

0 commit comments

Comments
 (0)