From c3d3edd6c0b22bfaaa249c3d4f03186a81e458e3 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 16 Apr 2014 17:33:22 +0200 Subject: [PATCH 1/2] 432777 - Async Write Loses Data with HTTPS Server. Fixed by properly flipping the aggregate buffer when copying bytes to it. --- .../org/eclipse/jetty/server/HttpOutput.java | 3 +- .../jetty/servlet/SSLAsyncIOServletTest.java | 179 ++++++++++++++++++ jetty-servlet/src/test/resources/keystore.jks | Bin 0 -> 2206 bytes .../src/test/resources/truststore.jks | Bin 0 -> 916 bytes 4 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 jetty-servlet/src/test/java/org/eclipse/jetty/servlet/SSLAsyncIOServletTest.java create mode 100644 jetty-servlet/src/test/resources/keystore.jks create mode 100644 jetty-servlet/src/test/resources/truststore.jks diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index 3379981c827..610db58cbf9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -24,7 +24,6 @@ import java.nio.ByteBuffer; import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritePendingException; import java.util.concurrent.atomic.AtomicReference; - import javax.servlet.RequestDispatcher; import javax.servlet.ServletOutputStream; import javax.servlet.ServletRequest; @@ -885,7 +884,9 @@ public class HttpOutput extends ServletOutputStream implements Runnable // Can we just aggregate the remainder? if (!_complete && _len parameters() + { + return Arrays.asList(new SslContextFactory[]{null}, new SslContextFactory[]{new SslContextFactory()}); + } + + private Server server; + private ServerConnector connector; + private SslContextFactory sslContextFactory; + private String contextPath; + private String servletPath; + + public SSLAsyncIOServletTest(SslContextFactory sslContextFactory) + { + this.sslContextFactory = sslContextFactory; + if (sslContextFactory != null) + { + sslContextFactory.setEndpointIdentificationAlgorithm(""); + sslContextFactory.setKeyStorePath("src/test/resources/keystore.jks"); + sslContextFactory.setKeyStorePassword("storepwd"); + sslContextFactory.setTrustStorePath("src/test/resources/truststore.jks"); + sslContextFactory.setTrustStorePassword("storepwd"); + } + } + + public void prepare(HttpServlet servlet) throws Exception + { + server = new Server(); + + connector = new ServerConnector(server, sslContextFactory); + server.addConnector(connector); + + contextPath = "/context"; + ServletContextHandler context = new ServletContextHandler(server, contextPath, true, false); + servletPath = "/servlet"; + context.addServlet(new ServletHolder(servlet), servletPath); + + server.start(); + } + + @After + public void dispose() throws Exception + { + server.stop(); + } + + @Test + public void testAsyncIOWritesWithAggregation() throws Exception + { + Random random = new Random(); + String chars = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"; + final byte[] content = new byte[50000]; + for (int i = 0; i < content.length; ++i) + content[i] = (byte)chars.charAt(random.nextInt(chars.length())); + + prepare(new HttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + final AsyncContext asyncContext = request.startAsync(); + asyncContext.setTimeout(0); + final int bufferSize = 4096; + response.setBufferSize(bufferSize); + response.getOutputStream().setWriteListener(new WriteListener() + { + private int writes; + private int written; + + @Override + public void onWritePossible() throws IOException + { + ServletOutputStream output = asyncContext.getResponse().getOutputStream(); + do + { + int toWrite = content.length - written; + if (toWrite == 0) + { + asyncContext.complete(); + return; + } + + toWrite = Math.min(toWrite, bufferSize); + + // Perform a write that is smaller than the buffer size to + // trigger the condition where the bytes are aggregated. + if (writes == 1) + toWrite -= 16; + + output.write(content, written, toWrite); + ++writes; + written += toWrite; + } + while (output.isReady()); + } + + @Override + public void onError(Throwable t) + { + asyncContext.complete(); + } + }); + } + }); + + try (Socket client = newClient()) + { + String request = "" + + "GET " + contextPath + servletPath + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "\r\n"; + OutputStream output = client.getOutputStream(); + output.write(request.getBytes("UTF-8")); + output.flush(); + + BufferedReader input = new BufferedReader(new InputStreamReader(client.getInputStream(), "UTF-8")); + SimpleHttpParser parser = new SimpleHttpParser(); + SimpleHttpResponse response = parser.readResponse(input); + Assert.assertEquals("200", response.getCode()); + Assert.assertArrayEquals(content, response.getBody().getBytes("UTF-8")); + } + } + + private Socket newClient() throws IOException + { + return sslContextFactory == null ? new Socket("localhost", connector.getLocalPort()) + : sslContextFactory.getSslContext().getSocketFactory().createSocket("localhost", connector.getLocalPort()); + } +} diff --git a/jetty-servlet/src/test/resources/keystore.jks b/jetty-servlet/src/test/resources/keystore.jks new file mode 100644 index 0000000000000000000000000000000000000000..428ba54776ede2fdcdeedd879edb927c2abd9953 GIT binary patch literal 2206 zcmcgt`9Bkm8{cNkoMUp6gmShKn!AQX*(l6Nj(i=TnQPOKYtv{*Wg>ItE=Q!pRYH8a z$Sp#S#2lYw#aw;$y9u4T}83H*%lp zAKZay0sy=q1Qoo85aAQh;$ zD(c2EIN#D7WwYDLKUg!CotQPD@dp;5FR#bgaace(^x$6g5frD~(_b(MI^J&*A2DRp zf5Q2onfE(zvUb9|9C`66)YFRNM6~xrz4;iVbU=P|*YT2eWHFJJtr+M@zt2qPm)K~rRcqcs=LM12)PX0TT%QO zlf*xkqD3}7l)1J`5W(>=9nR0e6j-<79<11v3ZuXXcQpoCsqY~n`$FN+S}hcVm5Y>G zXnD{@DYs1@{S0z(lW+?86LWKtku$$-(khsh>0qRUXn=84`GRn?77M^_JY`durnN;KE zW#OJ`h<6xcB{I))ekGpc*Ylt}0cx4|OMBDPQvx4`r`}4Ze5_ipdObGMTi3bZHd5PC zcY0;?uBWu$PSvjJeb87nY7ghNv?%M@SoDl6IWt`bQCosfSh$#D6$ea~QhKM^ud2Ut z+9PYJuVpoELmN-A`F$BicO{BSYg@#tS%avVfb}DxL)|NanJ)#zB!2~?#Ot%H7--9N zU$bs0fS5G!m5M4&WK3#a|H|Tgw*?X-;H+Lu@kwA>qSR~7UC7b)7MJXTn6PG>n@8jP zW+}F^X$$c;U~4ryqRF; z>`j!tbLMK4ZGyY643|~?%Mu#fm!l%wAKjBDmd+VYmp3S#$scD$~bxbf|z#)hShN0*AhRaPDcmqrftGlHq4^54MM$Xfy(2> zH8QYVMzmn_oHbvJCB`IN~E&{1*h&0gEM{e zKvWvzp(!BqMX8`t#)~0nq}Wa zr6>FRPyp;AAB&)1$5@;r$23J{K&~>TWjZf7V$wFzmGM95CXhFG1cJNVAXks}C+&2- zbf9Qn*D8N}Afd2kpwDxns3%1uaFhAqDV8ksWiWY|quuLGZ0)SqrJ!Y8yX}@}IyC$C zQ3rCUsn}#>F#D8%D?q~ySy4j&he%Bs{{7V%rl!ui`@KQP?NTi+_iN{cwom&9RaMRR zB~z!hz|0HAgB9_Ijvpe-zr#jLbckJsc>vmo{+im?t8lA;N#fD4?{lb&J0V8Gocq%; f1ihv=QIDh{M_<9V+45Z2{KE4_qW}V3B0uV%GgrOJ literal 0 HcmV?d00001 diff --git a/jetty-servlet/src/test/resources/truststore.jks b/jetty-servlet/src/test/resources/truststore.jks new file mode 100644 index 0000000000000000000000000000000000000000..839cb8c35151c2b7c64afca24b6b72caad070a05 GIT binary patch literal 916 zcmezO_TO6u1_mY|W(3o$xs} zE~X|%Muz1J{3AIFGbaABoD&*5saD@gH|APIn|qhRGl}gsUzm=o9G*UXZaLfkb^*)o zjA*-gTf)`m_MQJYE&gJ}p^PHkrj!4^W|XX5a=N7A{;n#yaON&k_bHloe-^*hm?Z91 zlB>xeD=<(C>yn{9D54u}krkl}HQ(Uscha(++qf!T9y+xaEfnXd1O zi0)T?voO%;QH9LK;*_O3mBblqm)!31vU@hm;^%>mh5U@y3R%l0gzi`2yxH!+?kPOi zt!Tnsz1x9B3U2~8STZp)GB6^C5HPs_Lx_=~O<3xi>MmQ;D_g$D<_pdct`+TyzWTQ= zW5Finm(sGEe;ty^>vg$!cV)t>;H#Mev23$*WWBpyJ}Ir;RW+Htrt6{Pk&qz&-XG2@ z8@{&Lu%DX7m47Uny+-3w`=4V611q#Ub(U`xZCtSK^2LO^3(s|HW&N14dV4@A&(kX% z*S_eUPs-bSWRp>avt;CP@7K+G&3=b&1eO-s3f`;Cf91p#$)FW&xME3L8sEBQQDVCvfG>mdwqnk+GXd2ihXqpv z;usF(WoYYmu8DZZa4%1z=+hI+*gpkUykAy5tj#grb*gH!M6TqIcifYBGVe^&T#-2O K*=+x>r_BKeJV|!| literal 0 HcmV?d00001 From 7c8c45c3979ad790adbc8cd6c70b37b290c6f8d9 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 17 Apr 2014 13:27:34 +0200 Subject: [PATCH 2/2] 432993 - Improve handling of ProxyTo and Prefix parameters in ProxyServlet.Transparent. Fixed case of empty context path and missing Prefix parameter. --- .../org/eclipse/jetty/proxy/ProxyServlet.java | 33 ++++++++------- .../eclipse/jetty/proxy/ProxyServletTest.java | 42 +++++++++++++++++-- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ProxyServlet.java b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ProxyServlet.java index 065937c1eb7..09217f0dfe5 100644 --- a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ProxyServlet.java +++ b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ProxyServlet.java @@ -597,15 +597,14 @@ public class ProxyServlet extends HttpServlet } /** - * Transparent Proxy. - *

- * This convenience extension to ProxyServlet configures the servlet as a transparent proxy. - * The servlet is configured with init parameters: + * This convenience extension to {@link ProxyServlet} configures the servlet as a transparent proxy. + * This servlet is configured with the following init parameters: *

    - *
  • proxyTo - a URI like http://host:80/context to which the request is proxied. - *
  • prefix - a URI prefix that is striped from the start of the forwarded URI. + *
  • proxyTo - a mandatory URI like http://host:80/context to which the request is proxied.
    • + *
  • prefix - an optional URI prefix that is stripped from the start of the forwarded URI.
    • *
- * For example, if a request is received at /foo/bar and the 'proxyTo' parameter is "http://host:80/context" + *

+ * For example, if a request is received at "/foo/bar", the 'proxyTo' parameter is "http://host:80/context" * and the 'prefix' parameter is "/foo", then the request would be proxied to "http://host:80/context/bar". */ public static class Transparent extends ProxyServlet @@ -630,21 +629,23 @@ public class ProxyServlet extends HttpServlet ServletConfig config = getServletConfig(); - String prefix = config.getInitParameter("prefix"); - _prefix = prefix == null ? _prefix : prefix; - - // Adjust prefix value to account for context path - String contextPath = getServletContext().getContextPath(); - _prefix = _prefix == null ? contextPath : (contextPath + _prefix); - String proxyTo = config.getInitParameter("proxyTo"); _proxyTo = proxyTo == null ? _proxyTo : proxyTo; if (_proxyTo == null) throw new UnavailableException("Init parameter 'proxyTo' is required."); - if (!_prefix.startsWith("/")) - throw new UnavailableException("Init parameter 'prefix' parameter must start with a '/'."); + String prefix = config.getInitParameter("prefix"); + if (prefix != null) + { + if (!prefix.startsWith("/")) + throw new UnavailableException("Init parameter 'prefix' must start with a '/'."); + _prefix = prefix; + } + + // Adjust prefix value to account for context path + String contextPath = getServletContext().getContextPath(); + _prefix = _prefix == null ? contextPath : (contextPath + _prefix); _log.debug(config.getServletName() + " @ " + _prefix + " to " + _proxyTo); } diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java index ec32b8b63f0..7c384b05238 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java @@ -18,8 +18,6 @@ package org.eclipse.jetty.proxy; -import static java.nio.file.StandardOpenOption.CREATE; - import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; @@ -30,6 +28,7 @@ import java.net.HttpCookie; import java.nio.ByteBuffer; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardOpenOption; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -39,7 +38,6 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.zip.GZIPOutputStream; - import javax.servlet.AsyncContext; import javax.servlet.AsyncEvent; import javax.servlet.AsyncListener; @@ -93,6 +91,11 @@ public class ProxyServletTest private ServerConnector serverConnector; private void prepareProxy(ProxyServlet proxyServlet) throws Exception + { + prepareProxy(proxyServlet, new HashMap()); + } + + private void prepareProxy(ProxyServlet proxyServlet, Map initParams) throws Exception { proxy = new Server(); proxyConnector = new ServerConnector(proxy); @@ -101,6 +104,7 @@ public class ProxyServletTest ServletContextHandler proxyCtx = new ServletContextHandler(proxy, "/", true, false); this.proxyServlet = proxyServlet; ServletHolder proxyServletHolder = new ServletHolder(proxyServlet); + proxyServletHolder.setInitParameters(initParams); proxyCtx.addServlet(proxyServletHolder, "/*"); proxy.start(); @@ -379,7 +383,7 @@ public class ProxyServletTest final Path temp = Files.createTempFile(targetTestsDir, "test_", null); byte[] kb = new byte[1024]; new Random().nextBytes(kb); - try (OutputStream output = Files.newOutputStream(temp, CREATE)) + try (OutputStream output = Files.newOutputStream(temp, StandardOpenOption.CREATE)) { for (int i = 0; i < length; ++i) output.write(kb); @@ -735,6 +739,36 @@ public class ProxyServletTest Assert.assertTrue(response.getHeaders().containsKey(PROXIED_HEADER)); } + @Test + public void testTransparentProxyWithoutPrefix() throws Exception + { + final String target = "/test"; + prepareServer(new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + if (req.getHeader("Via") != null) + resp.addHeader(PROXIED_HEADER, "true"); + resp.setStatus(target.equals(req.getRequestURI()) ? 200 : 404); + } + }); + + final String proxyTo = "http://localhost:" + serverConnector.getLocalPort(); + ProxyServlet.Transparent proxyServlet = new ProxyServlet.Transparent(); + Map initParams = new HashMap<>(); + initParams.put("proxyTo", proxyTo); + prepareProxy(proxyServlet, initParams); + + // Make the request to the proxy, it should transparently forward to the server + ContentResponse response = client.newRequest("localhost", proxyConnector.getLocalPort()) + .path(target) + .timeout(5, TimeUnit.SECONDS) + .send(); + Assert.assertEquals(200, response.getStatus()); + Assert.assertTrue(response.getHeaders().containsKey(PROXIED_HEADER)); + } + @Test public void testCachingProxy() throws Exception {