-
-
Notifications
You must be signed in to change notification settings - Fork 15.9k
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
Correctly handle wildcard address when bind to socket and using nativ… #4770
Conversation
@trustin @Scottmitch PTAL... This was reported in the Cassandra issue tracker: |
d5cbd3c
to
54ce69a
Compare
@@ -54,7 +54,7 @@ public void setup() { | |||
server = (EpollServerSocketChannel) bootstrap.group(GROUP) | |||
.channel(EpollServerSocketChannel.class) | |||
.handler(new ChannelInboundHandlerAdapter()) | |||
.bind(new InetSocketAddress(0)).syncUninterruptibly().channel(); | |||
.bind(new InetSocketAddress(NetUtil.LOCALHOST4, 0)).syncUninterruptibly().channel(); |
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.
This is needed to explicit bind on ipv4
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.
can you explain this change?
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.
yes.. this is needed as when you use new InetSocketAddress(0)
it basically creates a wildcard address (0.0.0.0:port). The EpollSocketTcpMd5Test
expect to have the server bound to ipv4 address.
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.
Not a big deal but would use LOCALHOST
provide any value by allowing for more variation (depending on environment)?
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.
nope as LOCALHOST may be an ipv6 address.
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.
What I'm asking is would s/LOCALHOST4/LOCALHOST/g
in this file provide any benefit?
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.
I know what you are asking but NetUtil.LOCALHOST
may be an ipv6 address. We need to ensure we always use an ipv4 one.
…e transport Motivation: When a wildcard address is used to bind a socket and ipv4 and ipv6 are usable we should accept both (just like JDK IO/NIO does). Modifications: Detect wildcard address and if so use in6addr_any Result: Correctly accept ipv4 and ipv6
lgtm! |
54ce69a
to
11e16a8
Compare
…e transport
Motivation:
When a wildcard address is used to bind a socket and ipv4 and ipv6 are usable we should accept both (just like JDK IO/NIO does).
Modifications:
Detect wildcard address and if so use in6addr_any
Result:
Correctly accept ipv4 and ipv6