444214 - Socks4Proxy fails when reading less than 8 bytes.

2 files changed, 123 insertions, 12 deletions

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java b/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java
index 6886ef1aa2..de40538f7b 100644
--- a/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java
+++ b/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java
@@ -79,6 +79,7 @@ public class Socks4Proxy extends ProxyConfiguration.Proxy
         private static final Pattern IPv4_PATTERN = Pattern.compile("(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})");
         private static final Logger LOG = Log.getLogger(Socks4ProxyConnection.class);
 
+        private final Socks4Parser parser = new Socks4Parser();
         private final ClientConnectionFactory connectionFactory;
         private final Map<String, Object> context;
 
@@ -152,17 +153,27 @@ public class Socks4Proxy extends ProxyConfiguration.Proxy
         {
             try
             {
-                ByteBuffer buffer = BufferUtil.allocate(8);
-                int filled = getEndPoint().fill(buffer);
-                if (LOG.isDebugEnabled())
-                    LOG.debug("Read SOCKS4 connect response, {} bytes", filled);
-                if (filled != 8)
-                    throw new IOException("Invalid response from SOCKS4 proxy");
-                int result = buffer.get(1);
-                if (result == 0x5A)
-                    tunnel();
-                else
-                    throw new IOException("SOCKS4 tunnel failed with code " + result);
+                while (true)
+                {
+                    // Avoid to read too much from the socket: ask
+                    // the parser how much left there is to read.
+                    ByteBuffer buffer = BufferUtil.allocate(parser.expected());
+                    int filled = getEndPoint().fill(buffer);
+                    if (LOG.isDebugEnabled())
+                        LOG.debug("Read SOCKS4 connect response, {} bytes", filled);
+
+                    if (filled < 0)
+                        throw new IOException("SOCKS4 tunnel failed, connection closed");
+
+                    if (filled == 0)
+                    {
+                        fillInterested();
+                        return;
+                    }
+
+                    if (parser.parse(buffer))
+                        return;
+                }
             }
             catch (Throwable x)
             {
@@ -170,6 +181,14 @@ public class Socks4Proxy extends ProxyConfiguration.Proxy
             }
         }
 
+        private void onSocks4Response(int responseCode) throws IOException
+        {
+            if (responseCode == 0x5A)
+                tunnel();
+            else
+                throw new IOException("SOCKS4 tunnel failed with code " + responseCode);
+        }
+
         private void tunnel()
         {
             try
@@ -189,5 +208,34 @@ public class Socks4Proxy extends ProxyConfiguration.Proxy
                 failed(x);
             }
         }
+
+        private class Socks4Parser
+        {
+            private static final int EXPECTED_LENGTH = 8;
+            private int cursor;
+            private int response;
+
+            private boolean parse(ByteBuffer buffer) throws IOException
+            {
+                while (buffer.hasRemaining())
+                {
+                    byte current = buffer.get();
+                    if (cursor == 1)
+                        response = current & 0xFF;
+                    ++cursor;
+                    if (cursor == EXPECTED_LENGTH)
+                    {
+                        onSocks4Response(response);
+                        return true;
+                    }
+                }
+                return false;
+            }
+
+            private int expected()
+            {
+                return EXPECTED_LENGTH - cursor;
+            }
+        }
     }
 }
diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/Socks4ProxyTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/Socks4ProxyTest.java
index 6711061e6a..1d8a17900b 100644
--- a/jetty-client/src/test/java/org/eclipse/jetty/client/Socks4ProxyTest.java
+++ b/jetty-client/src/test/java/org/eclipse/jetty/client/Socks4ProxyTest.java
@@ -51,8 +51,8 @@ public class Socks4ProxyTest
     @After
     public void dispose() throws Exception
     {
-        server.close();
         client.stop();
+        server.close();
     }
 
     @Test
@@ -115,5 +115,68 @@ public class Socks4ProxyTest
         channel.write(ByteBuffer.wrap(response.getBytes("UTF-8")));
 
         Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
+
+        channel.close();
+    }
+
+    @Test
+    public void testSocks4ProxyWithSplitResponse() throws Exception
+    {
+        int proxyPort = server.socket().getLocalPort();
+        client.getProxyConfiguration().getProxies().add(new Socks4Proxy("localhost", proxyPort));
+
+        final CountDownLatch latch = new CountDownLatch(1);
+
+        String serverHost = "127.0.0.13"; // Test expects an IP address.
+        int serverPort = proxyPort + 1; // Any port will do
+        client.newRequest(serverHost, serverPort)
+                .path("/path")
+                .timeout(5, TimeUnit.SECONDS)
+                .send(new Response.CompleteListener()
+                {
+                    @Override
+                    public void onComplete(Result result)
+                    {
+                        if (result.isSucceeded())
+                            latch.countDown();
+                        else
+                            result.getFailure().printStackTrace();
+                    }
+                });
+
+        SocketChannel channel = server.accept();
+
+        int socks4MessageLength = 9;
+        ByteBuffer buffer = ByteBuffer.allocate(socks4MessageLength);
+        int read = channel.read(buffer);
+        Assert.assertEquals(socks4MessageLength, read);
+
+        // Socks4 response, with split bytes.
+        byte[] chunk1 = new byte[]{0, 0x5A, 0};
+        byte[] chunk2 = new byte[]{0, 0, 0, 0, 0};
+        channel.write(ByteBuffer.wrap(chunk1));
+
+        // Wait before sending the second chunk.
+        Thread.sleep(1000);
+
+        channel.write(ByteBuffer.wrap(chunk2));
+
+        buffer = ByteBuffer.allocate(3);
+        read = channel.read(buffer);
+        Assert.assertEquals(3, read);
+        buffer.flip();
+        Assert.assertEquals("GET", StandardCharsets.UTF_8.decode(buffer).toString());
+
+        // Response
+        String response = "" +
+                "HTTP/1.1 200 OK\r\n" +
+                "Content-Length: 0\r\n" +
+                "Connection: close\r\n" +
+                "\r\n";
+        channel.write(ByteBuffer.wrap(response.getBytes("UTF-8")));
+
+        Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
+
+        channel.close();
     }
 }