-
Notifications
You must be signed in to change notification settings - Fork 524
p2p: make sure p2p http server runs on all interfaces #6123
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
p2p: make sure p2p http server runs on all interfaces #6123
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6123 +/- ##
==========================================
+ Coverage 56.14% 56.18% +0.04%
==========================================
Files 494 494
Lines 69931 69936 +5
==========================================
+ Hits 39266 39297 +31
+ Misses 27987 27963 -24
+ Partials 2678 2676 -2 ☔ View full report in Codecov by Sentry. |
| if len(streamHost.Addrs()) == 0 { | ||
| logging.Base().Debugf("MakeHTTPServer: no addresses for %s, asking to listen all interfaces", streamHost.ID()) | ||
| httpServer.ListenAddrs = []multiaddr.Multiaddr{ | ||
| multiaddr.StringCast("/ip4/0.0.0.0/tcp/0/http"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this /ip4/0.0.0.0/tcp/0/http mean it will pick a different port from libp2p or the same one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not used from what I see in the source code, it gets a listener from basic_host but checks ListenAddrs for empty. But I agree, it is better to use the same port as basic_host, I'll followup in another PR.
Summary
libp2phttp server rejects to run when no listen addresses can be determined and no
ListenAddrsprovided. After filtering out all non-routable and private addresses via address factory inhost.Addrs(), our http server failed to start sincehost.Addrs()returned empty.Fixed by handling this situation explicitly in
MakeHTTPServerand providing0.0.0.0viaListenAddrs.Note, if
NetAddressis set to some specific address, all this new logic is not executed - http server will start on theNetAddressinterface as before this change.Test Plan
Added an explicit unit test for this
NetAddress = ":0"case