Set ContentLength for TestHost #24107
Conversation
ghost
added
the
| else if (string.Equals(header.Key, HeaderNames.ContentLength, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| if (long.TryParse(header.Value.First(), out var contentLength)) | ||
| { | ||
| req.ContentLength = contentLength; | ||
| } | ||
| } |
There was a problem hiding this comment.
This shouldn't have affected the behavior, the Asp.Net ContentLength property is already read from the header. You should get the same test results if you remove this section. Does the test fail before adding this and pass after?
The expected workaround was reading System.Net.Http's ContentLength property because it has the side-effect of generating the header. #18463 (comment)
There was a problem hiding this comment.
Does the test fail before adding this and pass after?
Yes, but I changed the code as you said. Reading the property would set the header. Now if there's no body on the request the ContentLength is Zero. Is this the expected behavior?
| @@ -144,6 +144,10 @@ internal ClientHandler(PathString pathBase, ApplicationWrapper application) | |||
|
|
|||
| if (requestContent != null) | |||
There was a problem hiding this comment.
This code's a bit outdated, requestContent is never null, it's always set on line 69.
| { | ||
| var handler = new ClientHandler(new PathString(""), new DummyApplication(context => | ||
| { | ||
| Assert.Equal(0, context.Request.ContentLength); |
There was a problem hiding this comment.
Interesting. Because TestServer defaults to a StreamContent(Stream.Null), and Stream.Null's default length is 0, all requests that don't specify a body will get Content-Length: 0.
There was a problem hiding this comment.
Can you add a test where the client intentionally sets Transfer-Encoding: chunked? We don't want to automatically add a content-length in that scenario.
There was a problem hiding this comment.
Setting Transfer-Encoding: chunked for ByteArrayContent didn't do anything, the ContentLength was still 0.
But creating a content which return false on TryComputeLength would make ContentLength null. Is this good enough?
|
Thanks |
Fixes #18463