Skip to content

[Redirect] Allow redirect response to have body #580

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

Conversation

fabianfett
Copy link
Member

Motivation

Currently, we don’t consume the response body of redirect responses. Because of this requests, that receive a redirect response with response body, may hang indefinitely.

Changes

  • Consume redirect response body if less than 3kb
  • Cancel redirect response if larger than 3kb

Result

Redirect responses are consumed. Fixes #574

@fabianfett fabianfett added the 🔨 semver/patch No public API change. label Apr 11, 2022
@fabianfett fabianfett requested a review from Lukasa April 11, 2022 18:51
@fabianfett fabianfett force-pushed the ff-support-redirect-responses-with-body branch from 42d9c58 to d2c0583 Compare April 11, 2022 18:54
@fabianfett
Copy link
Member Author

@swift-server-bot test this please!

extension RequestBag {
struct StateMachine {
/// The maximum body size allowed, before a redirect response is cancelled.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this belongs to the static let above right?

@@ -16,14 +16,20 @@ import struct Foundation.URL
import NIOCore
import NIOHTTP1

extension HTTPClient {
fileprivate static let maxBodySizeRedirectResponse = 1024 * 3
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add a comment here to explain why it is 3kb?

@fabianfett fabianfett force-pushed the ff-support-redirect-responses-with-body branch from 5210407 to d83b1a0 Compare April 12, 2022 07:29
### Motivation

Currently, we don’t consume the response body of redirect responses. Because of this requests, that receive a redirect response with response body, may hang indefinitely.

### Changes

- Consume redirect response body if less than 3kb
- Cancel redirect response if larger than 3kb

### Result

Redirect responses are consumed. Fixes swift-server#574
@fabianfett fabianfett force-pushed the ff-support-redirect-responses-with-body branch from d83b1a0 to 5364192 Compare April 12, 2022 07:54
@fabianfett fabianfett merged commit 9d8cd95 into swift-server:main Apr 12, 2022
@fabianfett fabianfett deleted the ff-support-redirect-responses-with-body branch April 12, 2022 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GET to a URL with a redirection never completes nor times out
3 participants