From 3486046f1e0cf38745d3055d449552420acec59a Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 29 Jan 2014 18:26:32 +0100 Subject: [PATCH] 426870 - HTTP 1.0 Request with Connection: keep-alive and response content hangs. Refactored tests. --- ....java => HttpGeneratorServerHTTPTest.java} | 198 ++++++------ .../jetty/http/HttpGeneratorServerTest.java | 300 +----------------- 2 files changed, 106 insertions(+), 392 deletions(-) rename jetty-http/src/test/java/org/eclipse/jetty/http/{HttpGeneratorServerTRTest.java => HttpGeneratorServerHTTPTest.java} (72%) diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTRTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerHTTPTest.java similarity index 72% rename from jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTRTest.java rename to jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerHTTPTest.java index a3f543780a3..2925f3e41f2 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTRTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerHTTPTest.java @@ -18,12 +18,10 @@ package org.eclipse.jetty.http; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; - import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collection; +import java.util.EnumSet; import java.util.List; import org.eclipse.jetty.util.BufferUtil; @@ -33,22 +31,77 @@ import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameter; import org.junit.runners.Parameterized.Parameters; +import static org.hamcrest.Matchers.either; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + @RunWith(Parameterized.class) -public class HttpGeneratorServerTRTest +public class HttpGeneratorServerHTTPTest { - private static class TR + @Parameter(value = 0) + public Run run; + private String _content; + private String _reason; + + @Test + public void testHTTP() throws Exception + { + Handler handler = new Handler(); + + HttpGenerator gen = new HttpGenerator(); + + String t = run.toString(); + + run.result.getHttpFields().clear(); + + String response = run.result.build(run.httpVersion, gen, "OK\r\nTest", run.connection.val, null, run.chunks); + + if (run.httpVersion == 9) + { + assertFalse(t, gen.isPersistent()); + if (run.result._body != null) + assertEquals(t, run.result._body, response); + return; + } + + HttpParser parser = new HttpParser(handler); + parser.setHeadResponse(run.result._head); + + parser.parseNext(BufferUtil.toBuffer(response)); + + if (run.result._body != null) + assertEquals(t, run.result._body, this._content); + + if (run.httpVersion == 10) + assertTrue(t, gen.isPersistent() || run.result._contentLength >= 0 || EnumSet.of(ConnectionType.CLOSE, ConnectionType.KEEP_ALIVE, ConnectionType.NONE).contains(run.connection)); + else + assertTrue(t, gen.isPersistent() || EnumSet.of(ConnectionType.CLOSE, ConnectionType.TE_CLOSE).contains(run.connection)); + + if (run.httpVersion > 9) + assertEquals("OK??Test", _reason); + + if (_content == null) + assertTrue(t, run.result._body == null); + else + assertThat(t, run.result._contentLength, either(equalTo(_content.length())).or(equalTo(-1))); + } + + private static class Result { private HttpFields _fields = new HttpFields(); private final String _body; private final int _code; - String _connection; - int _contentLength; - String _contentType; + private String _connection; + private int _contentLength; + private String _contentType; private final boolean _head; - String _other; - String _te; + private String _other; + private String _te; - private TR(int code, String contentType, int contentLength, String content, boolean head) + private Result(int code, String contentType, int contentLength, String content, boolean head) { _code = code; _contentType = contentType; @@ -65,17 +118,17 @@ public class HttpGeneratorServerTRTest _te = te; if (_contentType != null) - _fields.put("Content-Type",_contentType); + _fields.put("Content-Type", _contentType); if (_contentLength >= 0) - _fields.put("Content-Length","" + _contentLength); + _fields.put("Content-Length", "" + _contentLength); if (_connection != null) - _fields.put("Connection",_connection); + _fields.put("Connection", _connection); if (_te != null) - _fields.put("Transfer-Encoding",_te); + _fields.put("Transfer-Encoding", _te); if (_other != null) - _fields.put("Other",_other); + _fields.put("Other", _other); - ByteBuffer source = _body == null?null:BufferUtil.toBuffer(_body); + ByteBuffer source = _body == null ? null : BufferUtil.toBuffer(_body); ByteBuffer[] chunks = new ByteBuffer[nchunks]; ByteBuffer content = null; int c = 0; @@ -89,30 +142,27 @@ public class HttpGeneratorServerTRTest chunks[i - 1].limit(chunks[i].position()); } content = chunks[c++]; - // System.err.printf("content %d %s%n",c,BufferUtil.toDetailString(content)); } ByteBuffer header = null; ByteBuffer chunk = null; HttpGenerator.ResponseInfo info = null; - loop: while (true) + loop: + while (true) { // if we have unwritten content if (source != null && content != null && content.remaining() == 0 && c < nchunks) - { content = chunks[c++]; - // System.err.printf("content %d %s%n",c,BufferUtil.toDetailString(content)); - } // Generate boolean last = !BufferUtil.hasContent(content); - HttpGenerator.Result result = gen.generateResponse(info,header,chunk,content,last); + HttpGenerator.Result result = gen.generateResponse(info, header, chunk, content, last); switch (result) { case NEED_INFO: - info = new HttpGenerator.ResponseInfo(HttpVersion.fromVersion(version),_fields,_contentLength,_code,reason,_head); + info = new HttpGenerator.ResponseInfo(HttpVersion.fromVersion(version), _fields, _contentLength, _code, reason, _head); continue; case NEED_HEADER: @@ -157,7 +207,7 @@ public class HttpGeneratorServerTRTest @Override public String toString() { - return "[" + _code + "," + _contentType + "," + _contentLength + "," + (_body == null?"null":"content") + "]"; + return "[" + _code + "," + _contentType + "," + _contentLength + "," + (_body == null ? "null" : "content") + "]"; } public HttpFields getHttpFields() @@ -168,11 +218,6 @@ public class HttpGeneratorServerTRTest private class Handler implements HttpParser.ResponseHandler { - private final List _hdr = new ArrayList<>(); - private final List _val = new ArrayList<>(); - private int _status; - private HttpVersion _version; - @Override public boolean content(ByteBuffer ref) { @@ -204,16 +249,12 @@ public class HttpGeneratorServerTRTest @Override public boolean parsedHeader(HttpField field) { - _hdr.add(field.getName()); - _val.add(field.getValue()); return false; } @Override public boolean startResponse(HttpVersion version, int status, String reason) { - _version = version; - _status = status; _reason = reason; return false; } @@ -235,25 +276,25 @@ public class HttpGeneratorServerTRTest private static class Run { - public static Run[] as(TR tr, int ver, int chunks, ConnectionType connection) + public static Run[] as(Result result, int ver, int chunks, ConnectionType connection) { Run run = new Run(); - run.tr = tr; + run.result = result; run.httpVersion = ver; run.chunks = chunks; run.connection = connection; - return new Run[] { run }; + return new Run[]{run}; } - TR tr; - ConnectionType connection; - int httpVersion; - int chunks; + private Result result; + private ConnectionType connection; + private int httpVersion; + private int chunks; @Override public String toString() { - return String.format("tr=%s,ver=%d,chunks=%d,connection=%s",tr,httpVersion,chunks,connection.name()); + return String.format("result=%s,version=%d,chunks=%d,connection=%s", result, httpVersion, chunks, connection.name()); } } @@ -289,20 +330,21 @@ public class HttpGeneratorServerTRTest @Parameters(name = "{0}") public static Collection data() { - TR[] trs = { - /* 0 */new TR(200,null,-1,null,false), - /* 1 */new TR(200,null,-1,CONTENT,false), - /* 2 */new TR(200,null,CONTENT.length(),null,true), - /* 3 */new TR(200,null,CONTENT.length(),CONTENT,false), - /* 4 */new TR(200,"text/html",-1,null,true), - /* 5 */new TR(200,"text/html",-1,CONTENT,false), - /* 6 */new TR(200,"text/html",CONTENT.length(),null,true), - /* 7 */new TR(200,"text/html",CONTENT.length(),CONTENT,false), }; + Result[] results = { + new Result(200, null, -1, null, false), + new Result(200, null, -1, CONTENT, false), + new Result(200, null, CONTENT.length(), null, true), + new Result(200, null, CONTENT.length(), CONTENT, false), + new Result(200, "text/html", -1, null, true), + new Result(200, "text/html", -1, CONTENT, false), + new Result(200, "text/html", CONTENT.length(), null, true), + new Result(200, "text/html", CONTENT.length(), CONTENT, false) + }; List data = new ArrayList<>(); // For each test result - for (TR tr : trs) + for (Result result : results) { // Loop over HTTP versions for (int v = 9; v <= 11; v++) @@ -315,62 +357,12 @@ public class HttpGeneratorServerTRTest { if (connection.isSupportedByHttp(v)) { - data.add(Run.as(tr,v,chunks,connection)); + data.add(Run.as(result, v, chunks, connection)); } } } } } - return data; } - - @Parameter(value = 0) - public Run run; - - private String _content; - private String _reason; - - @Test - public void testHTTP() throws Exception - { - Handler handler = new Handler(); - - HttpGenerator gen = new HttpGenerator(); - - String t = run.toString(); - - run.tr.getHttpFields().clear(); - - String response = run.tr.build(run.httpVersion,gen,"OK\r\nTest",run.connection.val,null,run.chunks); - - if (run.httpVersion == 9) - { - assertFalse(t,gen.isPersistent()); - if (run.tr._body != null) - assertEquals(t,run.tr._body,response); - return; - } - - HttpParser parser = new HttpParser(handler); - parser.setHeadResponse(run.tr._head); - - parser.parseNext(BufferUtil.toBuffer(response)); - - if (run.tr._body != null) - assertEquals(t,run.tr._body,this._content); - - if (run.httpVersion == 10) - assertTrue(t,gen.isPersistent() || run.tr._contentLength >= 0 || run.connection == ConnectionType.CLOSE || run.connection == ConnectionType.NONE); - else - assertTrue(t,gen.isPersistent() || run.connection == ConnectionType.CLOSE || run.connection == ConnectionType.TE_CLOSE); - - if (run.httpVersion > 9) - assertEquals("OK??Test",_reason); - - if (_content == null) - assertTrue(t,run.tr._body == null); - else - assertThat(t,run.tr._contentLength,either(equalTo(_content.length())).or(equalTo(-1))); - } } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java index 9a9d4002b52..a65b3028806 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java @@ -19,8 +19,6 @@ package org.eclipse.jetty.http; import java.nio.ByteBuffer; -import java.util.ArrayList; -import java.util.List; import org.eclipse.jetty.http.HttpGenerator.ResponseInfo; import org.eclipse.jetty.util.BufferUtil; @@ -28,301 +26,25 @@ import org.junit.Assert; import org.junit.Test; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.either; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; public class HttpGeneratorServerTest { - private class Handler implements HttpParser.ResponseHandler - { - @Override - public boolean content(ByteBuffer ref) - { - if (_content == null) - _content = ""; - _content += BufferUtil.toString(ref); - ref.position(ref.limit()); - return false; - } - - @Override - public void earlyEOF() - { - } - - @Override - public boolean headerComplete() - { - _content = null; - return false; - } - - @Override - public boolean messageComplete() - { - return true; - } - - @Override - public boolean parsedHeader(HttpField field) - { - _hdr.add(field.getName()); - _val.add(field.getValue()); - return false; - } - - @Override - public boolean startResponse(HttpVersion version, int status, String reason) - { - _version = version; - _status = status; - _reason = reason; - return false; - } - - @Override - public void badMessage(int status, String reason) - { - throw new IllegalStateException(reason); - } - - @Override - public int getHeaderCacheSize() - { - return 256; - } - } - - private static class TR - { - private HttpFields _fields = new HttpFields(); - private final String _body; - private final int _code; - String _connection; - int _contentLength; - String _contentType; - private final boolean _head; - String _other; - String _te; - - private TR(int code, String contentType, int contentLength, String content, boolean head) - { - _code = code; - _contentType = contentType; - _contentLength = contentLength; - _other = "value"; - _body = content; - _head = head; - } - - private String build(int version, HttpGenerator gen, String reason, String connection, String te, int nchunks) throws Exception - { - String response = ""; - _connection = connection; - _te = te; - - if (_contentType != null) - _fields.put("Content-Type", _contentType); - if (_contentLength >= 0) - _fields.put("Content-Length", "" + _contentLength); - if (_connection != null) - _fields.put("Connection", _connection); - if (_te != null) - _fields.put("Transfer-Encoding", _te); - if (_other != null) - _fields.put("Other", _other); - - ByteBuffer source = _body == null ? null : BufferUtil.toBuffer(_body); - ByteBuffer[] chunks = new ByteBuffer[nchunks]; - ByteBuffer content = null; - int c = 0; - if (source != null) - { - for (int i = 0; i < nchunks; i++) - { - chunks[i] = source.duplicate(); - chunks[i].position(i * (source.capacity() / nchunks)); - if (i > 0) - chunks[i - 1].limit(chunks[i].position()); - } - content = chunks[c++]; - // System.err.printf("content %d %s%n",c,BufferUtil.toDetailString(content)); - } - ByteBuffer header = null; - ByteBuffer chunk = null; - HttpGenerator.ResponseInfo info = null; - - loop: - while (true) - { - // if we have unwritten content - if (source != null && content != null && content.remaining() == 0 && c < nchunks) - { - content = chunks[c++]; - // System.err.printf("content %d %s%n",c,BufferUtil.toDetailString(content)); - } - - // Generate - boolean last = !BufferUtil.hasContent(content); - - HttpGenerator.Result result = gen.generateResponse(info, header, chunk, content, last); - - switch (result) - { - case NEED_INFO: - info = new HttpGenerator.ResponseInfo(HttpVersion.fromVersion(version), _fields, _contentLength, _code, reason, _head); - continue; - - case NEED_HEADER: - header = BufferUtil.allocate(2048); - continue; - - case NEED_CHUNK: - chunk = BufferUtil.allocate(HttpGenerator.CHUNK_SIZE); - continue; - - case FLUSH: - if (BufferUtil.hasContent(header)) - { - response += BufferUtil.toString(header); - header.position(header.limit()); - } - if (BufferUtil.hasContent(chunk)) - { - response += BufferUtil.toString(chunk); - chunk.position(chunk.limit()); - } - if (BufferUtil.hasContent(content)) - { - response += BufferUtil.toString(content); - content.position(content.limit()); - } - break; - - case CONTINUE: - continue; - - case SHUTDOWN_OUT: - break; - - case DONE: - break loop; - } - } - return response; - } - - @Override - public String toString() - { - return "[" + _code + "," + _contentType + "," + _contentLength + "," + (_body == null ? "null" : "content") + "]"; - } - - public HttpFields getHttpFields() - { - return _fields; - } - } - - public final static String[] connections = {null, "keep-alive", "close", "TE, close"}; - public final static String CONTENT = "The quick brown fox jumped over the lazy dog.\nNow is the time for all good men to come to the aid of the party\nThe moon is blue to a fish in love.\n"; - - private final List _hdr = new ArrayList<>(); - private final List _val = new ArrayList<>(); - private String _content; - private String _reason; - private int _status; - private HttpVersion _version; - private final TR[] tr = - { - /* 0 */ new TR(200, null, -1, null, false), - /* 1 */ new TR(200, null, -1, CONTENT, false), - /* 2 */ new TR(200, null, CONTENT.length(), null, true), - /* 3 */ new TR(200, null, CONTENT.length(), CONTENT, false), - /* 4 */ new TR(200, "text/html", -1, null, true), - /* 5 */ new TR(200, "text/html", -1, CONTENT, false), - /* 6 */ new TR(200, "text/html", CONTENT.length(), null, true), - /* 7 */ new TR(200, "text/html", CONTENT.length(), CONTENT, false), - }; - - @Test - public void testHTTP() throws Exception - { - Handler handler = new Handler(); - - // Loop over HTTP versions - for (int v = 9; v <= 11; v++) - { - // For each test result - for (int r = 0; r < tr.length; r++) - { - HttpGenerator gen = new HttpGenerator(); - - // Loop over chunks - for (int chunks = 1; chunks <= 6; chunks++) - { - // Loop over Connection values - for (int c = 0; c < (v == 11 ? connections.length : (connections.length - 1)); c++) - { - String t = "v=" + v + ",chunks=" + chunks + ",connection=" + connections[c] + ",tr=" + r + "=" + tr[r]; - - gen.reset(); - tr[r].getHttpFields().clear(); - - String response = tr[r].build(v, gen, "OK\r\nTest", connections[c], null, chunks); - - if (v == 9) - { - assertFalse(t, gen.isPersistent()); - if (tr[r]._body != null) - assertEquals(t, tr[r]._body, response); - continue; - } - - HttpParser parser = new HttpParser(handler); - parser.setHeadResponse(tr[r]._head); - - parser.parseNext(BufferUtil.toBuffer(response)); - - if (tr[r]._body != null) - assertEquals(t, tr[r]._body, this._content); - - if (v == 10) - assertTrue(t, gen.isPersistent() || tr[r]._contentLength >= 0 || c == 2 || c == 1 || c == 0); - else - assertTrue(t, gen.isPersistent() || c == 2 || c == 3); - - if (v > 9) - assertEquals("OK??Test", _reason); - - if (_content == null) - assertTrue(t, tr[r]._body == null); - else - assertThat(t, tr[r]._contentLength, either(equalTo(_content.length())).or(equalTo(-1))); - } - } - } - } - } - @Test public void testSendServerXPoweredBy() throws Exception { ByteBuffer header = BufferUtil.allocate(8096); ResponseInfo info = new ResponseInfo(HttpVersion.HTTP_1_1, new HttpFields(), -1, 200, null, false); HttpFields fields = new HttpFields(); - fields.add(HttpHeader.SERVER,"SomeServer"); - fields.add(HttpHeader.X_POWERED_BY,"SomePower"); + fields.add(HttpHeader.SERVER, "SomeServer"); + fields.add(HttpHeader.X_POWERED_BY, "SomePower"); ResponseInfo infoF = new ResponseInfo(HttpVersion.HTTP_1_1, fields, -1, 200, null, false); String head; - - HttpGenerator gen = new HttpGenerator(true,true); + + HttpGenerator gen = new HttpGenerator(true, true); gen.generateResponse(info, header, null, null, true); head = BufferUtil.toString(header); BufferUtil.clear(header); @@ -339,8 +61,8 @@ public class HttpGeneratorServerTest assertThat(head, containsString("X-Powered-By: Jetty(9.x.x)")); assertThat(head, containsString("X-Powered-By: SomePower")); gen.reset(); - - gen = new HttpGenerator(false,false); + + gen = new HttpGenerator(false, false); gen.generateResponse(info, header, null, null, true); head = BufferUtil.toString(header); BufferUtil.clear(header); @@ -356,7 +78,7 @@ public class HttpGeneratorServerTest assertThat(head, containsString("Server: SomeServer")); assertThat(head, not(containsString("X-Powered-By: Jetty(9.x.x)"))); assertThat(head, containsString("X-Powered-By: SomePower")); - gen.reset(); + gen.reset(); } @Test @@ -453,7 +175,7 @@ public class HttpGeneratorServerTest out += BufferUtil.toString(content0); BufferUtil.clear(content0); - result = gen.generateResponse(null,null,chunk, content1, false); + result = gen.generateResponse(null, null, chunk, content1, false); assertEquals(HttpGenerator.Result.FLUSH, result); assertEquals(HttpGenerator.State.COMMITTED, gen.getState()); out += BufferUtil.toString(chunk); @@ -461,17 +183,17 @@ public class HttpGeneratorServerTest out += BufferUtil.toString(content1); BufferUtil.clear(content1); - result = gen.generateResponse(null,null,chunk, null, true); + result = gen.generateResponse(null, null, chunk, null, true); assertEquals(HttpGenerator.Result.CONTINUE, result); assertEquals(HttpGenerator.State.COMPLETING, gen.getState()); - result = gen.generateResponse(null,null,chunk, null, true); + result = gen.generateResponse(null, null, chunk, null, true); assertEquals(HttpGenerator.Result.FLUSH, result); assertEquals(HttpGenerator.State.COMPLETING, gen.getState()); out += BufferUtil.toString(chunk); BufferUtil.clear(chunk); - result = gen.generateResponse(null,null,chunk, null, true); + result = gen.generateResponse(null, null, chunk, null, true); assertEquals(HttpGenerator.Result.DONE, result); assertEquals(HttpGenerator.State.END, gen.getState());