Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve body handling for HEAD responses #2310

Merged
merged 1 commit into from Apr 14, 2020
Merged

Conversation

tanner0101
Copy link
Member

@tanner0101 tanner0101 commented Apr 13, 2020

Improves logic for handling HEAD responses (#2310).

Previously when HEAD requests were detected, the response body would be replaced with an empty body and the content-length header would be re-added. This caused problems with responses that don't use the content-length header, even for non-HEAD requests.

Now, when HEAD requests are detected, the response body is not changed. Instead, a flag is set signaling the serializer to skip serialization of the body.

@tanner0101 tanner0101 changed the title Don re-add content-length header for HEAD request responses Improve body handling for HEAD responses Apr 13, 2020
@tanner0101 tanner0101 added bug Something isn't working semver-patch Internal changes only labels Apr 13, 2020
@tanner0101 tanner0101 added this to Awaiting Review in Vapor 4 via automation Apr 13, 2020
@@ -32,6 +32,9 @@ public final class Response: CustomStringConvertible {
didSet { self.headers.updateContentLength(self.body.count) }
}

// If `true`, don't serialize the body.
var forHeadRequest: Bool
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Propose to rename to headOnlyRequest

@tanner0101 tanner0101 merged commit bee26c5 into master Apr 14, 2020
Vapor 4 automation moved this from Awaiting Review to Done Apr 14, 2020
@tanner0101 tanner0101 deleted the tn-head-res-body branch April 14, 2020 18:16
@tanner0101
Copy link
Member Author

These changes are now available in 4.0.2

pull bot pushed a commit to scope-demo/vapor that referenced this pull request Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working semver-patch Internal changes only
Projects
Vapor 4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants