Hosts file parser cleanup (#42)

* Hosts file parser cleanup

---------

Co-authored-by: Tilmann Zäschke <tilmann.zaeschke@inf.ethz.ch>

CHANGELOG.md

@@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fixed handling of channel options. [#37](https://github.com/netsec-ethz/scion-java-client/pull/37)
 - Merged SCION_DAEMON_HOST and SCION_DAEMON_PORT into a single SCION_DAEMON property.
   [#39](https://github.com/netsec-ethz/scion-java-client/pull/39)
+- Some cleanup related to hists file parser. [#42](https://github.com/netsec-ethz/scion-java-client/pull/42)
 
 ### Removed
 

pom.xml

@@ -314,6 +314,19 @@
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-source-plugin</artifactId>
+                <version>3.3.0</version>
+                <executions>
+                    <execution>
+                        <id>attach-sources</id>
+                        <goals>
+                            <goal>jar-no-fork</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
         </plugins>
     </build>
 
@@ -359,19 +372,6 @@
                             </execution>
                         </executions>
                     </plugin>
-                    <plugin>
-                        <groupId>org.apache.maven.plugins</groupId>
-                        <artifactId>maven-source-plugin</artifactId>
-                        <version>3.3.0</version>
-                        <executions>
-                            <execution>
-                                <id>attach-sources</id>
-                                <goals>
-                                    <goal>jar-no-fork</goal>
-                                </goals>
-                            </execution>
-                        </executions>
-                    </plugin>
                     <plugin>
                         <groupId>org.apache.maven.plugins</groupId>
                         <artifactId>maven-javadoc-plugin</artifactId>

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

@@ -224,9 +224,9 @@ public C connect(SocketAddress addr) throws IOException {
    * - The path will be replaced with a new path once it is expired.<br>
    *
    * <p>NB: This method does internally not call {@link
-   * java.nio.channels.DatagramChannel}.connect(). That means this method does NOT perform any
-   * additional security checks associated with connect(). It will however perform a `bind(null)`
-   * unless the channel is already bound.
+   * java.nio.channels.DatagramChannel#connect(SocketAddress)}. That means this method does NOT
+   * perform any additional security checks associated with connect(). It will however perform a
+   * `bind(null)` unless the channel is already bound.
    *
    * <p>"connect()" is understood to provide connect to a destination address (IP+port).<br>
    * - send()ing packet to another destination will cause an Exception.<br>
@@ -248,7 +248,7 @@ public C connect(RequestPath path) throws IOException {
     // - connect() and bind() have lock conflict with concurrent call to receiver()
     // - connect() after bind() is fine, but it changes the local address from ANY to specific IF
 
-    // We have two manage two connection states, internal (state of the internallly used channel)
+    // We have two manage two connection states, internal (state of the internally used channel)
     // and external (as reported to API users).
 
     // Externally, for an API user:

src/main/java/org/scion/ScionAddress.java

@@ -14,8 +14,6 @@
 
 package org.scion;
 
-import java.net.Inet4Address;
-import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 
@@ -35,20 +33,18 @@ private ScionAddress(long isdAs, String hostName, InetAddress ip) {
     this.isdAs = isdAs;
   }
 
+  static ScionAddress create(long isdAs, InetAddress address) {
+    return new ScionAddress(isdAs, address.getHostName(), address);
+  }
+
   static ScionAddress create(long isdAs, String hostName, String ipString) {
-    InetAddress ip;
     try {
-      if (ipString.indexOf('.') > 0) {
-        ip = Inet4Address.getByName(ipString);
-      } else {
-        // Must be IPv6 or invalid
-        ip = Inet6Address.getByName(ipString);
-      }
+      InetAddress ip = InetAddress.getByName(ipString);
+      return new ScionAddress(isdAs, hostName, ip);
     } catch (UnknownHostException e) {
       // This should never happen because we always call getByName() with an IP address
-      throw new RuntimeException(e);
+      throw new ScionRuntimeException(e);
     }
-    return new ScionAddress(isdAs, hostName, ip);
   }
 
   public long getIsdAs() {

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

@@ -418,6 +418,12 @@ public ScionAddress getScionAddress(String hostName) throws ScionException {
       return address;
     }
 
+    // Check /etc/scion/hosts
+    HostsFileParser.HostEntry entry = hostsFile.find(hostName);
+    if (entry != null) {
+      return ScionAddress.create(entry.getIsdAs(), entry.getAddress());
+    }
+
     // Use local ISD/AS for localhost addresses
     if (isLocalhost(hostName)) {
       return ScionAddress.create(getLocalIsdAs(), hostName, hostName);

src/main/java/org/scion/internal/DNSHelper.java

@@ -39,6 +39,8 @@ public class DNSHelper {
   private static final String ERR_PARSING_TXT_LOG = ERR_PARSING_TXT + "{}";
   private static final String ERR_PARSING_TXT_LOG2 = ERR_PARSING_TXT + "{} {}";
 
+  private DNSHelper() {}
+
   /**
    * Perform a DNS lookup on "hostName" for a TXT entry with key "key". All matching entries are
    * forwarded to the "valueParser" until the "valueParser" returns not "null".
@@ -51,73 +53,70 @@ public class DNSHelper {
    * @param <R> Result type.
    */
   public static <R> R queryTXT(String hostName, String key, Function<String, R> valueParser) {
-
+    String nameStr = hostName.endsWith(".") ? hostName : hostName + ".";
     try {
-      Record[] records = new Lookup(hostName, Type.TXT).run();
-      if (records == null) {
-        return null;
-      }
-      for (int i = 0; i < records.length; i++) {
-        TXTRecord txt = (TXTRecord) records[i];
-        String entry = txt.rdataToString();
-        if (entry.startsWith("\"" + key + "=")) {
-          if (entry.endsWith("\"")) {
-            String data = entry.substring(key.length() + 2, entry.length() - 1);
-            R result = valueParser.apply(data);
-            if (result != null) {
-              return result;
-            }
+      return queryTXT(Name.fromString(nameStr), key, valueParser);
+    } catch (TextParseException e) {
+      LOG.info(ERR_PARSING_TXT_LOG, e.getMessage());
+    }
+    return null;
+  }
+
+  public static <R> R queryTXT(Name name, String key, Function<String, R> valueParser) {
+    Record[] records = new Lookup(name, Type.TXT).run();
+    if (records == null) {
+      return null;
+    }
+    for (int i = 0; i < records.length; i++) {
+      TXTRecord txt = (TXTRecord) records[i];
+      String entry = txt.rdataToString();
+      if (entry.startsWith("\"" + key + "=")) {
+        if (entry.endsWith("\"")) {
+          String data = entry.substring(key.length() + 2, entry.length() - 1);
+          R result = valueParser.apply(data);
+          if (result != null) {
+            return result;
           }
-          LOG.info(ERR_PARSING_TXT_LOG, entry);
         }
+        LOG.info(ERR_PARSING_TXT_LOG, entry);
       }
-    } catch (TextParseException e) {
-      LOG.info(ERR_PARSING_TXT_LOG, e.getMessage());
     }
     return null;
   }
 
-  public static InetAddress queryA(String hostName) throws IOException {
+  public static InetAddress queryA(Name hostName) {
     Record[] recordsA = new Lookup(hostName, Type.A).run();
     if (recordsA == null) {
       throw new ScionRuntimeException("No DNS A entry found for host: " + hostName);
     }
-    for (int i = 0; i < recordsA.length; i++) {
-      ARecord ar = (ARecord) recordsA[i];
-      // TODO just return the first one for now
-      return ar.getAddress();
-    }
-    return null;
+    // just return the first one for now
+    return ((ARecord) recordsA[0]).getAddress();
   }
 
-  public static InetAddress queryAAAA(String hostName) throws IOException {
+  public static InetAddress queryAAAA(Name hostName) {
     Record[] recordsA = new Lookup(hostName, Type.AAAA).run();
     if (recordsA == null) {
       throw new ScionRuntimeException("No DNS AAAA entry found for host: " + hostName);
     }
-    for (int i = 0; i < recordsA.length; i++) {
-      AAAARecord ar = (AAAARecord) recordsA[i];
-      // TODO just return the first one for now
-      return ar.getAddress();
-    }
-    return null;
+    // just return the first one for now
+    return ((AAAARecord) recordsA[0]).getAddress();
   }
 
   public static String searchForDiscoveryService() {
-    try {
-      for (Name n : Lookup.getDefaultSearchPath()) {
-        String a = getScionDiscoveryAddress(n.toString());
-        if (a != null) {
-          return a;
-        }
+    for (Name domain : Lookup.getDefaultSearchPath()) {
+      String a = getScionDiscoveryAddress(domain);
+      if (a != null) {
+        return a;
       }
-      return null;
-    } catch (IOException e) {
-      throw new ScionRuntimeException("Error looking up NAPTR records from OS DNS search paths", e);
     }
+    return null;
   }
 
   public static String getScionDiscoveryAddress(String hostName) throws IOException {
+    return getScionDiscoveryAddress(Name.fromString(hostName));
+  }
+
+  private static String getScionDiscoveryAddress(Name hostName) {
     Record[] records = new Lookup(hostName, Type.NAPTR).run();
     if (records == null) {
       return null;
@@ -127,23 +126,22 @@ public static String getScionDiscoveryAddress(String hostName) throws IOExceptio
       NAPTRRecord nr = (NAPTRRecord) records[i];
       String naptrService = nr.getService();
       if (STR_X_SCION_TCP.equals(naptrService)) {
-        String host = nr.getReplacement().toString();
         String naptrFlag = nr.getFlags();
         int port = getScionDiscoveryPort(hostName);
         if ("A".equals(naptrFlag)) {
-          InetAddress addr = DNSHelper.queryA(host);
+          InetAddress addr = DNSHelper.queryA(nr.getReplacement());
           return addr.getHostAddress() + ":" + port;
         }
         if ("AAAA".equals(naptrFlag)) {
-          InetAddress addr = DNSHelper.queryAAAA(host);
+          InetAddress addr = DNSHelper.queryAAAA(nr.getReplacement());
           return "[" + addr.getHostAddress() + "]:" + port;
         } // keep going and collect more hints
       }
     }
     return null;
   }
 
-  private static int getScionDiscoveryPort(String hostName) {
+  private static int getScionDiscoveryPort(Name hostName) {
     Integer discoveryPort =
         DNSHelper.queryTXT(
             hostName,

src/main/java/org/scion/internal/HostsFileParser.java

@@ -41,18 +41,26 @@ public class HostsFileParser {
 
   public static class HostEntry {
     private final long isdAs;
-    private final String[] hostNames;
+    private final String hostName;
     private final InetAddress address;
 
-    HostEntry(long isdAs, InetAddress address, String[] hostNames) {
+    HostEntry(long isdAs, InetAddress address, String hostName) {
       this.isdAs = isdAs;
-      this.hostNames = hostNames;
+      this.hostName = hostName;
       this.address = address;
     }
 
     public long getIsdAs() {
       return isdAs;
     }
+
+    public String getHostName() {
+      return hostName;
+    }
+
+    public InetAddress getAddress() {
+      return address;
+    }
   }
 
   public HostsFileParser() {
@@ -98,21 +106,23 @@ private void parseLine(String line) {
       long isdIa = ScionUtil.parseIA(addrParts[0]);
       check(addrParts[1].startsWith("["), "Expected `[` before address");
       check(addrParts[1].endsWith("]"), "Expected `]` after address");
-      String addr = addrParts[1].substring(1, addrParts[1].length() - 1).trim();
-      check(!addr.isEmpty(), "Address is empty");
+      String addrStr = addrParts[1].substring(1, addrParts[1].length() - 1).trim();
+      check(!addrStr.isEmpty(), "Address is empty");
       // We allow anything here, even host names (which will trigger a DNS lookup).
       // Is that ok?
-      InetAddress inetAddress = InetAddress.getByName(addr);
+      InetAddress inetAddr = InetAddress.getByName(addrStr);
+
+      // TODO
+      // 4) Singleton ?!?!?!?!!!
 
       String[] hostNames = Arrays.copyOfRange(lineParts, 1, lineParts.length);
-      HostEntry e = new HostEntry(isdIa, inetAddress, hostNames);
       for (String hostName : hostNames) {
-        entries.put(hostName, e);
+        entries.put(hostName, new HostEntry(isdIa, inetAddr, hostName));
       }
       // The following may differ, e.g. for IPv6
       // TODO find a better way, i.e. use InetAddress instances as keys?
-      entries.put(e.address.getHostName(), e);
-      entries.put(addr, e);
+      entries.put(inetAddr.getHostName(), new HostEntry(isdIa, inetAddr, inetAddr.getHostName()));
+      entries.put(addrStr, new HostEntry(isdIa, inetAddr, addrStr));
     } catch (IndexOutOfBoundsException | IllegalArgumentException | UnknownHostException e) {
       LOG.info("ERROR {} while parsing file {}: {}", e.getMessage(), PATH_LINUX, line);
     }

src/test/java/org/scion/demo/PingPongDemoTest.java

@@ -31,9 +31,9 @@ public static void beforeAll() {
   @Test
   void test() throws InterruptedException {
     AtomicInteger failures = new AtomicInteger();
-    PingPongChannelServer.PRINT = true;
+    PingPongChannelServer.PRINT = false;
     PingPongChannelServer.NETWORK = DemoConstants.Network.MOCK_TOPOLOGY_IPV4;
-    PingPongChannelClient.PRINT = true;
+    PingPongChannelClient.PRINT = false;
     PingPongChannelClient.NETWORK = DemoConstants.Network.MOCK_TOPOLOGY_IPV4;
     Thread server =
         new Thread(