From 25a1864de7036e0d09b5ce858e095926c80a78a5 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 10 Jan 2017 11:12:14 -0400 Subject: [PATCH 1/2] Issue #1229 - Adding testcase for WAR (expanded) with WEB-INF/lib/jetty-http.jar --- .../jetty/websocket/server/WSServer.java | 35 ++++++++++++++++--- .../server/WebSocketUpgradeFilterTest.java | 27 ++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java index 9ca3a1d7f4a..22aefa61bd8 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java @@ -18,12 +18,15 @@ package org.eclipse.jetty.websocket.server; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertThat; import java.io.File; import java.io.IOException; import java.net.MalformedURLException; import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import org.eclipse.jetty.annotations.AnnotationConfiguration; @@ -35,6 +38,7 @@ import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.IO; +import org.eclipse.jetty.toolchain.test.JAR; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.OS; import org.eclipse.jetty.toolchain.test.TestingDir; @@ -47,7 +51,6 @@ import org.eclipse.jetty.webapp.MetaInfConfiguration; import org.eclipse.jetty.webapp.WebAppContext; import org.eclipse.jetty.webapp.WebInfConfiguration; import org.eclipse.jetty.webapp.WebXmlConfiguration; -import org.junit.Assert; /** * Utility to build out exploded directory WebApps, in the /target/tests/ directory, for testing out servers that use javax.websocket endpoints. @@ -82,7 +85,7 @@ public class WSServer ClassLoader cl = Thread.currentThread().getContextClassLoader(); String endpointPath = clazz.getName().replace('.','/') + ".class"; URL classUrl = cl.getResource(endpointPath); - Assert.assertThat("Class URL for: " + clazz,classUrl,notNullValue()); + assertThat("Class URL for: " + clazz,classUrl,notNullValue()); File destFile = new File(classesDir,OS.separators(endpointPath)); FS.ensureDirExists(destFile.getParentFile()); File srcFile = new File(classUrl.toURI()); @@ -93,7 +96,31 @@ public class WSServer { copyClass(endpointClass); } - + + public void copyLib(Class clazz, String jarFileName) throws URISyntaxException, IOException + { + webinf = new File(contextDir,"WEB-INF"); + FS.ensureDirExists(webinf); + File libDir = new File(webinf,"lib"); + FS.ensureDirExists(libDir); + File jarFile = new File(libDir, jarFileName); + + URL codeSourceURL = clazz.getProtectionDomain().getCodeSource().getLocation(); + assertThat("Class CodeSource URL is file scheme", codeSourceURL.getProtocol(), is("file")); + + File sourceCodeSourceFile = new File(codeSourceURL.toURI()); + if (sourceCodeSourceFile.isDirectory()) + { + LOG.info("Creating " + jarFile + " from " + sourceCodeSourceFile); + JAR.create(sourceCodeSourceFile, jarFile); + } + else + { + LOG.info("Copying " + sourceCodeSourceFile + " to " + jarFile); + IO.copy(sourceCodeSourceFile, jarFile); + } + } + public void copyWebInf(String testResourceName) throws IOException { webinf = new File(contextDir,"WEB-INF"); @@ -173,7 +200,7 @@ public class WSServer contexts = new ContextHandlerCollection(); handlers.addHandler(contexts); server.setHandler(handlers); - + server.start(); String host = connector.getHost(); diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java index 131b289a9b6..4e0884c1ed8 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java @@ -252,6 +252,33 @@ public class WebSocketUpgradeFilterTest } }}); + // WSUF from web.xml, SCI active, apply app-ws configuration via ServletContextListener with WEB-INF/lib/jetty-http.jar + + cases.add(new Object[]{"wsuf/WebAppContext/web.xml/ServletContextListener/jetty-http.jar", new ServerProvider() + { + @Override + public Server newServer() throws Exception + { + File testDir = MavenTestingUtils.getTargetTestingDir("WSUF-webxml"); + + WSServer server = new WSServer(testDir, "/"); + + server.copyWebInf("wsuf-config-via-listener.xml"); + server.copyClass(InfoSocket.class); + server.copyClass(InfoContextAttributeListener.class); + // Add a jetty-http.jar to ensure that the classloader constraints + // and the WebAppClassloader setup is sane and correct + // The odd version string is present to capture bad regex behavior in Jetty + server.copyLib(org.eclipse.jetty.http.pathmap.PathSpec.class, "jetty-http-9.99.999.jar"); + server.start(); + + WebAppContext webapp = server.createWebAppContext(); + server.deployWebapp(webapp); + + return server.getServer(); + } + }}); + // WSUF from web.xml, SCI active, apply app-ws configuration via Servlet.init cases.add(new Object[]{"wsuf/WebAppContext/web.xml/Servlet.init", new ServerProvider() From 36dcf47f189cc42708424591795efeaf783bfca8 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 11 Jan 2017 17:36:04 +1100 Subject: [PATCH 2/2] Issue #1234 onBadMessage Added a boolean to determine if headerComplete has been called, and if so then earlyEOF is called --- .../org/eclipse/jetty/http/HttpParser.java | 47 +++++++++--------- .../jetty/server/HttpChannelOverHttp.java | 6 ++- .../jetty/server/AsyncRequestReadTest.java | 1 + .../jetty/server/HttpConnectionTest.java | 2 +- .../jetty/server/HttpServerTestBase.java | 49 +++++++++++++++++++ .../jetty/server/HttpServerTestFixture.java | 21 ++++++++ .../jetty/servlet/PostServletTest.java | 44 +++++++---------- 7 files changed, 118 insertions(+), 52 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index c747df41570..b2b3a2fc8bc 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -156,6 +156,7 @@ public class HttpParser private int _responseStatus; private int _headerBytes; private boolean _host; + private boolean _headerComplete; /* ------------------------------------------------------------------------------- */ private volatile State _state=State.START; @@ -730,6 +731,7 @@ public class HttpParser setState(State.END); BufferUtil.clear(buffer); handle=_handler.headerComplete()||handle; + _headerComplete=true; handle=_handler.messageComplete()||handle; return handle; } @@ -800,6 +802,7 @@ public class HttpParser setState(State.END); BufferUtil.clear(buffer); handle=_handler.headerComplete()||handle; + _headerComplete=true; handle=_handler.messageComplete()||handle; return handle; } @@ -1057,22 +1060,26 @@ public class HttpParser case EOF_CONTENT: setState(State.EOF_CONTENT); handle=_handler.headerComplete()||handle; + _headerComplete=true; return handle; case CHUNKED_CONTENT: setState(State.CHUNKED_CONTENT); handle=_handler.headerComplete()||handle; + _headerComplete=true; return handle; case NO_CONTENT: setState(State.END); handle=_handler.headerComplete()||handle; + _headerComplete=true; handle=_handler.messageComplete()||handle; return handle; default: setState(State.CONTENT); handle=_handler.headerComplete()||handle; + _headerComplete=true; return handle; } } @@ -1426,39 +1433,30 @@ public class HttpParser LOG.warn("parse exception: {} in {} for {}",e.toString(),_state,_handler); if (DEBUG) LOG.debug(e); + badMessage(); - switch(_state) - { - case CLOSED: - break; - case CLOSE: - _handler.earlyEOF(); - break; - default: - setState(State.CLOSE); - _handler.badMessage(400,"Bad Message "+e.toString()); - } } catch(Exception|Error e) { BufferUtil.clear(buffer); - LOG.warn("parse exception: "+e.toString()+" for "+_handler,e); - - switch(_state) - { - case CLOSED: - break; - case CLOSE: - _handler.earlyEOF(); - break; - default: - setState(State.CLOSE); - _handler.badMessage(400,null); - } + badMessage(); } return false; } + + protected void badMessage() + { + if (_headerComplete) + { + _handler.earlyEOF(); + } + else if (_state!=State.CLOSED) + { + setState(State.CLOSE); + _handler.badMessage(400,_requestHandler!=null?"Bad Request":"Bad Response"); + } + } protected boolean parseContent(ByteBuffer buffer) { @@ -1677,6 +1675,7 @@ public class HttpParser _contentChunk=null; _headerBytes=0; _host=false; + _headerComplete=false; } /* ------------------------------------------------------------------------------- */ diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java index 33e63508176..76de6958b2a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java @@ -219,11 +219,15 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque @Override public void earlyEOF() { + _httpConnection.getGenerator().setPersistent(false); // If we have no request yet, just close if (_metadata.getMethod() == null) _httpConnection.close(); - else if (onEarlyEOF()) + else if (onEarlyEOF() || _delayedForContent) + { + _delayedForContent = false; handle(); + } } @Override diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncRequestReadTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncRequestReadTest.java index 928f1216bcc..b98647e2f1b 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncRequestReadTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncRequestReadTest.java @@ -297,6 +297,7 @@ public class AsyncRequestReadTest BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream())); assertThat(in.readLine(),containsString("HTTP/1.1 200 OK")); assertThat(in.readLine(),containsString("Content-Length:")); + assertThat(in.readLine(),containsString("Connection: close")); assertThat(in.readLine(),containsString("Server:")); in.readLine(); assertThat(in.readLine(),containsString("XXXXXXX")); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java index b7f127b418a..75ad85a3eb7 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java @@ -421,7 +421,7 @@ public class HttpConnectionTest try(StacklessLogging stackless = new StacklessLogging(HttpParser.class)) { - response=connector.getResponses("GET /bad/encoding%1 HTTP/1.1\r\n"+ + response=connector.getResponse("GET /bad/encoding%1 HTTP/1.1\r\n"+ "Host: localhost\r\n"+ "Connection: close\r\n"+ "\r\n"); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java index e2c755e9c17..ae541b568be 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java @@ -49,6 +49,7 @@ import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.server.handler.AbstractHandler; @@ -666,6 +667,54 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture } } + @Test + public void testBlockingReadBadChunk() throws Exception + { + configureServer(new ReadHandler()); + + try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort())) + { + client.setSoTimeout(600000); + OutputStream os = client.getOutputStream(); + InputStream is = client.getInputStream(); + + os.write(( + "GET /data HTTP/1.1\r\n" + + "host: " + _serverURI.getHost() + ":" + _serverURI.getPort() + "\r\n" + + "content-type: unknown\r\n" + + "transfer-encoding: chunked\r\n" + + "\r\n" + ).getBytes()); + os.flush(); + Thread.sleep(50); + os.write(( + "a\r\n" + + "123456890\r\n" + ).getBytes()); + os.flush(); + + Thread.sleep(50); + os.write(( + "4\r\n" + + "abcd\r\n" + ).getBytes()); + os.flush(); + + Thread.sleep(50); + os.write(( + "X\r\n" + + "abcd\r\n" + ).getBytes()); + os.flush(); + + HttpTester.Response response = HttpTester.parseResponse(HttpTester.from(is)); + + assertThat(response.getStatus(),is(200)); + assertThat(response.getContent(),containsString("EofException")); + assertThat(response.getContent(),containsString("Early EOF")); + } + } + @Test public void testBlockingWhileWritingResponseContent() throws Exception { diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java index 2f8ae4e808d..b11960f7cd3 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java @@ -186,7 +186,28 @@ public class HttpServerTestFixture response.getOutputStream().print("Hello world\r\n"); } } + + protected static class ReadHandler extends AbstractHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + response.setStatus(200); + try + { + InputStream in = request.getInputStream(); + String input= IO.toString(in); + response.getWriter().printf("read %d%n",input.length()); + } + catch(Exception e) + { + response.getWriter().printf("caught %s%n",e); + } + } + } + protected static class DataHandler extends AbstractHandler { @Override diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/PostServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/PostServletTest.java index 2df852ddf59..f509f8fb072 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/PostServletTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/PostServletTest.java @@ -159,11 +159,8 @@ public class PostServletTest req.append("\r\n"); req.append("\r\n"); - try (StacklessLogging scope = new StacklessLogging(ServletHandler.class)) - { - String resp = connector.getResponses(req.toString()); - assertThat(resp,is("")); // Aborted before response committed - } + String resp = connector.getResponse(req.toString()); + assertThat(resp,startsWith("HTTP/1.1 200 OK")); // exception eaten by handler assertTrue(complete.await(5,TimeUnit.SECONDS)); assertThat(ex0.get(),not(nullValue())); assertThat(ex1.get(),not(nullValue())); @@ -178,31 +175,26 @@ public class PostServletTest req.append("Transfer-Encoding: chunked\r\n"); req.append("\r\n"); - try (StacklessLogging scope = new StacklessLogging(ServletHandler.class)) - { - LocalConnector.LocalEndPoint endp=connector.executeRequest(req.toString()); - Thread.sleep(1000); - assertFalse(posted.get()); + LocalConnector.LocalEndPoint endp=connector.executeRequest(req.toString()); + Thread.sleep(1000); + assertFalse(posted.get()); - req.setLength(0); - // intentionally bad (not a valid chunked char here) - for (int i=1024;i-->0;) - req.append("xxxxxxxxxxxx"); - req.append("\r\n"); - req.append("\r\n"); + req.setLength(0); + // intentionally bad (not a valid chunked char here) + for (int i=1024;i-->0;) + req.append("xxxxxxxxxxxx"); + req.append("\r\n"); + req.append("\r\n"); - endp.addInput(req.toString()); + endp.addInput(req.toString()); - endp.waitUntilClosedOrIdleFor(1,TimeUnit.SECONDS); - String resp = endp.takeOutputString(); + endp.waitUntilClosedOrIdleFor(1,TimeUnit.SECONDS); + String resp = endp.takeOutputString(); - assertThat("resp", resp, containsString("HTTP/1.1 400 ")); - - } - - // null because it was never dispatched! - assertThat(ex0.get(),nullValue()); - assertThat(ex1.get(),nullValue()); + assertThat(resp,startsWith("HTTP/1.1 200 OK")); // exception eaten by handler + assertTrue(complete.await(5,TimeUnit.SECONDS)); + assertThat(ex0.get(),not(nullValue())); + assertThat(ex1.get(),not(nullValue())); }