search

pires / go-proxyproto

A Go library implementation of the PROXY protocol, versions 1 and 2.
Apache License 2.0
478 stars 107 forks source link

Update the way read header is handled #80

Closed antoniomika closed 3 years ago

antoniomika commented 3 years ago

edited by @pires

Fixes #75

pires commented 3 years ago

I'll have to think about this and this week I'm already packed. Thanks @antoniomika

drakkan commented 3 years ago

Hi,

I just tested this patch with SFTPGo for proxy SSH and FTP connections, it looks fine. Are there any blockers to merge it? thanks

coveralls commented 3 years ago

Coverage Status

Coverage increased (+1.2%) to 95.543% when pulling 4a3af4fa70ea7953f5d57258ca0030ab10f11f43 on antoniomika:am/fix_read_deadline into bab7dd263f970c6c7b509e39ecfa2d50560f3f31 on pires:bugfix/reset_conn_read_deadline_after_successful_header.

antoniomika commented 3 years ago

Hey @pires,

Added some more documentation, cleaned up some unneeded boilerplate and we should be ready to go!

Best,

antoniomika commented 3 years ago

@pires just a heads up, this should merge first, followed by #76 after this lands.

pires commented 3 years ago

@pires just a heads up, this should merge first, followed by #76 after this lands.

What is there in #76 that isn't solved here?

drakkan commented 3 years ago

@pires just a heads up, this should merge first, followed by #76 after this lands.

What is there in #76 that isn't solved here?

To apply this patch you have to apply #76 first.

pires commented 3 years ago

@pires just a heads up, this should merge first, followed by #76 after this lands.

What is there in #76 that isn't solved here?

To apply this patch you have to apply #76 first.

Damn, I must have that espresso urgently! 😅

antoniomika commented 3 years ago

Somehow #82 landed here and I'd like to give the chance for @drakkan to get their contribution in. Can you please remove this?

Sorry about that, I should've checked if there was another PR with the static check fixes. I use code spaces and was a bit OCD with the linting issues. I actually started working on #73 but those changes were more complicated so I decided to leave them for when I had more time. You can remove those changes in your PR now and merge in #82.

pires commented 3 years ago

Yeah #73 is painful. Also have a branch for a long while now 😅