diff --git a/CHANGELOG.md b/CHANGELOG.md index 57e240b89..f57785e41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fix NPE after 2nd send() in unconnected channel + cleanup. [#33](https://github.com/netsec-ethz/scion-java-client/pull/33) - Fixed traffic class not set. [#36](https://github.com/netsec-ethz/scion-java-client/pull/36) +- Fixed handling of channel options. [#37](https://github.com/netsec-ethz/scion-java-client/pull/37) ### Removed diff --git a/README.md b/README.md index 815c6b571..e78484b4e 100644 --- a/README.md +++ b/README.md @@ -221,6 +221,15 @@ Options are defined in `ScionSocketOptions`, see javadoc for details. | `SN_API_WRITE_TO_USER_BUFFER` | `false` | Throw exception when receiving an invalid packet | | `SN_PATH_EXPIRY_MARGIN` | `2` | A new path is requested if `now + margin > pathExpirationDate` | +The following standard options are **not** supported: + +| Option | +|--------------------------------| +| `StandardSocketOptions.SO_BROADCAST` | +| `StandardSocketOptions.IP_MULTICAST_IF` | +| `StandardSocketOptions.IP_MULTICAST_TTL` | +| `StandardSocketOptions.IP_MULTICAST_LOOP` | + ## Performance pitfalls - **Using `SocketAddress` for `send()`**. `send(buffer, socketAddress)` is a convenience function. However, when sending @@ -248,8 +257,8 @@ attempt to get network information in the following order until it succeeds: - Check for DNS NAPTR record (if record entry name is given) - Check for to daemon -The reason that the daemon is checked last is that it has a default setting (localhost:30255) while -the other options are skipped if no property or environment variable is defined. +The reason that the daemon is checked last is that it has a default setting (`localhost:30255`) +while the other options are skipped if no property or environment variable is defined. | Option | Java property | Environment variable | Default value | |-------------------------------------|----------------------------------|------------------------------|---------------| diff --git a/TODO.md b/TODO.md index 7c9396892..668487d8b 100644 --- a/TODO.md +++ b/TODO.md @@ -108,7 +108,6 @@ - Multipathing: We probably ignore that for now. Multipathing can be done in many different ways, it may be difficult to design a one-size-fits-all API. E.g. "Hercules" uses a round-robin fashion with multiple path to fire UDP packets. -- MulticastSocket / MulticastChannel (?) - For Android look into - android.net.Network: diff --git a/src/main/java/org/scion/AbstractDatagramChannel.java b/src/main/java/org/scion/AbstractDatagramChannel.java index 3aa14802c..885b8e157 100644 --- a/src/main/java/org/scion/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/AbstractDatagramChannel.java @@ -390,6 +390,7 @@ public boolean isConnected() { @SuppressWarnings({"unchecked", "deprecation"}) public T getOption(SocketOption option) throws IOException { + checkOpen(); synchronized (stateLock) { if (option instanceof ScionSocketOptions.SciSocketOption) { if (ScionSocketOptions.SN_API_THROW_PARSER_FAILURE.equals(option)) { @@ -402,12 +403,17 @@ public T getOption(SocketOption option) throws IOException { throw new UnsupportedOperationException(); } } + + if (StandardSocketOptions.SO_BROADCAST.equals(option)) { + throw new UnsupportedOperationException(); + } return channel.getOption(option); } } @SuppressWarnings({"unchecked", "deprecation"}) public C setOption(SocketOption option, T t) throws IOException { + checkOpen(); synchronized (stateLock) { if (option instanceof ScionSocketOptions.SciSocketOption) { if (ScionSocketOptions.SN_API_THROW_PARSER_FAILURE.equals(option)) { @@ -424,6 +430,9 @@ public C setOption(SocketOption option, T t) throws IOException { throw new UnsupportedOperationException(); } } else { + if (StandardSocketOptions.SO_BROADCAST.equals(option)) { + throw new UnsupportedOperationException(); + } channel.setOption(option, t); } return (C) this; diff --git a/src/test/java/org/scion/api/DatagramChannelApiTest.java b/src/test/java/org/scion/api/DatagramChannelApiTest.java index e8d29cd23..163ee639b 100644 --- a/src/test/java/org/scion/api/DatagramChannelApiTest.java +++ b/src/test/java/org/scion/api/DatagramChannelApiTest.java @@ -605,7 +605,7 @@ void testBug_doubleSendCausesNPE() throws IOException { } @Test - void setOption() throws IOException { + void setOption_SCION() throws IOException { try (DatagramChannel channel = DatagramChannel.open()) { assertFalse(channel.getOption(ScionSocketOptions.SN_API_THROW_PARSER_FAILURE)); DatagramChannel dc = channel.setOption(ScionSocketOptions.SN_API_THROW_PARSER_FAILURE, true); @@ -618,6 +618,14 @@ void setOption() throws IOException { int tc = channel.getOption(ScionSocketOptions.SN_TRAFFIC_CLASS); channel.setOption(ScionSocketOptions.SN_TRAFFIC_CLASS, tc + 1); assertEquals(tc + 1, channel.getOption(ScionSocketOptions.SN_TRAFFIC_CLASS)); + + channel.close(); + assertThrows( + ClosedChannelException.class, + () -> channel.getOption(ScionSocketOptions.SN_PATH_EXPIRY_MARGIN)); + assertThrows( + ClosedChannelException.class, + () -> channel.setOption(ScionSocketOptions.SN_PATH_EXPIRY_MARGIN, 11)); } } @@ -650,4 +658,32 @@ void setOption_TrafficClass() throws IOException { channel.send(buf, dummyAddress); } } + + @Test + void setOption_Standard() throws IOException { + try (DatagramChannel channel = DatagramChannel.open()) { + DatagramChannel ds = channel.setOption(StandardSocketOptions.SO_RCVBUF, 10000); + assertEquals(channel, ds); + assertEquals(10000, channel.getOption(StandardSocketOptions.SO_RCVBUF)); + + channel.setOption(StandardSocketOptions.SO_SNDBUF, 10000); + assertEquals(10000, channel.getOption(StandardSocketOptions.SO_SNDBUF)); + + channel.setOption(StandardSocketOptions.SO_REUSEADDR, true); + assertTrue(channel.getOption(StandardSocketOptions.SO_REUSEADDR)); + + channel.setOption(StandardSocketOptions.IP_TOS, 5); + assertEquals(5, channel.getOption(StandardSocketOptions.IP_TOS)); + + assertThrows( + UnsupportedOperationException.class, + () -> channel.getOption(StandardSocketOptions.SO_BROADCAST)); + channel.close(); + assertThrows( + ClosedChannelException.class, () -> channel.getOption(StandardSocketOptions.SO_RCVBUF)); + assertThrows( + ClosedChannelException.class, + () -> channel.setOption(StandardSocketOptions.SO_RCVBUF, 10000)); + } + } }