Skip to content

Commit 6343ac6

Browse files
jkotalikjkotalik
committed
Feedback
1 parent 7f4e74f commit 6343ac6

File tree

7 files changed

+41
-38
lines changed

7 files changed

+41
-38
lines changed

‎src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ http_set_startup_error_page_content(_In_ byte* errorPageContent, int length)
562562

563563
EXTERN_C __MIDL_DECLSPEC_DLLEXPORT
564564
HRESULT
565-
http_supports_trailers(
565+
http_has_response4(
566566
_In_ IN_PROCESS_HANDLER* pInProcessHandler,
567567
_Out_ BOOL* supportsTrailers
568568
)
@@ -577,7 +577,7 @@ http_supports_trailers(
577577

578578
EXTERN_C __MIDL_DECLSPEC_DLLEXPORT
579579
HRESULT
580-
http_response_set_unknown_trailer(
580+
http_response_set_trailer(
581581
_In_ IN_PROCESS_HANDLER* pInProcessHandler,
582582
_In_ PCSTR pszHeaderName,
583583
_In_ PCSTR pszHeaderValue,

‎src/Servers/IIS/IIS/src/Core/IISHttpContext.Features.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ private void Initialize()
6565
_currentIServerVariablesFeature = this;
6666
_currentIHttpMaxRequestBodySizeFeature = this;
6767
_currentITlsConnectionFeature = this;
68-
_currentIHttpResponseTrailersFeature = this;
68+
_currentIHttpResponseTrailersFeature = GetResponseTrailersFeature();
6969
}
7070

7171
internal object FastFeatureGet(Type key)

‎src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ private async Task WriteBody(bool flush = false)
169169
// if request is done no need to flush, http.sys would do it for us
170170
if (result.IsCompleted)
171171
{
172-
if (HasTrailers && NativeMethods.HttpSupportTrailer(_pInProcessHandler))
172+
// When is the reader completed? Is it always after the request pipeline exits? Or can CompleteAsync make it complete early?
173+
if (HasTrailers)
173174
{
174175
SetResponseTrailers();
175176
}

‎src/Servers/IIS/IIS/src/Core/IISHttpContext.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,7 @@ public unsafe void SetResponseTrailers()
445445
{
446446
var builder = new StringBuilder();
447447
builder.Append(headerValues[0]);
448+
448449
for (var i = 1; i < headerValues.Count; i++)
449450
{
450451
builder.Append(',');
@@ -457,10 +458,10 @@ public unsafe void SetResponseTrailers()
457458
var headerValueBytes = Encoding.UTF8.GetBytes(headerValue);
458459
fixed (byte* pHeaderValue = headerValueBytes)
459460
{
460-
var headerNameBytes = Encoding.UTF8.GetBytes(headerPair.Key);
461+
var headerNameBytes = Encoding.ASCII.GetBytes(headerPair.Key);
461462
fixed (byte* pHeaderName = headerNameBytes)
462463
{
463-
NativeMethods.HttpResponseSetUnknownTrailer(_pInProcessHandler, pHeaderName, pHeaderValue, (ushort)headerValueBytes.Length);
464+
NativeMethods.HttpResponseSetTrailer(_pInProcessHandler, pHeaderName, pHeaderValue, (ushort)headerValueBytes.Length);
464465
}
465466
}
466467
}
@@ -663,7 +664,7 @@ private async Task HandleRequest()
663664
}
664665
catch (Exception ex)
665666
{
666-
_logger.LogError(0, ex, $"Unexpected exception in {nameof(IISHttpContext)}.{nameof(HandleRequest)}.");
667+
_logger.LogError(0, ex, $"Unexpected exception in {nameof(IISHttpContext)}.{nameof(HandleRequest)}.");
667668
}
668669
finally
669670
{

‎src/Servers/IIS/IIS/src/NativeMethods.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,9 @@ private static extern unsafe int http_websockets_write_bytes(
142142
private static extern unsafe int http_response_set_unknown_header(IntPtr pInProcessHandler, byte* pszHeaderName, byte* pszHeaderValue, ushort usHeaderValueLength, bool fReplace);
143143

144144
[DllImport(AspNetCoreModuleDll)]
145-
private static extern unsafe int http_supports_trailers(IntPtr pInProcessHandler, out bool supportsTrailers);
145+
private static extern unsafe int http_has_response4(IntPtr pInProcessHandler, out bool isResponse4);
146146
[DllImport(AspNetCoreModuleDll)]
147-
private static extern unsafe int http_response_set_unknown_trailer(IntPtr pInProcessHandler, byte* pszHeaderName, byte* pszHeaderValue, ushort usHeaderValueLength);
147+
private static extern unsafe int http_response_set_trailer(IntPtr pInProcessHandler, byte* pszHeaderName, byte* pszHeaderValue, ushort usHeaderValueLength);
148148

149149
[DllImport(AspNetCoreModuleDll)]
150150
private static extern unsafe int http_response_set_known_header(IntPtr pInProcessHandler, int headerId, byte* pHeaderValue, ushort length, bool fReplace);
@@ -313,15 +313,15 @@ internal static unsafe void HttpSetStartupErrorPageContent(byte[] content)
313313
}
314314
}
315315

316-
internal static unsafe void HttpResponseSetUnknownTrailer(IntPtr pInProcessHandler, byte* pHeaderName, byte* pHeaderValue, ushort length)
316+
internal static unsafe void HttpResponseSetTrailer(IntPtr pInProcessHandler, byte* pHeaderName, byte* pHeaderValue, ushort length)
317317
{
318-
Validate(http_response_set_unknown_trailer(pInProcessHandler, pHeaderName, pHeaderValue, length));
318+
Validate(http_response_set_trailer(pInProcessHandler, pHeaderName, pHeaderValue, length));
319319
}
320320

321321
internal static unsafe bool HttpSupportTrailer(IntPtr pInProcessHandler)
322322
{
323323
bool supportsTrailers;
324-
Validate(http_supports_trailers(pInProcessHandler, out supportsTrailers));
324+
Validate(http_has_response4(pInProcessHandler, out supportsTrailers));
325325
return supportsTrailers;
326326
}
327327

‎src/Servers/IIS/IIS/test/Common.FunctionalTests/Inprocess/ResponseTrailersTests.cs

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,20 @@ public async Task ResponseTrailers_HTTP2_TrailersAvailable()
3030

3131
var response = await SendRequestAsync(deploymentResult.HttpClient.BaseAddress.ToString() + "ResponseTrailers_HTTP2_TrailersAvailable");
3232

33+
response.EnsureSuccessStatusCode();
34+
Assert.Equal(HttpVersion.Version11, response.Version);
35+
Assert.Empty(response.TrailingHeaders);
36+
}
37+
38+
[ConditionalFact]
39+
public async Task ResponseTrailers_HTTP1_TrailersNotAvailable()
40+
{
41+
var deploymentParameters = Fixture.GetBaseDeploymentParameters(hostingModel: IntegrationTesting.HostingModel.OutOfProcess);
42+
43+
var deploymentResult = await DeployAsync(deploymentParameters);
44+
45+
var response = await SendRequestAsync(deploymentResult.HttpClient.BaseAddress.ToString() + "ResponseTrailers_HTTP1_TrailersNotAvailable");
46+
3347
response.EnsureSuccessStatusCode();
3448
Assert.Equal(HttpVersion.Version20, response.Version);
3549
Assert.Empty(response.TrailingHeaders);
@@ -136,23 +150,18 @@ public async Task ResponseTrailers_WithContentLengthBodyAndDeclared_TrailersSent
136150
}
137151

138152
[ConditionalFact]
139-
public async Task ResponseTrailers_WithContentLengthBodyAndDeclaredButMissingTrailers_Completes()
153+
public async Task ResponseTrailers_MultipleValues_SentAsSeparateHeaders()
140154
{
141-
var body = "Hello World";
142-
143155
var deploymentParameters = GetHttpsDeploymentParameters();
144156
var deploymentResult = await DeployAsync(deploymentParameters);
145157

146-
var response = await SendRequestAsync(deploymentResult.HttpClient.BaseAddress.ToString() + "ResponseTrailers_WithContentLengthBodyAndDeclaredButMissingTrailers_Completes");
158+
var response = await SendRequestAsync(deploymentResult.HttpClient.BaseAddress.ToString() + "ResponseTrailers_MultipleValues_SentAsSeparateHeaders");
147159

148160
response.EnsureSuccessStatusCode();
149161
Assert.Equal(HttpVersion.Version20, response.Version);
150-
// Avoid HttpContent's automatic content-length calculation.
151-
Assert.True(response.Content.Headers.TryGetValues(HeaderNames.ContentLength, out var contentLength), HeaderNames.ContentLength);
152-
Assert.Equal(body.Length.ToString(CultureInfo.InvariantCulture), contentLength.First());
153-
Assert.Equal("TrailerName", response.Headers.Trailer.Single());
154-
Assert.Equal(body, await response.Content.ReadAsStringAsync());
155-
Assert.Empty(response.TrailingHeaders);
162+
Assert.NotEmpty(response.TrailingHeaders);
163+
164+
Assert.Equal(new[] { "TrailerValue0", "TrailerValue1" }, response.TrailingHeaders.GetValues("TrailerName"));
156165
}
157166

158167
private IISDeploymentParameters GetHttpsDeploymentParameters()

‎src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,6 +1041,13 @@ public Task ResponseTrailers_HTTP2_TrailersAvailable(HttpContext context)
10411041
return Task.FromResult(0);
10421042
}
10431043

1044+
public Task ResponseTrailers_HTTP1_TrailersNotAvailable(HttpContext context)
1045+
{
1046+
Assert.Equal("HTTP/1.1", context.Request.Protocol);
1047+
Assert.False(context.Response.SupportsTrailers());
1048+
return Task.FromResult(0);
1049+
}
1050+
10441051
public Task ResponseTrailers_ProhibitedTrailers_Blocked(HttpContext context)
10451052
{
10461053
Assert.True(context.Response.SupportsTrailers());
@@ -1098,22 +1105,7 @@ public async Task ResponseTrailers_WithContentLengthBodyAndDeclaredButMissingTra
10981105
await context.Response.WriteAsync(body);
10991106
}
11001107

1101-
private TaskCompletionSource<object> _trailersReceivedForBody = new TaskCompletionSource<object>();
1102-
public async Task ResponseTrailers_CompleteAsyncWithBody_TrailersSent(HttpContext context)
1103-
{
1104-
await context.Response.WriteAsync("Hello World");
1105-
context.Response.AppendTrailer("TrailerName", "Trailer Value");
1106-
await context.Response.CompleteAsync();
1107-
await _trailersReceivedForBody.Task;
1108-
}
1109-
1110-
public Task ResponseTrailers_CompleteAsyncWithBody_TrailersSent_Complete(HttpContext context)
1111-
{
1112-
_trailersReceivedForBody.SetResult(null);
1113-
return Task.FromResult(0);
1114-
}
1115-
1116-
public Task ResponseTrailers_MultipleValues_SentAsSeperateHeaders(HttpContext context)
1108+
public Task ResponseTrailers_MultipleValues_SentAsSeparateHeaders(HttpContext context)
11171109
{
11181110
context.Response.AppendTrailer("trailername", new StringValues(new[] { "TrailerValue0", "TrailerValue1" }));
11191111
return Task.FromResult(0);

0 commit comments

Comments
 (0)