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.

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

net/http: set LocalAddrContextKey after conn accepted #18686

Closed
bradfitz opened this issue Jan 17, 2017 · 9 comments
Closed

net/http: set LocalAddrContextKey after conn accepted #18686

bradfitz opened this issue Jan 17, 2017 · 9 comments
Labels
FrozenDueToAge help wanted Suggested Issues that may be good for new contributors looking for work to do.
Milestone
Go1.9

Comments

@bradfitz
Copy link
Contributor

From comment on closed issue: #6732 (comment)

The http server sets LocalAddrContextKey in Serve(Listener) before any accept.

So if you pass a Listener listening on 0.0.0.0, that is what LocalAddrContextKey will say also. The current implementation considered only the case of distinguishing between multiple explicit listeners, not distinguishing the addresses arriving on a wildcard interface.

Instead, we should set the context value after an accept takes place.

/cc @SpComb
@bradfitz bradfitz added help wanted Suggested Issues that may be good for new contributors looking for work to do. labels Jan 17, 2017
@bradfitz bradfitz added this to the Go1.9 milestone Jan 17, 2017
This was referenced Jan 17, 2017
Closed
Closed
@elico
Copy link

elico commented Jan 21, 2017

#18734 is not a duplicate since it's a special case and socket which this address is always changing du to the diffrent connections that is being intercepted by the proxy.
It's not static but dynamic for a tproxy socker and a server.

@gopherbot
Copy link

CL https://golang.org/cl/35490 mentions this issue.

@nerdatmath
Copy link
Contributor

Since someone has already provided a CL for this, should the HelpWanted label be removed?

@elico
Copy link

elico commented Feb 20, 2017

@gopherbot @bradfitz I didn't understood the changes mentioned by the bot.

@rekby
Copy link

rekby commented Feb 28, 2017 •
edited

Why it planned to go 1.9 instead go 1.8.1? I think it is very small change - small fix small bug, no language change.

I need local ip too for https://github.com/rekby/lets-proxy it need to route requests.

Now I have forked http package to own copy for workaround the problem and wait solve in standard library.

@bradfitz
Copy link
Contributor Author

Because we only fix critical bugs/regressions without workarounds in 1.x.y point releases.

@sabey
Copy link

sabey commented Aug 25, 2017

I don't know what the protocol for closed issues is, but shouldn't this be reflected in the patch notes?
I just encountered a Connection Local Addr and not the previous Listener Addr using r.Context().Value(http.LocalAddrContextKey)

@sabey sabey mentioned this issue Aug 25, 2017
Closed
@elico
Copy link

elico commented Aug 25, 2017

@sabey so, is it working? is there a solution?

@sabey
Copy link

sabey commented Aug 25, 2017

it does work, the change was just unexpected as I couldn't find the documentation for it

@golang golang locked and limited conversation to collaborators Aug 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
6 participants
@bradfitz @elico @nerdatmath @rekby @sabey @gopherbot