Skip to content

Commit bd38c11

Browse files
ladeakladeak
committed
Changes based on the review feedback
1 parent 38de60a commit bd38c11

File tree

5 files changed

+78
-114
lines changed

5 files changed

+78
-114
lines changed

‎src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,14 @@ protected override void OnReset()
121121
{
122122
_keepAlive = true;
123123
_connectionAborted = false;
124+
_userTrailers = null;
124125

125126
// Reset Http2 Features
126127
_currentIHttpMinRequestBodyDataRateFeature = this;
127128
_currentIHttp2StreamIdFeature = this;
128129
_currentIHttpResponseTrailersFeature = this;
129130
_currentIHttpResetFeature = this;
130131
_currentIPersistentStateFeature = this;
131-
_userTrailers = null;
132132
}
133133

134134
protected override void OnRequestProcessingEnded()

‎src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,12 +726,12 @@ protected override void OnReset()
726726
{
727727
_keepAlive = true;
728728
_connectionAborted = false;
729+
_userTrailers = null;
729730

730731
// Reset Http3 Features
731732
_currentIHttpMinRequestBodyDataRateFeature = this;
732733
_currentIHttpResponseTrailersFeature = this;
733734
_currentIHttpResetFeature = this;
734-
_userTrailers = null;
735735
}
736736

737737
protected override void ApplicationAbort() => ApplicationAbort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByApplication), Http3ErrorCode.InternalError);

‎src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,17 @@ internal async ValueTask<Memory<byte>> ExpectDataAsync()
687687
return http3WithPayload.Payload;
688688
}
689689

690+
internal async ValueTask<Dictionary<string, string>> ExpectTrailersAsync()
691+
{
692+
var http3WithPayload = await ReceiveFrameAsync(false, true);
693+
Http3InMemory.AssertFrameType(http3WithPayload.Type, Http3FrameType.Headers);
694+
695+
_headerHandler.DecodedHeaders.Clear();
696+
_headerHandler.QpackDecoder.Decode(http3WithPayload.PayloadSequence, this);
697+
_headerHandler.QpackDecoder.Reset();
698+
return _headerHandler.DecodedHeaders.ToDictionary(kvp => kvp.Key, kvp => kvp.Value, _headerHandler.DecodedHeaders.Comparer);
699+
}
700+
690701
internal async Task ExpectReceiveEndOfStream()
691702
{
692703
var result = await ReadApplicationInputAsync();

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs

Lines changed: 43 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -252,23 +252,23 @@ public async Task ResponseTrailers_MultipleStreams_Reset()
252252
};
253253

254254
var requestCount = 0;
255+
IHeaderDictionary trailersFirst = null;
256+
IHeaderDictionary trailersLast = null;
255257
await InitializeConnectionAsync(context =>
256258
{
257259
requestCount++;
258260

259261
var trailersFeature = context.Features.Get<IHttpResponseTrailersFeature>();
260-
261-
IHeaderDictionary trailers;
262262
if (requestCount == 1)
263263
{
264-
trailers = new ResponseTrailersWrapper(trailersFeature.Trailers);
265-
trailersFeature.Trailers = trailers;
264+
trailersFirst = new ResponseTrailersWrapper(trailersFeature.Trailers);
265+
trailersFeature.Trailers = trailersFirst;
266266
}
267267
else
268268
{
269-
trailers = trailersFeature.Trailers;
269+
trailersLast = trailersFeature.Trailers;
270270
}
271-
trailers["trailer-" + requestCount] = "true";
271+
trailersFeature.Trailers["trailer-" + requestCount] = "true";
272272
return Task.CompletedTask;
273273
});
274274

@@ -291,109 +291,55 @@ await ExpectAsync(Http2FrameType.HEADERS,
291291

292292
_decodedHeaders.Clear();
293293

294-
// Ping will trigger the stream to be returned to the pool so we can assert it
295-
await SendPingAsync(Http2PingFrameFlags.NONE);
296-
await ExpectAsync(Http2FrameType.PING,
297-
withLength: 8,
298-
withFlags: (byte)Http2PingFrameFlags.ACK,
299-
withStreamId: 0);
300-
await SendPingAsync(Http2PingFrameFlags.NONE);
301-
await ExpectAsync(Http2FrameType.PING,
302-
withLength: 8,
303-
withFlags: (byte)Http2PingFrameFlags.ACK,
304-
withStreamId: 0);
305-
306-
// Stream has been returned to the pool
307-
Assert.Equal(1, _connection.StreamPool.Count);
308-
309-
await StartStreamAsync(3, requestHeaders, endStream: true);
310-
311-
await ExpectAsync(Http2FrameType.HEADERS,
312-
withLength: 6,
313-
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS),
314-
withStreamId: 3);
315-
316-
trailersFrame = await ExpectAsync(Http2FrameType.HEADERS,
317-
withLength: 16,
318-
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
319-
withStreamId: 3);
320-
321-
_hpackDecoder.Decode(trailersFrame.PayloadSequence, endHeaders: true, handler: this);
322-
323-
Assert.Single(_decodedHeaders);
324-
Assert.Equal("true", _decodedHeaders["trailer-2"]);
325-
326-
_decodedHeaders.Clear();
327-
328-
await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false);
329-
}
330-
331-
[Fact]
332-
public async Task CustomResponseTrailersCollection_MultipleStreams_Reset()
333-
{
334-
IEnumerable<KeyValuePair<string, string>> requestHeaders = new[]
294+
for (int i = 1; i < 3; i++)
335295
{
336-
new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
337-
new KeyValuePair<string, string>(HeaderNames.Path, "/hello"),
338-
new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
339-
new KeyValuePair<string, string>(HeaderNames.Authority, "localhost:80"),
340-
new KeyValuePair<string, string>(HeaderNames.ContentType, "application/json")
341-
};
296+
int streamId = i * 2 + 1;
297+
// Ping will trigger the stream to be returned to the pool so we can assert it
298+
await PingAsync();
342299

343-
IHeaderDictionary trailersFirst = null;
344-
IHeaderDictionary trailersSecond = null;
345-
var requestCount = 0;
346-
await InitializeConnectionAsync(context =>
347-
{
348-
requestCount++;
300+
// Stream has been returned to the pool
301+
Assert.Equal(1, _connection.StreamPool.Count);
349302

350-
var trailersFeature = context.Features.Get<IHttpResponseTrailersFeature>();
351-
if (requestCount == 1)
352-
{
353-
trailersFirst = new ResponseTrailersWrapper(trailersFeature.Trailers);
354-
trailersFeature.Trailers = trailersFirst;
355-
}
356-
else
357-
{
358-
trailersSecond = trailersFeature.Trailers;
359-
}
360-
return Task.CompletedTask;
361-
});
303+
await StartStreamAsync(streamId, requestHeaders, endStream: true);
362304

363-
await StartStreamAsync(1, requestHeaders, endStream: true);
305+
await ExpectAsync(Http2FrameType.HEADERS,
306+
withLength: 6,
307+
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS),
308+
withStreamId: streamId);
364309

365-
var trailersFrame = await ExpectAsync(Http2FrameType.HEADERS,
366-
withLength: 36,
367-
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
368-
withStreamId: 1);
310+
trailersFrame = await ExpectAsync(Http2FrameType.HEADERS,
311+
withLength: 16,
312+
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
313+
withStreamId: streamId);
369314

370-
// Ping will trigger the stream to be returned to the pool so we can assert it
371-
await SendPingAsync(Http2PingFrameFlags.NONE);
372-
await ExpectAsync(Http2FrameType.PING,
373-
withLength: 8,
374-
withFlags: (byte)Http2PingFrameFlags.ACK,
375-
withStreamId: 0);
376-
await SendPingAsync(Http2PingFrameFlags.NONE);
377-
await ExpectAsync(Http2FrameType.PING,
378-
withLength: 8,
379-
withFlags: (byte)Http2PingFrameFlags.ACK,
380-
withStreamId: 0);
315+
_hpackDecoder.Decode(trailersFrame.PayloadSequence, endHeaders: true, handler: this);
381316

382-
// Stream has been returned to the pool
383-
Assert.Equal(1, _connection.StreamPool.Count);
317+
Assert.Single(_decodedHeaders);
318+
Assert.Equal("true", _decodedHeaders[$"trailer-{i + 1}"]);
384319

385-
await StartStreamAsync(3, requestHeaders, endStream: true);
320+
_decodedHeaders.Clear();
386321

387-
trailersFrame = await ExpectAsync(Http2FrameType.HEADERS,
388-
withLength: 6,
389-
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
390-
withStreamId: 3);
322+
}
391323

392324
Assert.NotNull(trailersFirst);
393-
Assert.NotNull(trailersSecond);
394-
Assert.NotSame(trailersFirst, trailersSecond);
325+
Assert.NotNull(trailersLast);
326+
Assert.NotSame(trailersFirst, trailersLast);
395327

396-
await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false);
328+
await StopConnectionAsync(expectedLastStreamId: 5, ignoreNonGoAwayFrames: false);
329+
330+
async Task PingAsync()
331+
{
332+
await SendPingAsync(Http2PingFrameFlags.NONE);
333+
await ExpectAsync(Http2FrameType.PING,
334+
withLength: 8,
335+
withFlags: (byte)Http2PingFrameFlags.ACK,
336+
withStreamId: 0);
337+
await SendPingAsync(Http2PingFrameFlags.NONE);
338+
await ExpectAsync(Http2FrameType.PING,
339+
withLength: 8,
340+
withFlags: (byte)Http2PingFrameFlags.ACK,
341+
withStreamId: 0);
342+
}
397343
}
398344

399345
[Fact]

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ public async Task VariableMultipleStreamsInSequence_Success(int count, bool send
397397
}
398398

399399
[Fact]
400-
public async Task CustomResponseTrailersCollection_MultipleStreams_Reset()
400+
public async Task ResponseTrailers_MultipleStreams_Reset()
401401
{
402402
var requestHeaders = new[]
403403
{
@@ -408,34 +408,43 @@ public async Task CustomResponseTrailersCollection_MultipleStreams_Reset()
408408
new KeyValuePair<string, string>(HeaderNames.ContentType, "application/json")
409409
};
410410

411-
IHeaderDictionary trailersFirst = null;
412-
IHeaderDictionary trailersSecond = null;
413411
var requestCount = 0;
414-
await Http3Api.InitializeConnectionAsync(async context =>
412+
IHeaderDictionary trailersFirst = null;
413+
IHeaderDictionary trailersLast = null;
414+
await Http3Api.InitializeConnectionAsync(context =>
415415
{
416-
requestCount++;
417-
418416
var trailersFeature = context.Features.Get<IHttpResponseTrailersFeature>();
419-
if (requestCount == 1)
417+
if (requestCount == 0)
420418
{
421419
trailersFirst = new ResponseTrailersWrapper(trailersFeature.Trailers);
422420
trailersFeature.Trailers = trailersFirst;
423421
}
424422
else
425423
{
426-
trailersSecond = trailersFeature.Trailers;
424+
trailersLast = trailersFeature.Trailers;
427425
}
428-
await context.Response.Body.WriteAsync(Encoding.UTF8.GetBytes($"Hello world").AsMemory());
426+
trailersFeature.Trailers[$"trailer-{requestCount++}"] = "true";
427+
return Task.CompletedTask;
429428
});
430429

431-
for (int i = 0; i < 2; i++)
430+
431+
for (int i = 0; i < 3; i++)
432432
{
433-
var streamContext = await MakeRequestAsync(i, requestHeaders, sendData: false, waitForServerDispose: true);
433+
var requestStream = await Http3Api.CreateRequestStream();
434+
await requestStream.SendHeadersAsync(requestHeaders, endStream: true);
435+
var responseHeaders = await requestStream.ExpectHeadersAsync();
436+
437+
var data = await requestStream.ExpectTrailersAsync();
438+
Assert.Single(data);
439+
Assert.True(data.TryGetValue($"trailer-{i}", out var trailerValue) && bool.Parse(trailerValue));
440+
441+
await requestStream.ExpectReceiveEndOfStream();
442+
await requestStream.OnDisposedTask.DefaultTimeout();
434443
}
435444

436445
Assert.NotNull(trailersFirst);
437-
Assert.NotNull(trailersSecond);
438-
Assert.NotSame(trailersFirst, trailersSecond);
446+
Assert.NotNull(trailersLast);
447+
Assert.NotSame(trailersFirst, trailersLast);
439448
}
440449

441450
private async Task<ConnectionContext> MakeRequestAsync(int index, KeyValuePair<string, string>[] headers, bool sendData, bool waitForServerDispose)
@@ -504,7 +513,5 @@ public ResponseTrailersWrapper(IHeaderDictionary headers)
504513
public bool TryGetValue(string key, out StringValues value) => _innerHeaders.TryGetValue(key, out value);
505514
IEnumerator IEnumerable.GetEnumerator() => _innerHeaders.GetEnumerator();
506515
}
507-
508-
509516
}
510517
}

0 commit comments

Comments
 (0)