From 33eb768d6972fe67b5058c96941645cb03fea6e7 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 25 Oct 2016 15:44:54 +0200 Subject: [PATCH] Fixes #1029 - Restore Request.setHttpVersion(). Also cleaned up the asymmetry in MetaData between the setter (setHttpVersion()) and the getter (getVersion()). --- .../org/eclipse/jetty/http/HttpGenerator.java | 16 ++-- .../java/org/eclipse/jetty/http/MetaData.java | 17 +++- .../eclipse/jetty/http2/hpack/HpackTest.java | 10 +-- .../client/http/HttpReceiverOverHTTP2.java | 2 +- .../http2/server/HttpChannelOverHTTP2.java | 6 +- .../org/eclipse/jetty/server/HttpChannel.java | 4 +- .../jetty/server/HttpChannelOverHttp.java | 6 +- .../org/eclipse/jetty/server/Request.java | 12 ++- .../server/HttpVersionCustomizerTest.java | 81 +++++++++++++++++++ 9 files changed, 125 insertions(+), 29 deletions(-) create mode 100644 jetty-server/src/test/java/org/eclipse/jetty/server/HttpVersionCustomizerTest.java diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java index b03133a96b4..0e5047c1066 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java @@ -214,7 +214,7 @@ public class HttpGenerator // If we have not been told our persistence, set the default if (_persistent==null) { - _persistent=info.getVersion().ordinal() > HttpVersion.HTTP_1_0.ordinal(); + _persistent=info.getHttpVersion().ordinal() > HttpVersion.HTTP_1_0.ordinal(); if (!_persistent && HttpMethod.CONNECT.is(info.getMethod())) _persistent=true; } @@ -226,7 +226,7 @@ public class HttpGenerator // generate ResponseLine generateRequestLine(info,header); - if (info.getVersion()==HttpVersion.HTTP_0_9) + if (info.getHttpVersion()==HttpVersion.HTTP_0_9) throw new BadMessageException(500,"HTTP/0.9 not supported"); generateHeaders(info,header,content,last); @@ -342,7 +342,7 @@ public class HttpGenerator { if (info==null) return Result.NEED_INFO; - HttpVersion version=info.getVersion(); + HttpVersion version=info.getHttpVersion(); if (version==null) throw new BadMessageException(500,"No version"); switch(version) @@ -523,7 +523,7 @@ public class HttpGenerator header.put((byte)' '); header.put(StringUtil.getBytes(request.getURIString())); header.put((byte)' '); - header.put(request.getVersion().toBytes()); + header.put(request.getHttpVersion().toBytes()); header.put(HttpTokens.CRLF); } @@ -628,7 +628,7 @@ public class HttpGenerator case TRANSFER_ENCODING: { - if (_info.getVersion() == HttpVersion.HTTP_1_1) + if (_info.getHttpVersion() == HttpVersion.HTTP_1_1) transfer_encoding = field; // Do NOT add yet! break; @@ -682,7 +682,7 @@ public class HttpGenerator case KEEP_ALIVE: { - if (_info.getVersion() == HttpVersion.HTTP_1_0) + if (_info.getHttpVersion() == HttpVersion.HTTP_1_0) { keep_alive = true; if (response!=null) @@ -774,7 +774,7 @@ public class HttpGenerator // For a request with HTTP 1.0 & Connection: keep-alive // we *must* close the connection, otherwise the client // has no way to detect the end of the content. - if (!isPersistent() || _info.getVersion().ordinal() < HttpVersion.HTTP_1_1.ordinal()) + if (!isPersistent() || _info.getHttpVersion().ordinal() < HttpVersion.HTTP_1_1.ordinal()) _endOfContent = EndOfContent.EOF_CONTENT; } break; @@ -825,7 +825,7 @@ public class HttpGenerator // If this is a response, work out persistence if (response!=null) { - if (!isPersistent() && (close || _info.getVersion().ordinal() > HttpVersion.HTTP_1_0.ordinal())) + if (!isPersistent() && (close || _info.getHttpVersion().ordinal() > HttpVersion.HTTP_1_0.ordinal())) { if (connection==null) header.put(CONNECTION_CLOSE); diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java b/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java index 725870db92b..9f683503d0d 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java @@ -57,10 +57,19 @@ public class MetaData implements Iterable return false; } + /** + * @deprecated use {@link #getHttpVersion()} instead + */ + @Deprecated + public HttpVersion getVersion() + { + return getHttpVersion(); + } + /** * @return the HTTP version of this MetaData object */ - public HttpVersion getVersion() + public HttpVersion getHttpVersion() { return _httpVersion; } @@ -155,7 +164,7 @@ public class MetaData implements Iterable public Request(Request request) { - this(request.getMethod(),new HttpURI(request.getURI()), request.getVersion(), new HttpFields(request.getFields()), request.getContentLength()); + this(request.getMethod(),new HttpURI(request.getURI()), request.getHttpVersion(), new HttpFields(request.getFields()), request.getContentLength()); } // TODO MetaData should be immuttable!!! @@ -218,7 +227,7 @@ public class MetaData implements Iterable { HttpFields fields = getFields(); return String.format("%s{u=%s,%s,h=%d}", - getMethod(), getURI(), getVersion(), fields == null ? -1 : fields.size()); + getMethod(), getURI(), getHttpVersion(), fields == null ? -1 : fields.size()); } } @@ -292,7 +301,7 @@ public class MetaData implements Iterable public String toString() { HttpFields fields = getFields(); - return String.format("%s{s=%d,h=%d}", getVersion(), getStatus(), fields == null ? -1 : fields.size()); + return String.format("%s{s=%d,h=%d}", getHttpVersion(), getStatus(), fields == null ? -1 : fields.size()); } } } diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java index fce577bbb57..353396d48c0 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java @@ -18,10 +18,6 @@ package org.eclipse.jetty.http2.hpack; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; - import java.nio.ByteBuffer; import org.eclipse.jetty.http.BadMessageException; @@ -38,6 +34,10 @@ import org.eclipse.jetty.util.BufferUtil; import org.junit.Assert; import org.junit.Test; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + public class HpackTest { final static HttpField ServerJetty = new PreEncodedHttpField(HttpHeader.SERVER,"jetty"); @@ -187,7 +187,7 @@ public class HpackTest private void assertMetadataSame(MetaData expected, MetaData actual) { assertThat("Metadata.contentLength",actual.getContentLength(),is(expected.getContentLength())); - assertThat("Metadata.version" + ".version", actual.getVersion(), is(expected.getVersion())); + assertThat("Metadata.version" + ".version", actual.getHttpVersion(),is(expected.getHttpVersion())); assertHttpFieldsSame("Metadata.fields",expected.getFields(),actual.getFields()); } diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java index f95c09d429f..be8296a615f 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java @@ -67,7 +67,7 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements Stream.Listen HttpResponse response = exchange.getResponse(); MetaData.Response metaData = (MetaData.Response)frame.getMetaData(); - response.version(metaData.getVersion()).status(metaData.getStatus()).reason(metaData.getReason()); + response.version(metaData.getHttpVersion()).status(metaData.getStatus()).reason(metaData.getReason()); if (responseBegin(exchange)) { diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpChannelOverHTTP2.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpChannelOverHTTP2.java index 02ba47cf825..7d6558a333e 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpChannelOverHTTP2.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpChannelOverHTTP2.java @@ -126,7 +126,7 @@ public class HttpChannelOverHTTP2 extends HttpChannel LOG.debug("HTTP2 Request #{}/{}, delayed={}:{}{} {} {}{}{}", stream.getId(), Integer.toHexString(stream.getSession().hashCode()), _delayedUntilContent, System.lineSeparator(), - request.getMethod(), request.getURI(), request.getVersion(), + request.getMethod(), request.getURI(), request.getHttpVersion(), System.lineSeparator(), fields); } @@ -157,7 +157,7 @@ public class HttpChannelOverHTTP2 extends HttpChannel Stream stream = getStream(); LOG.debug("HTTP2 PUSH Request #{}/{}:{}{} {} {}{}{}", stream.getId(), Integer.toHexString(stream.getSession().hashCode()), System.lineSeparator(), - request.getMethod(), request.getURI(), request.getVersion(), + request.getMethod(), request.getURI(), request.getHttpVersion(), System.lineSeparator(), request.getFields()); } @@ -199,7 +199,7 @@ public class HttpChannelOverHTTP2 extends HttpChannel { Stream stream = getStream(); LOG.debug("HTTP2 Commit Response #{}/{}:{}{} {} {}{}{}", - stream.getId(), Integer.toHexString(stream.getSession().hashCode()), System.lineSeparator(), info.getVersion(), info.getStatus(), info.getReason(), + stream.getId(), Integer.toHexString(stream.getSession().hashCode()), System.lineSeparator(), info.getHttpVersion(), info.getStatus(), info.getReason(), System.lineSeparator(), info.getFields()); } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index 88b61a5e0aa..9c1e297fdb1 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -606,7 +606,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor if (LOG.isDebugEnabled()) LOG.debug("REQUEST for {} on {}{}{} {} {}{}{}",request.getURIString(),this,System.lineSeparator(), - request.getMethod(),request.getURIString(),request.getVersion(),System.lineSeparator(), + request.getMethod(),request.getURIString(),request.getHttpVersion(),System.lineSeparator(), request.getFields()); } @@ -752,7 +752,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor _committedMetaData=info; if (LOG.isDebugEnabled()) LOG.debug("COMMIT for {} on {}{}{} {} {}{}{}",getRequest().getRequestURI(),this,System.lineSeparator(), - info.getStatus(),info.getReason(),info.getVersion(),System.lineSeparator(), + info.getStatus(),info.getReason(),info.getHttpVersion(),System.lineSeparator(), info.getFields()); } 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 6668678874c..33e63508176 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 @@ -136,7 +136,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque case EXPECT: { - if (_metadata.getVersion() == HttpVersion.HTTP_1_1) + if (_metadata.getHttpVersion() == HttpVersion.HTTP_1_1) { HttpHeaderValue expect = HttpHeaderValue.CACHE.get(value); switch (expect == null ? HttpHeaderValue.UNKNOWN : expect) @@ -266,7 +266,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque boolean persistent; - switch (_metadata.getVersion()) + switch (_metadata.getHttpVersion()) { case HTTP_0_9: { @@ -350,7 +350,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque default: { - throw new IllegalStateException("unsupported version " + _metadata.getVersion()); + throw new IllegalStateException("unsupported version " + _metadata.getHttpVersion()); } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 05492304feb..8f64c3c5074 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -80,7 +80,6 @@ import org.eclipse.jetty.server.handler.ContextHandler.Context; import org.eclipse.jetty.server.session.AbstractSession; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.AttributesMap; -import org.eclipse.jetty.util.HostPort; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.MultiPartInputStreamParser; @@ -1086,7 +1085,7 @@ public class Request implements HttpServletRequest MetaData.Request metadata = _metaData; if (metadata==null) return null; - HttpVersion version = metadata.getVersion(); + HttpVersion version = metadata.getHttpVersion(); if (version==null) return null; return version.toString(); @@ -1099,7 +1098,7 @@ public class Request implements HttpServletRequest public HttpVersion getHttpVersion() { MetaData.Request metadata = _metaData; - return metadata==null?null:metadata.getVersion(); + return metadata==null?null:metadata.getHttpVersion(); } /* ------------------------------------------------------------ */ @@ -2066,6 +2065,13 @@ public class Request implements HttpServletRequest metadata.setMethod(method); } + public void setHttpVersion(HttpVersion version) + { + MetaData.Request metadata = _metaData; + if (metadata!=null) + metadata.setHttpVersion(version); + } + /* ------------------------------------------------------------ */ public boolean isHead() { diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpVersionCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpVersionCustomizerTest.java new file mode 100644 index 00000000000..819354879f0 --- /dev/null +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpVersionCustomizerTest.java @@ -0,0 +1,81 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.server; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.channels.SocketChannel; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.toolchain.test.TestTracker; +import org.hamcrest.Matchers; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; + +public class HttpVersionCustomizerTest +{ + @Rule + public TestTracker tracker = new TestTracker(); + + @Test + public void testCustomizeHttpVersion() throws Exception + { + Server server = new Server(); + HttpConfiguration httpConfig = new HttpConfiguration(); + httpConfig.addCustomizer((connector, config, request) -> request.setHttpVersion(HttpVersion.HTTP_1_1)); + ServerConnector connector = new ServerConnector(server, new HttpConnectionFactory(httpConfig)); + server.addConnector(connector); + server.setHandler(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + Assert.assertEquals(HttpVersion.HTTP_1_1.asString(), request.getProtocol()); + } + }); + server.start(); + + try + { + try (SocketChannel socket = SocketChannel.open(new InetSocketAddress("localhost", connector.getLocalPort()))) + { + HttpTester.Request request = HttpTester.newRequest(); + request.setVersion(HttpVersion.HTTP_1_0); + socket.write(request.generate()); + + HttpTester.Response response = HttpTester.parseResponse(HttpTester.from(socket)); + Assert.assertNotNull(response); + Assert.assertThat(response.getStatus(), Matchers.equalTo(HttpStatus.OK_200)); + } + } + finally + { + server.stop(); + } + } +}