Internal cleanup

CHANGELOG.md

@@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - SCMP API changes. [#71](https://github.com/scionproto-contrib/jpan/pull/71)
 - **BREAKING CHANGE**: `DatagramChannel.receive()` returns a subclass of `InetSocketAddress` 
   [#86](https://github.com/scionproto-contrib/jpan/pull/86) 
+- Internal cleanup. [#88](https://github.com/scionproto-contrib/jpan/pull/88)
 
 ### Fixed
 - Fixed locking and resizing of buffers. [#68](https://github.com/scionproto-contrib/jpan/pull/68)

src/main/java/org/scion/jpan/AbstractDatagramChannel.java

@@ -179,8 +179,10 @@ public InetSocketAddress getLocalAddress() throws IOException {
   /**
    * Returns the remote address.
    *
+   * @return The remote address or 'null' if this channel is not connected.
    * @see DatagramChannel#getRemoteAddress()
-   * @return The remote address.
+   * @see #connect(SocketAddress)
+   * @see #connect(RequestPath)
    * @throws IOException If an I/O error occurs
    */
   public InetSocketAddress getRemoteAddress() throws IOException {
@@ -193,7 +195,6 @@ public InetSocketAddress getRemoteAddress() throws IOException {
 
   public void disconnect() throws IOException {
     synchronized (stateLock) {
-      channel.disconnect(); // TODO Why ? We shouldn´t do that...?
       connectionPath = null;
     }
   }
@@ -214,12 +215,15 @@ public void close() throws IOException {
   }
 
   /**
-   * Connect to a destination host. Note: - A SCION channel will internally connect to the next
-   * border router (first hop) instead of the remote host.
+   * Connect to a destination host.
    *
-   * <p>NB: This method does internally no call {@link java.nio.channels.DatagramChannel}.connect(),
-   * instead it calls bind(). That means this method does NOT perform any additional security checks
-   * associated with connect(), only those associated with bind().
+   * <p>NB: A SCION channel will internally connect to the next border router (first hop) instead of
+   * the remote host.
+   *
+   * <p>NB: This method does internally not call {@link
+   * java.nio.channels.DatagramChannel}.connect(), instead it calls bind(). That means this method
+   * does NOT perform any additional security checks associated with connect(), only those
+   * associated with bind().
    *
    * @param addr Address of remote host.
    * @return This channel.
@@ -301,7 +305,7 @@ public C connect(RequestPath path) throws IOException {
    *
    * @return the current Path or `null` if not path is connected.
    */
-  public Path getConnectionPath() {
+  public RequestPath getConnectionPath() {
     synchronized (stateLock) {
       return connectionPath;
     }
@@ -398,17 +402,12 @@ public void setOverrideSourceAddress(InetSocketAddress address) {
     this.overrideExternalAddress = address;
   }
 
-  protected int sendRaw(ByteBuffer buffer, InetSocketAddress address, Path path)
-      throws IOException {
-    if (cfgRemoteDispatcher && path != null && path.getRawPath().length == 0) {
-      return channel.send(
-          buffer, new InetSocketAddress(address.getAddress(), Constants.DISPATCHER_PORT));
+  protected int sendRaw(ByteBuffer buffer, Path path) throws IOException {
+    if (cfgRemoteDispatcher && path.getRawPath().length == 0) {
+      InetAddress remoteHostIP = path.getFirstHopAddress().getAddress();
+      return channel.send(buffer, new InetSocketAddress(remoteHostIP, Constants.DISPATCHER_PORT));
     }
-    return channel.send(buffer, address);
-  }
-
-  protected int sendRaw(ByteBuffer buffer, InetSocketAddress address) throws IOException {
-    return sendRaw(buffer, address, null);
+    return channel.send(buffer, path.getFirstHopAddress());
   }
 
   public Consumer<Scmp.Message> setScmpErrorListener(Consumer<Scmp.Message> listener) {

src/main/java/org/scion/jpan/Path.java

@@ -55,15 +55,21 @@ public long getRemoteIsdAs() {
 
   @Override
   public String toString() {
-    return "Path{"
-        + "rmtIsdAs="
-        + ScionUtil.toStringIA(dstIsdAs)
-        + ", rmtAddress="
-        + dstAddress
-        + ", rmtPort="
-        + dstPort
-        + ", pathRaw="
-        + Arrays.toString(pathRaw)
-        + '}';
+    try {
+      return "Path{"
+          + "rmtIsdAs="
+          + ScionUtil.toStringIA(dstIsdAs)
+          + ", rmtAddress="
+          + dstAddress
+          + ", rmtPort="
+          + dstPort
+          + ", firstHop="
+          + getFirstHopAddress()
+          + ", pathRaw="
+          + Arrays.toString(getRawPath())
+          + '}';
+    } catch (UnknownHostException e) {
+      throw new ScionRuntimeException(e);
+    }
   }
 }

src/main/java/org/scion/jpan/ResponsePath.java

@@ -22,6 +22,8 @@
  * ISD/AS, IP and port of the local host. This is mostly for convenience to avoid looking up this
  * information, but it also ensures that the return packet header contains the exact information
  * sent/expected by the client.
+ *
+ * <p>A ResponsePath is immutable and thus thread safe.
  */
 public class ResponsePath extends Path {
 
@@ -80,15 +82,14 @@ public int getLocalPort() {
   @Override
   public String toString() {
     return "ResponsePath{"
-        + super.toString()
-        + ", firstHopAddress="
-        + firstHopAddress
         + ", localIsdAs="
         + srcIsdAs
         + ", localAddress="
         + srcAddress
         + ", localPort="
         + srcPort
+        + ", "
+        + super.toString()
         + '}';
   }
 }

src/main/java/org/scion/jpan/ScionDatagramChannel.java

@@ -101,7 +101,7 @@ public void send(ByteBuffer srcBuffer, SocketAddress destination) throws IOExcep
    * Attempts to send the content of the buffer to the destinationAddress.
    *
    * @param srcBuffer Data to send
-   * @param path Path to destination. If this is a Request path and it is expired then it will
+   * @param path Path to destination. If this is a RequestPath, and it is expired, then it will
    *     automatically be replaced with a new path. Expiration of ResponsePaths is not checked
    * @return either the path argument or a new path if the path was an expired RequestPath. Note
    *     that ResponsePaths are not checked for expiration.
@@ -123,7 +123,7 @@ public Path send(ByteBuffer srcBuffer, Path path) throws IOException {
         throw new IOException("Packet is larger than max send buffer size.");
       }
       buffer.flip();
-      sendRaw(buffer, actualPath.getFirstHopAddress(), actualPath);
+      sendRaw(buffer, actualPath);
       return actualPath;
     } finally {
       writeLock().unlock();
@@ -169,15 +169,16 @@ public int write(ByteBuffer src) throws IOException {
     try {
       checkOpen();
       checkConnected(true);
+      Path path = getConnectionPath();
 
       ByteBuffer buffer = getBufferSend(src.remaining());
       int len = src.remaining();
       // + 8 for UDP overlay header length
-      checkPathAndBuildHeader(buffer, getConnectionPath(), len + 8, InternalConstants.HdrTypes.UDP);
+      checkPathAndBuildHeader(buffer, path, len + 8, InternalConstants.HdrTypes.UDP);
       buffer.put(src);
       buffer.flip();
 
-      int sent = sendRaw(buffer, getConnectionPath().getFirstHopAddress(), getConnectionPath());
+      int sent = sendRaw(buffer, path);
       if (sent < buffer.limit() || buffer.remaining() > 0) {
         throw new ScionException("Failed to send all data.");
       }

src/main/java/org/scion/jpan/ScionDatagramSocket.java

@@ -535,7 +535,7 @@ public void leaveGroup(SocketAddress mcastaddr, NetworkInterface netIf) {
    * @see ScionDatagramChannel#getConnectionPath()
    */
   public RequestPath getConnectionPath() {
-    return (RequestPath) channel.getConnectionPath();
+    return channel.getConnectionPath();
   }
 
   /**

src/main/java/org/scion/jpan/ScionService.java

@@ -377,7 +377,7 @@ public long getLocalIsdAs() {
 
   /**
    * @param hostName hostName of the host to resolve
-   * @return A ScionAddress
+   * @return The ISD/AS code for a hostname
    * @throws ScionException if the DNS/TXT lookup did not return a (valid) SCION address.
    */
   public long getIsdAs(String hostName) throws ScionException {

src/main/java/org/scion/jpan/ScmpChannel.java

@@ -229,7 +229,7 @@ Scmp.TimedMessage sendEchoRequest(Scmp.EchoMessage request) throws IOException {
             buffer, Scmp.Type.INFO_128, localPort, request.getSequenceNumber(), request.getData());
         buffer.flip();
         request.setSizeSent(buffer.remaining());
-        sendRaw(buffer, path.getFirstHopAddress());
+        sendRaw(buffer, path);
 
         int sizeReceived = receive(request);
         request.setSizeReceived(sizeReceived);
@@ -261,7 +261,7 @@ Scmp.TimedMessage sendTracerouteRequest(
         int posPath = ScionHeaderParser.extractPathHeaderPosition(buffer);
         buffer.put(posPath + node.posHopFlags, node.hopFlags);
 
-        sendRaw(buffer, path.getFirstHopAddress());
+        sendRaw(buffer, path);
 
         receive(request);
         return request;
@@ -356,7 +356,7 @@ void sendEchoResponses() throws IOException {
                 buffer, Scmp.Type.INFO_129, port, msg.getSequenceNumber(), msg.getData());
             buffer.flip();
             msg.setSizeSent(buffer.remaining());
-            sendRaw(buffer, path.getFirstHopAddress());
+            sendRaw(buffer, path);
             log.info("Responded to SCMP {} from {}", type, path.getRemoteAddress());
           } else {
             log.info("Dropped SCMP message with type {} from {}", type, path.getRemoteAddress());

src/test/java/org/scion/jpan/api/DatagramChannelApiServerTest.java

@@ -85,9 +85,8 @@ void send_withoutService() throws IOException {
               new byte[4],
               1,
               new InetSocketAddress("127.0.0.1", 1));
-      Path p2 = channel.send(ByteBuffer.allocate(0), path);
+      channel.send(ByteBuffer.allocate(0), path);
       assertNull(channel.getService());
-      assertEquals(path, p2);
     }
   }
 

src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java

@@ -509,7 +509,7 @@ void write_expiredRequestPath() throws IOException {
           ByteBuffer sendBuf = ByteBuffer.wrap(PingPongChannelHelper.MSG.getBytes());
           try {
             channel.write(sendBuf);
-            RequestPath newPath = (RequestPath) channel.getConnectionPath();
+            RequestPath newPath = channel.getConnectionPath();
             assertTrue(newPath.getExpiration() > expiredPath.getExpiration());
             assertTrue(Instant.now().getEpochSecond() < newPath.getExpiration());
           } catch (IOException e) {

src/test/java/org/scion/jpan/demo/PingPongChannelClient.java

@@ -17,7 +17,6 @@
 import java.io.IOException;
 import java.net.*;
 import java.nio.ByteBuffer;
-import org.scion.jpan.RequestPath;
 import org.scion.jpan.ScionDatagramChannel;
 import org.scion.jpan.ScionUtil;
 import org.scion.jpan.testutil.MockDNS;
@@ -51,11 +50,7 @@ private static void run() throws IOException {
       String msg = "Hello there!";
       ByteBuffer sendBuf = ByteBuffer.wrap(msg.getBytes());
       channel.write(sendBuf);
-      println(
-          "Sent via "
-              + ScionUtil.toStringPath((RequestPath) channel.getConnectionPath())
-              + ": "
-              + msg);
+      println("Sent via " + ScionUtil.toStringPath(channel.getConnectionPath()) + ": " + msg);
 
       println("Receiving ... (" + channel.getLocalAddress() + ")");
       ByteBuffer buffer = ByteBuffer.allocate(512);