From 1721a56c6d9586c8b90df78bf16db930011d6e97 Mon Sep 17 00:00:00 2001 From: Thomas Becker Date: Wed, 1 Feb 2012 10:22:11 +0100 Subject: [PATCH 1/2] 369349: directory with spaces --dry-run fix --- .../jetty/start/CommandLineBuilder.java | 12 ++++--- .../jetty/start/CommandLineBuilderTest.java | 27 ++++++++++---- .../org/eclipse/jetty/start/MainTest.java | 36 ++++++++++++------- 3 files changed, 51 insertions(+), 24 deletions(-) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java b/jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java index ba2e8b9dc83..3ea9ba76033 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java @@ -35,7 +35,8 @@ public class CommandLineBuilder */ public void addArg(String arg) { - args.add(quote(arg)); + if (arg != null) + args.add(quote(arg)); } /** @@ -78,7 +79,8 @@ public class CommandLineBuilder */ public void addRawArg(String arg) { - args.add(arg); + if (arg != null) + args.add(arg); } public List getArgs() @@ -100,7 +102,7 @@ public class CommandLineBuilder return arg; } StringBuilder buf = new StringBuilder(); -// buf.append('"'); + // buf.append('"'); boolean escaped = false; for (char c : arg.toCharArray()) { @@ -111,7 +113,7 @@ public class CommandLineBuilder escaped = (c == '\\'); buf.append(c); } -// buf.append('"'); + // buf.append('"'); return buf.toString(); } @@ -127,7 +129,7 @@ public class CommandLineBuilder { buf.append(' '); } - buf.append(arg); + buf.append(quote(arg)); delim = true; } diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java index c4ff43e326d..72194229aa9 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java @@ -12,22 +12,29 @@ package org.eclipse.jetty.start; //You may elect to redistribute this code under either of these licenses. //======================================================================== -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.is; + import org.junit.Assert; +import org.junit.Before; import org.junit.Test; public class CommandLineBuilderTest { + private CommandLineBuilder cmd = new CommandLineBuilder("java"); + + @Before + public void setUp() + { + cmd.addEqualsArg("-Djava.io.tmpdir","/home/java/temp dir/"); + cmd.addArg("--version"); + } + @Test public void testSimpleCommandline() { - CommandLineBuilder cmd = new CommandLineBuilder("java"); - cmd.addEqualsArg("-Djava.io.tmpdir","/home/java/temp dir/"); - cmd.addArg("--version"); - - Assert.assertThat(cmd.toString(), is("java -Djava.io.tmpdir=/home/java/temp\\ dir/ --version")); + Assert.assertThat(cmd.toString(),is("java -Djava.io.tmpdir=/home/java/temp\\ dir/ --version")); } - + @Test public void testQuotingSimple() { @@ -46,6 +53,12 @@ public class CommandLineBuilderTest assertQuoting("/opt/jetty 7 \"special\"/home","/opt/jetty\\ 7\\ \\\"special\\\"/home"); } + @Test + public void testToStringIsQuotedEvenIfArgsAreNotQuotedForProcessBuilder() + { + System.out.println(cmd.toString()); + } + private void assertQuoting(String raw, String expected) { String actual = CommandLineBuilder.quote(raw); diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java index f84c8eaa185..8ad2d45782c 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java @@ -13,10 +13,6 @@ package org.eclipse.jetty.start; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.notNullValue; -import static org.junit.Assert.assertEquals; - import java.io.File; import java.io.IOException; import java.lang.reflect.Field; @@ -25,10 +21,16 @@ import java.util.List; import java.util.Vector; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + /* ------------------------------------------------------------ */ /** */ @@ -96,14 +98,24 @@ public class MainTest Classpath classpath = nastyWayToCreateAClasspathObject("/jetty/home with spaces/"); CommandLineBuilder cmd = main.buildCommandLine(classpath,xmls); - Assert.assertThat("CommandLineBuilder shouldn't be null",cmd,notNullValue()); - String commandLine = cmd.toString(); - Assert.assertThat("CommandLine shouldn't be null",commandLine,notNullValue()); - Assert.assertThat("Classpath should be correctly quoted and match expected value",commandLine, - containsString("-cp /jetty/home with spaces/somejar.jar:/jetty/home with spaces/someotherjar.jar")); - Assert.assertThat("CommandLine should contain jvmArgs",commandLine,containsString("--exec -Xms1024m -Xmx1024m")); - Assert.assertThat("CommandLine should contain xmls",commandLine,containsString("jetty.xml jetty-jmx.xml jetty-logging.xml")); + assertThat("CommandLineBuilder shouldn't be null",cmd,notNullValue()); + List commandArgs = cmd.getArgs(); + assertThat("commandArgs should contain 11 elements",commandArgs.size(),equalTo(11)); + assertThat("args does not contain -cp",commandArgs,hasItems("-cp")); + assertThat("Classpath should be correctly quoted and match expected value",commandArgs, + hasItems("/jetty/home with spaces/somejar.jar:/jetty/home with spaces/someotherjar.jar")); + assertThat("args does not contain --exec",commandArgs,hasItems("--exec")); + assertThat("CommandLine should contain jvmArgs",commandArgs,hasItems("-Xms1024m")); + assertThat("CommandLine should contain jvmArgs", commandArgs, hasItems("-Xmx1024m")); + assertThat("CommandLine should contain xmls",commandArgs,hasItems("jetty.xml")); + assertThat("CommandLine should contain xmls",commandArgs,hasItems("jetty-jmx.xml")); + assertThat("CommandLine should contain xmls", commandArgs, hasItems("jetty-logging.xml")); + + String commandLine = cmd.toString(); + assertThat("cmd.toString() should be properly escaped",commandLine,containsString("-cp /jetty/home\\ with\\ " + + "spaces/somejar.jar:/jetty/home\\ with\\ spaces/someotherjar.jar")); + assertThat("cmd.toString() doesn't contain xml config files",commandLine,containsString(" jetty.xml jetty-jmx.xml jetty-logging.xml")); } private Classpath nastyWayToCreateAClasspathObject(String jettyHome) throws NoSuchFieldException, IllegalAccessException From 9173751438bf29033a8de0336d918380751a39f8 Mon Sep 17 00:00:00 2001 From: Thomas Becker Date: Thu, 23 Aug 2012 19:36:49 +0200 Subject: [PATCH 2/2] 387919: throw EOFException on early eof from client on http requests --- .../jetty/server/AbstractHttpConnection.java | 35 +++++++++++- .../org/eclipse/jetty/server/HttpInput.java | 17 +++--- .../jetty/server/HttpServerTestBase.java | 57 +++++++++++++++++-- .../ssl/SelectChannelServerSslTest.java | 14 +++-- .../jetty/server/ssl/SslSocketServerTest.java | 6 ++ .../test/resources/jetty-logging.properties | 3 + 6 files changed, 111 insertions(+), 21 deletions(-) create mode 100644 jetty-server/src/test/resources/jetty-logging.properties diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractHttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractHttpConnection.java index 8bf464a78d0..3d2d13f94e7 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractHttpConnection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractHttpConnection.java @@ -129,6 +129,7 @@ public abstract class AbstractHttpConnection extends AbstractConnection private boolean _head = false; private boolean _host = false; private boolean _delayedHandling=false; + private boolean _earlyEOF = false; /* ------------------------------------------------------------ */ public static AbstractHttpConnection getCurrentConnection() @@ -394,6 +395,12 @@ public abstract class AbstractHttpConnection extends AbstractConnection return _generator.isCommitted(); } + /* ------------------------------------------------------------ */ + public boolean isEarlyEOF() + { + return _earlyEOF; + } + /* ------------------------------------------------------------ */ public void reset() { @@ -407,6 +414,7 @@ public abstract class AbstractHttpConnection extends AbstractConnection _response.recycle(); _uri.clear(); _writer=null; + _earlyEOF = false; } /* ------------------------------------------------------------ */ @@ -443,7 +451,7 @@ public abstract class AbstractHttpConnection extends AbstractConnection { _uri.getPort(); String path = null; - + try { path = _uri.getDecodedPath(); @@ -454,7 +462,7 @@ public abstract class AbstractHttpConnection extends AbstractConnection LOG.ignore(e); path = _uri.getDecodedPath(StringUtil.__ISO_8859_1); } - + info=URIUtil.canonicalPath(path); if (info==null && !_request.getMethod().equals(HttpMethods.CONNECT)) { @@ -719,6 +727,7 @@ public abstract class AbstractHttpConnection extends AbstractConnection _requests); } + /* ------------------------------------------------------------ */ protected void startRequest(Buffer method, Buffer uri, Buffer version) throws IOException { uri=uri.asImmutableBuffer(); @@ -778,6 +787,7 @@ public abstract class AbstractHttpConnection extends AbstractConnection } } + /* ------------------------------------------------------------ */ protected void parsedHeader(Buffer name, Buffer value) throws IOException { int ho = HttpHeaders.CACHE.getOrdinal(name); @@ -839,6 +849,7 @@ public abstract class AbstractHttpConnection extends AbstractConnection _requestFields.add(name, value); } + /* ------------------------------------------------------------ */ protected void headerComplete() throws IOException { _requests++; @@ -909,6 +920,7 @@ public abstract class AbstractHttpConnection extends AbstractConnection _delayedHandling=true; } + /* ------------------------------------------------------------ */ protected void content(Buffer buffer) throws IOException { if (_delayedHandling) @@ -918,6 +930,7 @@ public abstract class AbstractHttpConnection extends AbstractConnection } } + /* ------------------------------------------------------------ */ public void messageComplete(long contentLength) throws IOException { if (_delayedHandling) @@ -927,6 +940,12 @@ public abstract class AbstractHttpConnection extends AbstractConnection } } + /* ------------------------------------------------------------ */ + public void earlyEOF() + { + _earlyEOF = true; + } + /* ------------------------------------------------------------ */ /* ------------------------------------------------------------ */ /* ------------------------------------------------------------ */ @@ -996,6 +1015,18 @@ public abstract class AbstractHttpConnection extends AbstractConnection if (LOG.isDebugEnabled()) LOG.debug("Bad request!: "+version+" "+status+" "+reason); } + + /* ------------------------------------------------------------ */ + /* + * (non-Javadoc) + * + * @see org.eclipse.jetty.server.server.HttpParser.EventHandler#earlyEOF() + */ + @Override + public void earlyEOF() + { + AbstractHttpConnection.this.earlyEOF(); + } } /* ------------------------------------------------------------ */ diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java index af5d510d86d..025cf68c8af 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java @@ -24,12 +24,13 @@ import javax.servlet.ServletInputStream; import org.eclipse.jetty.http.HttpParser; import org.eclipse.jetty.io.Buffer; +import org.eclipse.jetty.io.EofException; public class HttpInput extends ServletInputStream { protected final AbstractHttpConnection _connection; protected final HttpParser _parser; - + /* ------------------------------------------------------------ */ public HttpInput(AbstractHttpConnection connection) { @@ -44,11 +45,9 @@ public class HttpInput extends ServletInputStream @Override public int read() throws IOException { - int c=-1; - Buffer content=_parser.blockForContent(_connection.getMaxIdleTime()); - if (content!=null) - c= 0xff & content.get(); - return c; + byte[] bytes = new byte[1]; + int read = read(bytes, 0, 1); + return read < 0 ? -1 : 0xff & bytes[0]; } /* ------------------------------------------------------------ */ @@ -62,6 +61,8 @@ public class HttpInput extends ServletInputStream Buffer content=_parser.blockForContent(_connection.getMaxIdleTime()); if (content!=null) l= content.get(b, off, len); + else if (_connection.isEarlyEOF()) + throw new EofException("early EOF"); return l; } @@ -71,8 +72,4 @@ public class HttpInput extends ServletInputStream { return _parser.available(); } - - - - } 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 9d51233ffa0..e902ec014cc 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 @@ -30,15 +30,16 @@ import java.net.SocketException; import java.net.URL; import java.util.Arrays; import java.util.Random; -import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.Exchanger; +import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.ServletException; +import javax.servlet.ServletInputStream; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; @@ -50,10 +51,11 @@ import org.junit.Test; import org.junit.matchers.JUnitMatchers; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * @@ -148,8 +150,55 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture client.close(); } } - + @Test + public void testInterruptedRequest() throws Exception + { + final AtomicBoolean fourBytesRead = new AtomicBoolean(false); + final AtomicBoolean earlyEOFException = new AtomicBoolean(false); + configureServer(new AbstractHandler() + { + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + int contentLength = request.getContentLength(); + ServletInputStream inputStream = request.getInputStream(); + for (int i = 0; i < contentLength; i++) + { + try + { + inputStream.read(); + } + catch (EofException e) + { + earlyEOFException.set(true); + throw e; + } + if (i == 3) + fourBytesRead.set(true); + } + } + }); + + StringBuffer request = new StringBuffer("GET / HTTP/1.0\n"); + request.append("Host: localhost\n"); + request.append("Content-length: 6\n\n"); + request.append("foo"); + + Socket client = newSocket(HOST, _connector.getLocalPort()); + OutputStream os = client.getOutputStream(); + + os.write(request.toString().getBytes()); + os.flush(); + client.shutdownOutput(); + String response = readResponse(client); + client.close(); + +// assertThat("response contains 200 OK", response.contains(" 500 "), is(true)); //TODO: check with gregw, +// currently returns 200 + assertThat("The 4th byte (-1) has not been passed to the handler", fourBytesRead.get(), is(false)); + assertThat("EofException has been caught", earlyEOFException.get(), is(true)); + } /* * Feed the server the entire request at once. diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SelectChannelServerSslTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SelectChannelServerSslTest.java index 840219f66ee..096ecca0719 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SelectChannelServerSslTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SelectChannelServerSslTest.java @@ -17,8 +17,6 @@ // package org.eclipse.jetty.server.ssl; -import static org.junit.Assert.assertEquals; - import java.io.FileInputStream; import java.io.IOException; import java.io.OutputStream; @@ -26,13 +24,11 @@ import java.net.Socket; import java.security.KeyStore; import java.util.Arrays; import java.util.concurrent.atomic.AtomicInteger; - import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.TrustManagerFactory; - import org.eclipse.jetty.io.AsyncEndPoint; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.nio.SslConnection; @@ -41,8 +37,10 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; -import static org.junit.Assert.assertThat; + import static org.hamcrest.Matchers.lessThan; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; /** * HttpServer Tester. @@ -166,6 +164,12 @@ public class SelectChannelServerSslTest extends HttpServerTestBase } } + @Override + @Test + @Ignore("Override and ignore this test as SSLSocket.shutdownOutput() is not supported, " + + "but shutdownOutput() is needed by the test.") + public void testInterruptedRequest(){} + @Override @Ignore public void testAvailable() throws Exception diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslSocketServerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslSocketServerTest.java index 4da4106b243..0c53417929b 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslSocketServerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslSocketServerTest.java @@ -75,6 +75,12 @@ public class SslSocketServerTest extends HttpServerTestBase } + @Override + @Test + @Ignore("Override and ignore this test as SSLSocket.shutdownOutput() is not supported, " + + "but shutdownOutput() is needed by the test.") + public void testInterruptedRequest(){} + @Override @Test public void testFlush() throws Exception diff --git a/jetty-server/src/test/resources/jetty-logging.properties b/jetty-server/src/test/resources/jetty-logging.properties new file mode 100644 index 00000000000..d8439d19ea3 --- /dev/null +++ b/jetty-server/src/test/resources/jetty-logging.properties @@ -0,0 +1,3 @@ +# Setup default logging implementation for during testing +org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog +org.eclipse.jetty.server.LEVEL=INFO