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

[Dubbo-2064] Ipv6 support #2079

Merged
merged 9 commits into from
Aug 2, 2018
Merged

[Dubbo-2064] Ipv6 support #2079

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4a554bc
Url support ipv6 address.
ralf0131 Jul 13, 2018
8a23fe3
Unwrap if statement, since returned Enumeration can't be null. No fun…
ralf0131 Jul 14, 2018
3602e8e
Ensure Dubbo provider/consumer both works under ipv6 multicast network.
ralf0131 Jul 14, 2018
0e09fb0
Fix typo.
ralf0131 Jul 14, 2018
eb00834
Fix typo.
ralf0131 Jul 14, 2018
cba34cd
Disable mocking final class feature of mockito to avoid UT failure.
ralf0131 Jul 15, 2018
db01ebd
Add null check for NetworkInterface.getNetworkInterfaces()
ralf0131 Aug 2, 2018
9de876b
Improve UT.
ralf0131 Aug 2, 2018
fef30cc
Improve UT.
ralf0131 Aug 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dubbo-common/src/main/java/org/apache/dubbo/common/URL.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ public static URL valueOf(String url) {
}
url = url.substring(i + 1);
}
i = url.indexOf(":");
i = url.lastIndexOf(":");
if (i >= 0 && i < url.length() - 1) {
port = Integer.parseInt(url.substring(i + 1));
url = url.substring(0, i);
Expand Down
30 changes: 13 additions & 17 deletions dubbo-common/src/main/java/org/apache/dubbo/common/utils/NetUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,26 +194,22 @@ private static InetAddress getLocalAddress0() {
}
try {
Enumeration<NetworkInterface> interfaces = NetworkInterface.getNetworkInterfaces();
if (interfaces != null) {
while (interfaces.hasMoreElements()) {
try {
NetworkInterface network = interfaces.nextElement();
Enumeration<InetAddress> addresses = network.getInetAddresses();
if (addresses != null) {
while (addresses.hasMoreElements()) {
try {
InetAddress address = addresses.nextElement();
if (isValidAddress(address)) {
return address;
}
} catch (Throwable e) {
logger.warn(e);
}
while (interfaces.hasMoreElements()) {
Copy link
Member

Choose a reason for hiding this comment

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

    public static Enumeration<NetworkInterface> getNetworkInterfaces()
        throws SocketException {
        final NetworkInterface[] netifs = getAll();

        // specified to return null if no network interfaces
        if (netifs == null)
            return null;

Should check the interfaces property null value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I am using Java 10, the implementation is different with Java 8. In Java 10 the implementation guarantees no null value is returned. I will add the null check.

try {
NetworkInterface network = interfaces.nextElement();
Enumeration<InetAddress> addresses = network.getInetAddresses();
while (addresses.hasMoreElements()) {
try {
InetAddress address = addresses.nextElement();
if (isValidAddress(address)) {
return address;
}
} catch (Throwable e) {
Copy link
Member

Choose a reason for hiding this comment

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

rename e to ignored will be better

logger.warn(e);
}
} catch (Throwable e) {
logger.warn(e);
}
} catch (Throwable e) {
Copy link
Member

Choose a reason for hiding this comment

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

rename e to ignored will be better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, this is just coding style. I prefer to use

catch (Throwable e) {
   // ignore
}

instead of

catch(Throwable ignored) {

}

I found several codes in Dubbo's code that is written in the former way, but did not find my codes that is written in the way you suggested.

So I guess I am going to leave it unchanged.

logger.warn(e);
}
}
} catch (Throwable e) {
Expand Down
16 changes: 16 additions & 0 deletions dubbo-common/src/test/java/org/apache/dubbo/common/URLTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,4 +652,20 @@ public void testUserNamePasswordContainsAt(){
assertEquals("1.0.0", url.getParameter("version"));
assertEquals("morgan", url.getParameter("application"));
}


@Test
public void testIpV6Address(){
// Test username or password contains "@"
URL url = URL.valueOf("ad@min111:haha@1234@2001:0db8:85a3:08d3:1319:8a2e:0370:7344:20880/context/path?version=1.0.0&application=morgan");
assertNull(url.getProtocol());
assertEquals("ad@min111", url.getUsername());
assertEquals("haha@1234", url.getPassword());
assertEquals("2001:0db8:85a3:08d3:1319:8a2e:0370:7344", url.getHost());
assertEquals(20880, url.getPort());
assertEquals("context/path", url.getPath());
assertEquals(2, url.getParameters().size());
assertEquals("1.0.0", url.getParameter("version"));
assertEquals("morgan", url.getParameter("application"));
}
}