-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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
net/http: check RemoteAddr isn't nil before dereferencing #60823
Conversation
This PR (HEAD: ff3505d) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/503656 to see it. Tip: You can toggle comments from me using the |
Message from Damien Neil: Patch Set 1: Code-Review+2 Run-TryBot+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/503656. |
Message from Gopher Robot: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/503656. |
Message from Gopher Robot: Patch Set 1: TryBot-Result+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/503656. |
Message from Dmitri Shuralyov: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/503656. |
Message from Dmitri Shuralyov: Patch Set 1: Auto-Submit+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/503656. |
RemoteAddr can return nil in some cases, this fix prevents a panic. I chatted with @neild about this beforehand, but what's happening in our case is that a connection comes in to the HTTP server which is then immediately closed (we discovered this issue by accident using nmap). The network implementation that we're using (it happens to be gVisor via its gonet adaptor) is returning nil from RemoteAddr(), presumably as there is no remote at that point. But, ultimately, since RemoteAddr returns an interface it is always possible for it to return nil, and indeed conn.RemoteAddr in this file does exactly that if the conn is not ok. Change-Id: Ibe67ae6e30b68e2776df5ee2911bf5f1dc539641 GitHub-Last-Rev: ff3505d GitHub-Pull-Request: #60823 Reviewed-on: https://go-review.googlesource.com/c/go/+/503656 Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Damien Neil <dneil@google.com> Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
This PR is being closed because golang.org/cl/503656 has been merged. |
RemoteAddr can return nil in some cases, this fix prevents a panic.
I chatted with @neild about this beforehand, but what's happening in our
case is that a connection comes in to the HTTP server which is then
immediately closed (we discovered this issue by accident using nmap).
The network implementation that we're using (it happens to be gVisor
via its gonet adaptor) is returning nil from RemoteAddr(), presumably
as there is no remote at that point.
But, ultimately, since RemoteAddr returns an interface it is always
possible for it to return nil, and indeed conn.RemoteAddr in this file
does exactly that if the conn is not ok.