Skip to content

Netty connection timeouts of 0 or -1 are not applied correctly #16535

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Netty connection timeouts of 0 or -1 are not applied correctly #16535

wants to merge 1 commit into from

Conversation

ayudovin
Copy link
Contributor

@ayudovin ayudovin commented Apr 11, 2019 •
edited by philwebb
Loading

Fix for #16497

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 11, 2019
@philwebb philwebb mentioned this pull request Apr 11, 2019
Closed
@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 11, 2019
@philwebb philwebb added this to the 2.1.x milestone Apr 11, 2019
@philwebb
Copy link
Member

Thanks once again @ayudovin !

@snicoll
Copy link
Member

snicoll commented Apr 15, 2019

@ayudovin can you please remove the merge commit from the PR? (and not use merge commits in general). Rebasing your changes from master and push force the result on your existing branch (fix-connection-timeout-netty) is what I'd like to see. Thank you.

@ayudovin ayudovin force-pushed the fix-connection-timeout-netty branch from 39229ce to f54bd28 Compare April 15, 2019 07:54
@ayudovin ayudovin force-pushed the fix-connection-timeout-netty branch from f54bd28 to 8b83046 Compare April 15, 2019 08:15
@ayudovin
Copy link
Contributor Author

@snicoll, I tried to follow your suggestion. Can you check that it's alright?

@snicoll
Copy link
Member

snicoll commented Apr 15, 2019

Perfect, thank you very much!

@ayudovin
Copy link
Contributor Author

@snicoll, Do I need to rebase again my changes?
As I see, failed tests are not related to my changes

@philwebb
Copy link
Member

@ayudovin There's no need for an additional rebase, we'll take care of that when merging.

@ayudovin
Copy link
Contributor Author

@philwebb Do you have updates about this pull request?

@snicoll
Copy link
Member

snicoll commented Apr 30, 2019 •
edited
Loading

@ayudovin the PR is triaged with an assigned milestone. All that's left is a review and merge. We have a lot of things going on at the moment and we'll get to it as soon as we can.

@philwebb philwebb added the for: merge-with-amendments Needs some changes when we merge label May 15, 2019
@philwebb philwebb changed the title Fix connection timeout configuration for Netty Netty connection timeouts of 0 or -1 are not applied correctly Jun 17, 2019
@philwebb philwebb mentioned this pull request Jun 17, 2019
Closed
philwebb pushed a commit that referenced this pull request Jun 17, 2019
@ayudovin @philwebb
Fix connection timeout configuration for Netty
b0e4c71 
Update `NettyWebServerFactoryCustomizer` to deal with the fact that
Netty treats `0` and negative connection timeout values differently to
Tomcat, Undertow and Jetty.

See gh-16535
philwebb added a commit that referenced this pull request Jun 17, 2019
@philwebb
Polish "Fix connection timeout configuration for Netty"
692bda1 
See gh-16535
@philwebb philwebb closed this in 5e3438f Jun 17, 2019
@philwebb philwebb modified the milestones: 2.1.x, 2.1.6 Jun 17, 2019
@philwebb
Copy link
Member

Thanks for the fix @ayudovin, and for discovering and fixing the bug in PropertyMapper!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
for: merge-with-amendments Needs some changes when we merge type: bug A general bug
Projects
None yet
Milestone
2.1.6
Development

Successfully merging this pull request may close these issues.
4 participants
@ayudovin @philwebb @snicoll @spring-projects-issues