Skip to content
New issue

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

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

Already on GitHub? # to your account

ReadHeaderTimeout is setting a hard timeout on connections regardless of header sending #75

Closed
vansante opened this issue Jul 13, 2021 · 7 comments · Fixed by #76
Closed
Assignees
Labels
Milestone

Comments

@vansante
Copy link

I think this setting is supposed to timeout connections that are not sending the headers and terminate them after this amount of time. However, I set the setting to 5 seconds, and the result is that all connections are terminated after 5 seconds, even if they correctly sent the headers. The connection starts up, but is then abruptly ended after the 5 seconds are passed.

I think what is missing in #74 is a call to SetReadDeadline to reset the timeout after the proxy header was sent successfully.

@pires pires self-assigned this Jul 13, 2021
@pires
Copy link
Owner

pires commented Jul 13, 2021

Hmmm, let me check.

@pires
Copy link
Owner

pires commented Jul 13, 2021

You are correct. Thanks a ton for reporting. Fixing it now!

@pires pires added the bug label Jul 13, 2021
@pires pires added this to the 0.6 milestone Jul 13, 2021
pires added a commit that referenced this issue Jul 13, 2021
As reported in #75

Signed-off-by: Pires <pjpires@gmail.com>
pires added a commit that referenced this issue Jul 13, 2021
Fixes #75

Signed-off-by: Pires <pjpires@gmail.com>
@pires
Copy link
Owner

pires commented Jul 13, 2021

Can you, please, help test #75?

@vansante
Copy link
Author

I tested #76 and I got the same results with version github.com/pires/go-proxyproto@v0.6.1-0.20210713120647-bab7dd263f97 in my go.mod .

@jefferai
Copy link

Additionally, #74 is lacking in picking a suitable default. Considering the reason for that change's existence, it really should include a default timeout. If people really want to have no timeout at all, a negative value could be defined as "no timeout".

@pires
Copy link
Owner

pires commented Aug 23, 2021

@jefferai what do you think a sane default should be? One second?

Also, can you please help review #76? I think I fixed it in that PR but the reporter can't seem to reproduce successfully so more eyes/minds would help.

pires added a commit that referenced this issue Sep 8, 2021
As reported in #75

Signed-off-by: Pires <pjpires@gmail.com>
pires added a commit that referenced this issue Sep 8, 2021
Fixes #75

Signed-off-by: Pires <pjpires@gmail.com>
@pires pires closed this as completed in #76 Sep 8, 2021
@pires
Copy link
Owner

pires commented Sep 8, 2021

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants