Skip to content
This repository has been archived by the owner on Mar 19, 2019. It is now read-only.

Don't send chunked responses for HEAD requests #370

Merged
merged 1 commit into from
Jun 26, 2017
Merged

Conversation

Tratcher
Copy link
Member

#339
HEAD requests don't have response bodies, but they do emulate GET requests that may. HttpSysServer was assuming that the emulated GET request would result in a chunked response so it included the Transfer-Encoding: chunked header. However, it forgot to suppress the chunked terminator. This didn't show up in the tests because they weren't expecting additional data on the connection, and they also weren't re-using connections.

Rather than assuming chunking, it now omits the headers and pretends it's a content-length 0 response.

@Tratcher Tratcher added this to the 2.0.0 milestone Jun 24, 2017
@Tratcher Tratcher self-assigned this Jun 24, 2017
@Tratcher Tratcher requested review from halter73 and cesarblum June 24, 2017 01:03
Copy link
Contributor

@cesarblum cesarblum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit then :shipit:

Assert.True(response.Headers.Date.HasValue);
Assert.Equal("Microsoft-HTTPAPI/2.0", response.Headers.Server.ToString());
Assert.False(response.Content.Headers.Contains("Content-Length"));
Assert.Equal(0, response.Content.Headers.Count());

// Send a second request to check for connection corruption.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: change the comment to "to check that the connection wasn't corrupted." I was here wondering why the connection would be corrupted and how that code was checking that 😝

@Tratcher Tratcher merged commit c13ba3e into dev Jun 26, 2017
@Tratcher Tratcher deleted the tratcher/head branch June 26, 2017 19:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants