skip content-length on DELETE and OPTIONS methods with no body #32259
Conversation
|
@dotnet-bot test Outerloop Windows x64 Debug Build |
|
@dotnet-bot test Outerloop Windows x64 Debug Build |
| [InlineData("DELETE")] | ||
| [InlineData("OPTIONS")] | ||
| [InlineData("HEAD")] | ||
| public async Task HttpRequest_BodylessMethod_NoContentLenght(string method) |
|
Minor issue above, otherwise LGTM. |
|
@dotnet-bot test Outerloop Windows x64 Debug Build |
| { | ||
| if (IsWinHttpHandler || IsNetfxHandler || IsUapHandler) | ||
| { | ||
| // WinHttp differ on some versions. |
There was a problem hiding this comment.
The comment needs to be updated for other handlers as well.
| Task<HttpResponseMessage> requestTask = client.SendAsync(request); | ||
| await server.AcceptConnectionAsync(async connection => | ||
| { | ||
| var headers = await connection.ReadRequestHeaderAsync(); |
There was a problem hiding this comment.
Don't use 'var' since the type is not obvious.
| foreach (string line in headers) | ||
| { | ||
| // There should be no Content-Length header. | ||
| Assert.False(line.StartsWith("Content-length:", StringComparison.InvariantCultureIgnoreCase)); |
There was a problem hiding this comment.
You should use Assert.DoesNotContain pattern instead of Assert.False.
See example:
There was a problem hiding this comment.
It produces better output when things fail. And it is pattern we already use elsewhere in the tests.
There was a problem hiding this comment.
ok. Existing pattern also exit but I can change it as you suggested.
|
@dotnet-bot test Outerloop Windows x64 Debug Build |
| @@ -174,7 +174,8 @@ internal bool MustHaveRequestBody | |||
| // Normalize before calling this | |||
| Debug.Assert(ReferenceEquals(this, Normalize(this))); | |||
|
|
|||
| return !ReferenceEquals(this, HttpMethod.Get) && !ReferenceEquals(this, HttpMethod.Head) && !ReferenceEquals(this, HttpMethod.Connect); | |||
| return !ReferenceEquals(this, HttpMethod.Get) && !ReferenceEquals(this, HttpMethod.Head) && !ReferenceEquals(this, HttpMethod.Connect) && | |||
There was a problem hiding this comment.
Not a change request just an observation, you could use is here to make this cleaner.
return !(this is HttpMethod.Get || this is HttpMethod.Head || this is HttpMethod.Connect || this is HttpMethod.Options || this is HttpMethod.Delete);
There was a problem hiding this comment.
Why ReferenceEquals? Why not String.Equals()? Does this work with
var method = new System.Net.Http.HttpMethod("GET"); ?
... I see, .Normalize() does the magic!
…t/corefx#32259) * skip content-length on DELETE and OPTIONS methods * breakup long line * update after sync up * fix spelling * skip new test on winhttp * disable test also on netfx and uap * feedback from reviews * use Assert.DoesNotContain for assert Commit migrated from dotnet/corefx@9921193
fixes #31172