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

Check for proxy prefix early, so RemoteAddr doesn't block Accept #2

Closed
wants to merge 1 commit into from
Closed

Conversation

bradfitz
Copy link

Check for proxy prefix early, so RemoteAddr doesn't block Accept.

And idiomatic cleanups because I couldn't help myself.

I don't use this and don't care whether you accept this pull request. Feel free to close it outright if you'd like.

I created it as part of discussion for https://go-review.googlesource.com/#/c/15832/

block Accept.

And idiomatic cleanups because I couldn't help myself.
@@ -1,11 +1,12 @@
// Package proxoproto implements a net.Listener supporting HAProxy PROTO protocol.
Copy link

Choose a reason for hiding this comment

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

typo in package name

Copy link
Author

Choose a reason for hiding this comment

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

I have no intention of fixing this. You can take this broken PR and do with it as you please.

Or take this PR and fix the typo later.

Copy link

Choose a reason for hiding this comment

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

Right, just directing this at @armon when he merges it. Thanks for your help.

@glasser
Copy link

glasser commented Oct 14, 2015

@armon This pull request does clean up the code but it does not resolve #1. See discussion at https://go-review.googlesource.com/#/c/15832/1

@owenthereal
Copy link

@armon Any update on this?

@keymon
Copy link
Contributor

keymon commented Jul 12, 2016

+1

@glasser
Copy link

glasser commented Jul 12, 2016

This is fixed in Go 1.6.

@keymon
Copy link
Contributor

keymon commented Jul 12, 2016

It is fixed in Go 1.6 because it does not call the RemoteAddr on client connection.

But anyway, in our use case we want to use the library with gorouter from Cloudfoundry which uses go 1.5 :(

I am looking at the issue and I would come suggestions.

@glasser
Copy link

glasser commented Jul 12, 2016

I linked to an annoying workaround on #1.

@glasser
Copy link

glasser commented Jul 12, 2016

(and to reiterate, this PR #2 does not fix the problem and should not be merged.)

@keymon
Copy link
Contributor

keymon commented Jul 12, 2016

@glasser ack, will continue there

@pires
Copy link

pires commented Aug 24, 2016

I think #4 fixed this.

@jefferai
Copy link
Collaborator

Given that Go 1.8 and 1.7 are the only supported versions at this point, I'm going to close this.

@jefferai jefferai closed this Jun 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants