return as expected but close the socket

* rejecting a connection by throwing an exception didn't work
  in practice, we got gigabytes of NullPointerException warnings
  and the configserver couldn't respond to requests anymore.
* instead, make the requested object normally, but check the
  SocketChannel and close it for traffic if it's not allowed
  before returning.  This should make the normal handling of
  closed connections do cleanup for us.
* Log any unexpected exceptions (since we're not quite sure
  where the NullPointerException came from).

1 files changed, 23 insertions, 10 deletions

diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
index 4c30e125d74..bababa9a25c 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
@@ -29,18 +29,31 @@ public class RestrictedServerCnxnFactory extends NIOServerCnxnFactory {
     
     @Override
     protected NIOServerCnxn createConnection(SocketChannel socket, SelectionKey selection) throws IOException {
-        ImmutableSet<String> allowedZooKeeperClients = findAllowedZooKeeperClients();
-        String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName();
+        NIOServerCnxn ret = super.createConnection(socket, selection);
+        validateRemoteOrClose(socket);
+        return ret;
+    }
+
+    private void validateRemoteOrClose(SocketChannel socket) {
+        try {
+            String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName();
+
+            if (isLocalHost(remoteHost)) return; // always allow localhost
 
-        if (isLocalHost(remoteHost)) return super.createConnection(socket, selection); // always allow localhost
-        if (allowedZooKeeperClients.isEmpty()) return super.createConnection(socket, selection); // inactive: allow all
-        if (allowedZooKeeperClients.contains(remoteHost)) return super.createConnection(socket, selection); // allowed
+            ImmutableSet<String> allowedZooKeeperClients = findAllowedZooKeeperClients();
 
-        // Not allowed: Reject connection
-        String errorMessage = "Rejecting connection to ZooKeeper from " + remoteHost +
-                              ": This cluster only allow connection from hosts in: " + allowedZooKeeperClients;
-        log.info(errorMessage);
-        throw new IllegalArgumentException(errorMessage); // log and throw as this exception will be suppressed by zk
+            if (allowedZooKeeperClients.isEmpty()) return; // inactive: allow all
+            if (allowedZooKeeperClients.contains(remoteHost)) return; // allowed
+
+            // Not allowed: Reject connection
+            String errorMessage = "Rejecting connection to ZooKeeper from " + remoteHost +
+                                  ": This cluster only allow connection from hosts in: " + allowedZooKeeperClients;
+            log.info(errorMessage);
+            socket.shutdownInput();
+            socket.shutdownOutput();
+        } catch (Exception e) {
+            log.warning("Unexpected exception: "+e);
+        }
     }
 
     /** Returns the allowed client host names. If the list is empty any host is allowed. */