From 4c6d946fbadfb14806f8232b844c5e6a26eff2de Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Wed, 17 May 2017 19:15:40 -0700 Subject: [PATCH] KQueueSocket#setTrafficClass exceptions Motivation: MacOS will throw an error when attempting to set the IP_TOS socket option if IPv6 is available, and also when getting the value for IP_TOS. Modifications: - Socket#setTrafficClass and Socket#getTrafficClass should try to use IPv6 first, and check if the error code indicates the protocol is not supported before trying IPv4 Result: Fixes https://github.com/netty/netty/issues/6741. --- .../netty/channel/epoll/EpollSocketTest.java | 7 ++ .../channel/kqueue/KQueueSocketTest.java | 4 +- .../netty/channel/unix/tests/SocketTest.java | 7 ++ .../src/main/c/netty_unix_socket.c | 64 +++++++++++++------ 4 files changed, 60 insertions(+), 22 deletions(-) diff --git a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketTest.java b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketTest.java index 4a094b7e86..8a2c997634 100644 --- a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketTest.java +++ b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketTest.java @@ -19,6 +19,7 @@ import io.netty.channel.unix.DomainSocketAddress; import io.netty.channel.unix.PeerCredentials; import io.netty.channel.unix.tests.SocketTest; import io.netty.channel.unix.tests.UnixTestUtils; +import org.junit.BeforeClass; import org.junit.Test; import java.io.IOException; @@ -26,8 +27,14 @@ import java.io.IOException; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; public class EpollSocketTest extends SocketTest { + @BeforeClass + public static void loadJNI() { + assumeTrue(Epoll.isAvailable()); + } + @Test public void testTcpCork() throws Exception { assertFalse(socket.isTcpCork()); diff --git a/transport-native-kqueue/src/test/java/io/netty/channel/kqueue/KQueueSocketTest.java b/transport-native-kqueue/src/test/java/io/netty/channel/kqueue/KQueueSocketTest.java index 1634c27c11..38f09c00fd 100644 --- a/transport-native-kqueue/src/test/java/io/netty/channel/kqueue/KQueueSocketTest.java +++ b/transport-native-kqueue/src/test/java/io/netty/channel/kqueue/KQueueSocketTest.java @@ -17,7 +17,6 @@ package io.netty.channel.kqueue; import io.netty.channel.unix.DomainSocketAddress; import io.netty.channel.unix.PeerCredentials; -import io.netty.channel.unix.Socket; import io.netty.channel.unix.tests.SocketTest; import io.netty.channel.unix.tests.UnixTestUtils; import org.junit.BeforeClass; @@ -27,11 +26,12 @@ import java.io.IOException; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; public class KQueueSocketTest extends SocketTest { @BeforeClass public static void loadJNI() { - KQueue.isAvailable(); + assumeTrue(KQueue.isAvailable()); } @Test diff --git a/transport-native-unix-common-tests/src/main/java/io/netty/channel/unix/tests/SocketTest.java b/transport-native-unix-common-tests/src/main/java/io/netty/channel/unix/tests/SocketTest.java index b700bc4e4d..b5707baf1e 100644 --- a/transport-native-unix-common-tests/src/main/java/io/netty/channel/unix/tests/SocketTest.java +++ b/transport-native-unix-common-tests/src/main/java/io/netty/channel/unix/tests/SocketTest.java @@ -88,4 +88,11 @@ public abstract class SocketTest { socket.close(); socket.close(); } + + @Test + public void testTrafficClass() throws IOException { + final int value = 0x1; + socket.setTrafficClass(value); + assertEquals(value, socket.getTrafficClass()); + } } diff --git a/transport-native-unix-common/src/main/c/netty_unix_socket.c b/transport-native-unix-common/src/main/c/netty_unix_socket.c index d0da2769c9..6de618eb9a 100644 --- a/transport-native-unix-common/src/main/c/netty_unix_socket.c +++ b/transport-native-unix-common/src/main/c/netty_unix_socket.c @@ -288,28 +288,42 @@ static jobject _recvFrom(JNIEnv* env, jint fd, void* buffer, jint pos, jint limi return createDatagramSocketAddress(env, &addr, res); } +static void netty_unix_socket_optionHandleError(JNIEnv* env, int err, char* method) { + if (err == EBADF) { + netty_unix_errors_throwClosedChannelException(env); + } else { + netty_unix_errors_throwChannelExceptionErrorNo(env, method, err); + } +} + +static void netty_unix_socket_setOptionHandleError(JNIEnv* env, int err) { + netty_unix_socket_optionHandleError(env, err, "setsockopt() failed: "); +} + +static void netty_unix_socket_getOptionHandleError(JNIEnv* env, int err) { + netty_unix_socket_optionHandleError(env, err, "getsockopt() failed: "); +} + +static int netty_unix_socket_setOption0(jint fd, int level, int optname, const void* optval, socklen_t len) { + return setsockopt(fd, level, optname, optval, len); +} + +static int netty_unix_socket_getOption0(jint fd, int level, int optname, void* optval, socklen_t optlen) { + return getsockopt(fd, level, optname, optval, &optlen); +} + int netty_unix_socket_getOption(JNIEnv* env, jint fd, int level, int optname, void* optval, socklen_t optlen) { - int rc = getsockopt(fd, level, optname, optval, &optlen); + int rc = netty_unix_socket_getOption0(fd, level, optname, optval, optlen); if (rc < 0) { - int err = errno; - if (err == EBADF) { - netty_unix_errors_throwClosedChannelException(env); - } else { - netty_unix_errors_throwChannelExceptionErrorNo(env, "getsockopt() failed: ", err); - } + netty_unix_socket_getOptionHandleError(env, errno); } return rc; } int netty_unix_socket_setOption(JNIEnv* env, jint fd, int level, int optname, const void* optval, socklen_t len) { - int rc = setsockopt(fd, level, optname, optval, len); + int rc = netty_unix_socket_setOption0(fd, level, optname, optval, len); if (rc < 0) { - int err = errno; - if (err == EBADF) { - netty_unix_errors_throwClosedChannelException(env); - } else { - netty_unix_errors_throwChannelExceptionErrorNo(env, "setsockopt() failed: ", err); - } + netty_unix_socket_setOptionHandleError(env, errno); } return rc; } @@ -689,13 +703,18 @@ static void netty_unix_socket_setSoLinger(JNIEnv* env, jclass clazz, jint fd, ji } static void netty_unix_socket_setTrafficClass(JNIEnv* env, jclass clazz, jint fd, jint optval) { - netty_unix_socket_setOption(env, fd, IPPROTO_IP, IP_TOS, &optval, sizeof(optval)); - /* Try to set the ipv6 equivalent, but don't throw if this is an ipv4 only socket. */ - int rc = netty_unix_socket_setOption(env, fd, IPPROTO_IPV6, IPV6_TCLASS, &optval, sizeof(optval)); + int rc = netty_unix_socket_setOption0(fd, IPPROTO_IPV6, IPV6_TCLASS, &optval, sizeof(optval)); if (rc < 0 && errno != ENOPROTOOPT) { - netty_unix_errors_throwChannelExceptionErrorNo(env, "setting ipv6 dscp failed: ", errno); + netty_unix_socket_setOptionHandleError(env, errno); } + + /* Linux allows both ipv4 and ipv6 families to be set */ +#ifdef __linux__ + else { + netty_unix_socket_setOption(env, fd, IPPROTO_IP, IP_TOS, &optval, sizeof(optval)); + } +#endif } static jint netty_unix_socket_isKeepAlive(JNIEnv* env, jclass clazz, jint fd) { @@ -743,10 +762,15 @@ static jint netty_unix_socket_getSoLinger(JNIEnv* env, jclass clazz, jint fd) { } static jint netty_unix_socket_getTrafficClass(JNIEnv* env, jclass clazz, jint fd) { + /* macOS may throw an error if IPv6 is supported and it is not consulted first */ int optval; - if (netty_unix_socket_getOption(env, fd, IPPROTO_IP, IP_TOS, &optval, sizeof(optval)) == -1) { - return -1; + if (netty_unix_socket_getOption0(fd, IPPROTO_IPV6, IPV6_TCLASS, &optval, sizeof(optval)) == -1) { + if (errno != ENOPROTOOPT || netty_unix_socket_getOption0(fd, IPPROTO_IP, IP_TOS, &optval, sizeof(optval)) == -1) { + netty_unix_socket_getOptionHandleError(env, errno); + return -1; + } } + return optval; }