From 25a95b83f91c9fdd65bd8c528e45e061b58286db Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 2 Feb 2017 19:28:16 -0700 Subject: [PATCH 1/6] Bad tests take too long --- .../jetty/server/AbstractHttpTest.java | 3 +- .../server/HttpManyWaysToCommitTest.java | 41 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java index 736f0835a48..b350d58bfe9 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java @@ -92,7 +92,8 @@ public abstract class AbstractHttpTest writer.write("\r\n"); writer.flush(); - HttpTester.Response response = HttpTester.parseResponse(socket.getInputStream()); + HttpTester.Input input = HttpTester.from(socket.getInputStream()); + HttpTester.Response response = HttpTester.parseResponse(input); if ("HTTP/1.1".equals(httpVersion) && response.get("content-length") == null && response.get("transfer-encoding") == null diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java index 37b410b26c2..c5e60c4b34a 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java @@ -417,6 +417,47 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest } } + @Test + public void testSetContentLengthFlushAndWriteInsufficientBytes() throws Exception + { + server.setHandler(new SetContentLengthAndWriteInsufficientBytesHandler(true)); + server.start(); + try + { + HttpTester.Response response = executeRequest(); + char badChar = (char) -1; + String failed_body = "" + badChar + badChar + badChar; + assertThat("response code", response.getStatus(), is(200)); + assertHeader(response, "content-length", "6"); + assertThat(response.getContent(), endsWith(failed_body)); + } + catch(EOFException e) + { + // possible good response + } + } + + @Test + public void testSetContentLengthAndWriteInsufficientBytes() throws Exception + { + server.setHandler(new SetContentLengthAndWriteInsufficientBytesHandler(false)); + server.start(); + + try + { + HttpTester.Response response = executeRequest(); + char badChar = (char) -1; + String failed_body = "" + badChar + badChar + badChar; + assertThat("response code is 200", response.getStatus(), is(200)); + assertHeader(response, "content-length", "6"); + assertThat(response.getContent(), endsWith(failed_body)); + } + catch(EOFException e) + { + // expected + } + } + @Test public void testSetContentLengthAndWriteExactlyThatAmountOfBytes() throws Exception { From fbbd5f4777017229cfe5df39ea5d8f4f5ac4acb5 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 3 Feb 2017 04:53:03 -0700 Subject: [PATCH 2/6] HttpTester.parseResponse(Input) returns on TE/Chunked now --- .../test/java/org/eclipse/jetty/http/HttpTester.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java index a9eec75373c..b33c16c72b6 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java @@ -25,10 +25,13 @@ import java.nio.ByteBuffer; import java.nio.channels.ReadableByteChannel; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.util.Locale; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; /** @@ -59,6 +62,8 @@ import org.eclipse.jetty.util.StringUtil; */ public class HttpTester { + private final static Logger LOG = Log.getLogger(HttpTester.class); + private HttpTester() { } @@ -214,6 +219,13 @@ public class HttpTester if (r.isComplete()) return r; + + String te = r.get(HttpHeader.TRANSFER_ENCODING); + if(te != null && te.toLowerCase(Locale.ENGLISH).contains("chunked")) + return r; + + LOG.info("Incomplete Response: (parser={}) {}", parser, r); + in.setHttpParser(parser); return null; } From 0a1eee1c28820265d6bfacf7273af4d628c84ba9 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 3 Feb 2017 06:25:23 -0700 Subject: [PATCH 3/6] Fixing surefire breaking HttpManyWayToCommitTest + Reverting change to HttpTester.parseResponse(Input) + Providing new HttpTester.parsePartialResponse(Input) + InsufficientBytes tests no longer assert content strings with invalid characters (this was breaks the surefire report xml) --- .../org/eclipse/jetty/http/HttpTester.java | 19 ++++++++++++------- .../eclipse/jetty/http/HttpTesterTest.java | 7 +------ .../jetty/server/AbstractHttpTest.java | 2 +- .../server/HttpManyWaysToCommitTest.java | 15 +++++++++------ 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java index b33c16c72b6..47206e27746 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java @@ -25,7 +25,6 @@ import java.nio.ByteBuffer; import java.nio.channels.ReadableByteChannel; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.util.Locale; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; @@ -194,7 +193,17 @@ public class HttpTester } public static Response parseResponse(Input in) throws IOException - { + { + return parseResponse(in, false); + } + + public static Response parsePartialResponse(Input in) throws IOException + { + return parseResponse(in, true); + } + + private static Response parseResponse(Input in, boolean allowIncomplete) throws IOException + { Response r; HttpParser parser=in.takeHttpParser(); if (parser==null) @@ -217,13 +226,9 @@ public class HttpTester break; } - if (r.isComplete()) + if (allowIncomplete || r.isComplete()) return r; - String te = r.get(HttpHeader.TRANSFER_ENCODING); - if(te != null && te.toLowerCase(Locale.ENGLISH).contains("chunked")) - return r; - LOG.info("Incomplete Response: (parser={}) {}", parser, r); in.setHttpParser(parser); diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTesterTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTesterTest.java index c0e642ae887..4ecfaf277ba 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTesterTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTesterTest.java @@ -20,7 +20,7 @@ package org.eclipse.jetty.http; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.*; +import static org.junit.Assert.assertThat; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -30,14 +30,11 @@ import java.net.Socket; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; -import org.junit.Ignore; import org.junit.Test; public class HttpTesterTest { - @Test - @Ignore public void testExampleUsage() throws Exception { try(Socket socket = new Socket("www.google.com",80)) @@ -59,10 +56,8 @@ public class HttpTesterTest System.err.printf("%s: %s%n",field.getName(),field.getValue()); System.err.printf("%n%s%n",response.getContent()); } - } - @Test public void testGetRequestBuffer10() { diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java index b350d58bfe9..a544816252a 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java @@ -93,7 +93,7 @@ public abstract class AbstractHttpTest writer.flush(); HttpTester.Input input = HttpTester.from(socket.getInputStream()); - HttpTester.Response response = HttpTester.parseResponse(input); + HttpTester.Response response = HttpTester.parsePartialResponse(input); if ("HTTP/1.1".equals(httpVersion) && response.get("content-length") == null && response.get("transfer-encoding") == null diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java index c5e60c4b34a..f4595070355 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java @@ -23,6 +23,7 @@ import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertThat; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collection; @@ -425,11 +426,12 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest try { HttpTester.Response response = executeRequest(); - char badChar = (char) -1; - String failed_body = "" + badChar + badChar + badChar; assertThat("response code", response.getStatus(), is(200)); assertHeader(response, "content-length", "6"); - assertThat(response.getContent(), endsWith(failed_body)); + byte content[] = response.getContentBytes(); + assertThat("content bytes", content.length, is(6)); + String contentStr = new String(content, StandardCharsets.UTF_8); + assertThat("content bytes as string", contentStr, is("foo")); } catch(EOFException e) { @@ -446,11 +448,12 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest try { HttpTester.Response response = executeRequest(); - char badChar = (char) -1; - String failed_body = "" + badChar + badChar + badChar; assertThat("response code is 200", response.getStatus(), is(200)); assertHeader(response, "content-length", "6"); - assertThat(response.getContent(), endsWith(failed_body)); + byte content[] = response.getContentBytes(); + assertThat("content bytes", content.length, is(3)); + String contentStr = new String(content, StandardCharsets.UTF_8); + assertThat("content bytes as string", contentStr, is("foo")); } catch(EOFException e) { From cdc58b40088c3bb1b9fccb7fedcc0b161675a517 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 3 Feb 2017 10:29:27 -0700 Subject: [PATCH 4/6] Fixing InsufficientBytes test cases + Because of issues #1045 and #1185 insufficient bytes on a response results in a closed connection --- .../org/eclipse/jetty/http/HttpTester.java | 56 ++++++++++++------- .../jetty/server/AbstractHttpTest.java | 7 ++- .../server/HttpManyWaysToCommitTest.java | 41 ++++---------- 3 files changed, 52 insertions(+), 52 deletions(-) diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java index 47206e27746..230418909c2 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java @@ -118,7 +118,7 @@ public class HttpTester parser.parseNext(ByteBuffer.wrap(contentStream.toByteArray())); return r; } - + public abstract static class Input { boolean _eof=false; @@ -193,27 +193,41 @@ public class HttpTester } public static Response parseResponse(Input in) throws IOException - { - return parseResponse(in, false); - } - - public static Response parsePartialResponse(Input in) throws IOException - { - return parseResponse(in, true); - } - - private static Response parseResponse(Input in, boolean allowIncomplete) throws IOException { Response r; HttpParser parser=in.takeHttpParser(); if (parser==null) { r=new Response(); - parser =new HttpParser(r); + parser = new HttpParser(r); } else r=(Response)parser.getHandler(); + parseResponse(in, parser, r); + + if(r.isComplete()) + return r; + + in.setHttpParser(parser); + return null; + } + + public static void parseResponse(Input in, Response response) throws IOException + { + HttpParser parser = in.takeHttpParser(); + if (parser == null) + { + parser = new HttpParser(response); + } + parseResponse(in, parser, response); + + if (!response.isComplete()) + in.setHttpParser(parser); + } + + private static void parseResponse(Input in, HttpParser parser, Response r) throws IOException + { ByteBuffer buffer = in.getBuffer(); int len=0; @@ -226,18 +240,12 @@ public class HttpTester break; } - if (allowIncomplete || r.isComplete()) - return r; - - LOG.info("Incomplete Response: (parser={}) {}", parser, r); - - in.setHttpParser(parser); - return null; + System.out.printf("parseResponse() parser=%s%n%s%n", parser, r.toString()); } - public abstract static class Message extends HttpFields implements HttpParser.HttpHandler { + boolean _earlyEOF; boolean _complete=false; ByteArrayOutputStream _content; HttpVersion _version=HttpVersion.HTTP_1_0; @@ -350,8 +358,14 @@ public class HttpTester @Override public void earlyEOF() { + _earlyEOF = true; } - + + public boolean isEarlyEOF() + { + return _earlyEOF; + } + @Override public boolean content(ByteBuffer ref) { diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java index a544816252a..94b969dfbcd 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java @@ -66,7 +66,7 @@ public abstract class AbstractHttpTest { server = new Server(); connector = new ServerConnector(server,null,null,new ArrayByteBufferPool(64,2048,64*1024),1,1,new HttpConnectionFactory()); - connector.setIdleTimeout(10000); + connector.setIdleTimeout(100000); server.addConnector(connector); stacklessChannelLogging =new StacklessLogging(HttpChannel.class); @@ -92,9 +92,12 @@ public abstract class AbstractHttpTest writer.write("\r\n"); writer.flush(); + HttpTester.Response response = new HttpTester.Response(); HttpTester.Input input = HttpTester.from(socket.getInputStream()); - HttpTester.Response response = HttpTester.parsePartialResponse(input); + HttpTester.parseResponse(input, response); + if ("HTTP/1.1".equals(httpVersion) + && response.isComplete() && response.get("content-length") == null && response.get("transfer-encoding") == null && !__noBodyCodes.contains(response.getStatus())) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java index f4595070355..f1ae0e537cd 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java @@ -21,9 +21,9 @@ package org.eclipse.jetty.server; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collection; @@ -423,20 +423,14 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest { server.setHandler(new SetContentLengthAndWriteInsufficientBytesHandler(true)); server.start(); - try - { - HttpTester.Response response = executeRequest(); - assertThat("response code", response.getStatus(), is(200)); - assertHeader(response, "content-length", "6"); - byte content[] = response.getContentBytes(); - assertThat("content bytes", content.length, is(6)); - String contentStr = new String(content, StandardCharsets.UTF_8); - assertThat("content bytes as string", contentStr, is("foo")); - } - catch(EOFException e) - { - // possible good response - } + + HttpTester.Response response = executeRequest(); + System.out.println(response.toString()); + assertThat("response code", response.getStatus(), is(200)); + assertHeader(response, "content-length", "6"); + byte content[] = response.getContentBytes(); + assertThat("content bytes", content.length, is(0)); + assertTrue("response eof", response.isEarlyEOF()); } @Test @@ -445,20 +439,9 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest server.setHandler(new SetContentLengthAndWriteInsufficientBytesHandler(false)); server.start(); - try - { - HttpTester.Response response = executeRequest(); - assertThat("response code is 200", response.getStatus(), is(200)); - assertHeader(response, "content-length", "6"); - byte content[] = response.getContentBytes(); - assertThat("content bytes", content.length, is(3)); - String contentStr = new String(content, StandardCharsets.UTF_8); - assertThat("content bytes as string", contentStr, is("foo")); - } - catch(EOFException e) - { - // expected - } + HttpTester.Response response = executeRequest(); + assertThat("response has no status", response.getStatus(), is(0)); + assertTrue("response eof", response.isEarlyEOF()); } @Test From b72854f02b9d8b37c4149383dc2c78d540c08c1e Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 3 Feb 2017 10:34:30 -0700 Subject: [PATCH 5/6] Removing System.out --- jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java index 230418909c2..bc6b06c89fd 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java @@ -239,8 +239,6 @@ public class HttpTester if (in.fillBuffer()<=0) break; } - - System.out.printf("parseResponse() parser=%s%n%s%n", parser, r.toString()); } public abstract static class Message extends HttpFields implements HttpParser.HttpHandler From 98688052ef1b291a6a4c8198d5cc484444bb2bc3 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 6 Feb 2017 11:06:12 -0700 Subject: [PATCH 6/6] Disabling InsufficientBytes tests in 9.3 (fixed in 9.4+) --- .../jetty/server/AbstractHttpTest.java | 2 +- .../server/HttpManyWaysToCommitTest.java | 25 ++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java index 94b969dfbcd..1599c365561 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java @@ -66,7 +66,7 @@ public abstract class AbstractHttpTest { server = new Server(); connector = new ServerConnector(server,null,null,new ArrayByteBufferPool(64,2048,64*1024),1,1,new HttpConnectionFactory()); - connector.setIdleTimeout(100000); + connector.setIdleTimeout(10000); server.addConnector(connector); stacklessChannelLogging =new StacklessLogging(HttpChannel.class); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java index f1ae0e537cd..2fb52dc9f38 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java @@ -33,6 +33,8 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -419,6 +421,7 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest } @Test + @Ignore("Not implemented in Jetty 9.3 (Proper support present in Jetty 9.4+)") public void testSetContentLengthFlushAndWriteInsufficientBytes() throws Exception { server.setHandler(new SetContentLengthAndWriteInsufficientBytesHandler(true)); @@ -434,6 +437,7 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest } @Test + @Ignore("Not implemented in Jetty 9.3 (Proper support present in Jetty 9.4+)") public void testSetContentLengthAndWriteInsufficientBytes() throws Exception { server.setHandler(new SetContentLengthAndWriteInsufficientBytesHandler(false)); @@ -469,7 +473,26 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest assertThat("response code", response.getStatus(), is(200)); assertThat("response body", response.getContent(), is("foo")); } - + + private class SetContentLengthAndWriteInsufficientBytesHandler extends AbstractHandler + { + boolean flush; + private SetContentLengthAndWriteInsufficientBytesHandler(boolean flush) + { + this.flush = flush; + } + + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + response.setContentLength(6); + if (flush) + response.flushBuffer(); + response.getWriter().write("foo"); + } + } + private class SetContentLengthAndWriteThatAmountOfBytesHandler extends ThrowExceptionOnDemandHandler { private SetContentLengthAndWriteThatAmountOfBytesHandler(boolean throwException)