Repository: calcite-avatica Updated Branches: refs/heads/master cc3605e2a -> 160e1a938
[CALCITE-2086] Increased max allowed HTTP header size to 64KB This commit also allows configuration of this HTTP header max size via HttpServer.Builder. Downstream users can modify this based on their requirements, although 64KB should be sufficient for the present. Closes #21 Project: http://git-wip-us.apache.org/repos/asf/calcite-avatica/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite-avatica/commit/160e1a93 Tree: http://git-wip-us.apache.org/repos/asf/calcite-avatica/tree/160e1a93 Diff: http://git-wip-us.apache.org/repos/asf/calcite-avatica/diff/160e1a93 Branch: refs/heads/master Commit: 160e1a9382935997ba5a5ae42510d32734b76906 Parents: cc3605e Author: Josh Elser <[email protected]> Authored: Mon Dec 11 17:24:18 2017 -0500 Committer: Josh Elser <[email protected]> Committed: Mon Dec 11 19:27:05 2017 -0500 ---------------------------------------------------------------------- .travis.yml | 7 +- .../avatica/remote/AvaticaHttpClientImpl.java | 4 + .../calcite/avatica/server/HttpServer.java | 43 +++++- .../avatica/remote/RemoteHttpClientTest.java | 134 +++++++++++++++++++ 4 files changed, 183 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite-avatica/blob/160e1a93/.travis.yml ---------------------------------------------------------------------- diff --git a/.travis.yml b/.travis.yml index 583fb23..f719eab 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,7 +22,9 @@ matrix: include: - jdk: oraclejdk8 - jdk: openjdk7 - - jdk: ibmjava8 + # Travis appears to have broken IBM JDK. Can only get a JRE. + # Needs to be investigate. + # - jdk: ibmjava8 branches: only: - master @@ -44,5 +46,6 @@ cache: directories: - $HOME/.m2 sudo: required -group: edge +# Added to enable IBM Java. Temporarily removed. +#group: edge # End .travis.yml http://git-wip-us.apache.org/repos/asf/calcite-avatica/blob/160e1a93/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientImpl.java b/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientImpl.java index c100eec..6fda179 100644 --- a/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientImpl.java +++ b/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientImpl.java @@ -55,6 +55,10 @@ public class AvaticaHttpClientImpl implements AvaticaHttpClient { continue; } else if (responseCode != HttpURLConnection.HTTP_OK) { inputStream = connection.getErrorStream(); + if (inputStream == null) { + // HTTP Transport exception that resulted in no content coming back + throw new RuntimeException("Failed to read data from the server: HTTP/" + responseCode); + } } else { inputStream = connection.getInputStream(); } http://git-wip-us.apache.org/repos/asf/calcite-avatica/blob/160e1a93/server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java ---------------------------------------------------------------------- diff --git a/server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java b/server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java index 217cbeb..7ffb181 100644 --- a/server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java +++ b/server/src/main/java/org/apache/calcite/avatica/server/HttpServer.java @@ -29,8 +29,10 @@ import org.eclipse.jetty.security.HashLoginService; import org.eclipse.jetty.security.LoginService; import org.eclipse.jetty.security.authentication.BasicAuthenticator; import org.eclipse.jetty.security.authentication.DigestAuthenticator; +import org.eclipse.jetty.server.AbstractConnectionFactory; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.DefaultHandler; @@ -65,6 +67,7 @@ import javax.security.auth.login.LoginException; */ public class HttpServer { private static final Logger LOG = LoggerFactory.getLogger(HttpServer.class); + private static final int MAX_ALLOWED_HEADER_SIZE = 1024 * 64; private Server server; private int port = -1; @@ -72,6 +75,7 @@ public class HttpServer { private final AvaticaServerConfiguration config; private final Subject subject; private final SslContextFactory sslFactory; + private final int maxAllowedHeaderSize; @Deprecated public HttpServer(Handler handler) { @@ -132,11 +136,26 @@ public class HttpServer { */ public HttpServer(int port, AvaticaHandler handler, AvaticaServerConfiguration config, Subject subject, SslContextFactory sslFactory) { + this(port, handler, config, subject, sslFactory, MAX_ALLOWED_HEADER_SIZE); + } + + /** + * Constructs an {@link HttpServer}. + * @param port The listen port + * @param handler The Handler to run + * @param config Optional configuration for the server + * @param subject The javax.security Subject for the server, or null + * @param sslFactory A configured SslContextFactory, or null + * @param maxAllowedHeaderSize A maximum size in bytes that are allowed in an HTTP header + */ + public HttpServer(int port, AvaticaHandler handler, AvaticaServerConfiguration config, + Subject subject, SslContextFactory sslFactory, int maxAllowedHeaderSize) { this.port = port; this.handler = handler; this.config = config; this.subject = subject; this.sslFactory = sslFactory; + this.maxAllowedHeaderSize = maxAllowedHeaderSize; } static AvaticaHandler wrapJettyHandler(Handler handler) { @@ -226,10 +245,13 @@ public class HttpServer { } private ServerConnector getConnector() { + HttpConnectionFactory factory = new HttpConnectionFactory(); + factory.getHttpConfiguration().setRequestHeaderSize(maxAllowedHeaderSize); + if (null == sslFactory) { - return new ServerConnector(server); + return new ServerConnector(server, factory); } - return new ServerConnector(server, sslFactory); + return new ServerConnector(server, AbstractConnectionFactory.getFactories(sslFactory, factory)); } private RpcMetadataResponse createRpcServerMetadata(ServerConnector connector) throws @@ -411,6 +433,9 @@ public class HttpServer { private File truststore; private String truststorePassword; + // The maximum size in bytes of an http header the server will read (64KB) + private int maxAllowedHeaderSize = MAX_ALLOWED_HEADER_SIZE; + public Builder() {} public Builder withPort(int port) { @@ -622,6 +647,17 @@ public class HttpServer { } /** + * Configures the maximum size, in bytes, of an HTTP header that the server will read. + * + * @param maxHeaderSize Maximums HTTP header size in bytes + * @return <code>this</code> + */ + public Builder withMaxHeaderSize(int maxHeaderSize) { + this.maxAllowedHeaderSize = maxHeaderSize; + return this; + } + + /** * Builds the HttpServer instance from <code>this</code>. * @return An HttpServer. */ @@ -666,7 +702,8 @@ public class HttpServer { sslFactory.setTrustStorePassword(truststorePassword); } - return new HttpServer(port, handler, serverConfig, subject, sslFactory); + return new HttpServer(port, handler, serverConfig, subject, sslFactory, + maxAllowedHeaderSize); } /** http://git-wip-us.apache.org/repos/asf/calcite-avatica/blob/160e1a93/server/src/test/java/org/apache/calcite/avatica/remote/RemoteHttpClientTest.java ---------------------------------------------------------------------- diff --git a/server/src/test/java/org/apache/calcite/avatica/remote/RemoteHttpClientTest.java b/server/src/test/java/org/apache/calcite/avatica/remote/RemoteHttpClientTest.java new file mode 100644 index 0000000..25d7c7a --- /dev/null +++ b/server/src/test/java/org/apache/calcite/avatica/remote/RemoteHttpClientTest.java @@ -0,0 +1,134 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.avatica.remote; + +import org.apache.calcite.avatica.ConnectionConfig; +import org.apache.calcite.avatica.ConnectionSpec; +import org.apache.calcite.avatica.server.HttpServer; + +import org.junit.AfterClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.URL; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.Statement; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.Assert.assertFalse; + +/** + * Tests for the HttpClient which manipulate things at the HTTP layer instead of Avatica's + * "application" layer. + */ +@RunWith(Parameterized.class) +public class RemoteHttpClientTest { + private static Map<String, String> getHeaders() { + return HEADERS_REF.get(); + } + + private static final AtomicReference<Map<String, String>> HEADERS_REF = new AtomicReference<>(); + private static final AvaticaServersForTest SERVERS = new AvaticaServersForTest(); + + @Parameters(name = "{0}") + public static List<Object[]> parameters() throws Exception { + SERVERS.startServers(); + return SERVERS.getJUnitParameters(); + } + + @AfterClass public static void afterClass() throws Exception { + if (null != SERVERS) { + SERVERS.stopServers(); + } + } + + private final HttpServer server; + private final String url; + private final int port; + private final Driver.Serialization serialization; + + public RemoteHttpClientTest(Driver.Serialization serialization, HttpServer server) { + this.server = server; + this.port = this.server.getPort(); + this.serialization = serialization; + this.url = SERVERS.getJdbcUrl(port, serialization); + } + + static String generateString(int length) { + StringBuilder sb = new StringBuilder(length); + for (int i = 0; i < length; i++) { + sb.append('a'); + } + return sb.toString(); + } + + @Test public void testLargeHeaders() throws Exception { + ConnectionSpec.getDatabaseLock().lock(); + // 32K header. Relies on the default value of 64K from HttpServer.Builder + HEADERS_REF.set(Collections.singletonMap("MyLargeHeader", generateString(1024 * 32))); + String modifiedUrl = url + ";httpclient_factory=" + HttpClientFactoryForTest.class.getName(); + try (Connection conn = DriverManager.getConnection(modifiedUrl); + Statement stmt = conn.createStatement()) { + assertFalse(stmt.execute("CREATE TABLE largeHeaders(pk integer not null primary key)")); + assertFalse(stmt.execute("DROP TABLE largeHeaders")); + } finally { + ConnectionSpec.getDatabaseLock().unlock(); + } + } + + /** + * Factory class to inject a custom HttpClient + */ + public static class HttpClientFactoryForTest implements AvaticaHttpClientFactory { + @Override public AvaticaHttpClient getClient(URL url, ConnectionConfig config, + KerberosConnection kerberosUtil) { + Map<String, String> headers = getHeaders(); + return new HeaderInjectingHttpClient(headers, url); + } + } + + /** + * HttpClient implementation which supports injecting custom HTTP headers + */ + public static class HeaderInjectingHttpClient extends AvaticaHttpClientImpl { + private final Map<String, String> headers; + + public HeaderInjectingHttpClient(Map<String, String> headers, URL url) { + super(url); + this.headers = Objects.requireNonNull(headers); + } + + @Override HttpURLConnection openConnection() throws IOException { + HttpURLConnection connection = super.openConnection(); + for (Entry<String, String> entry : headers.entrySet()) { + connection.setRequestProperty(entry.getKey(), entry.getValue()); + } + return connection; + } + } +} +// End RemoteHttpClientTest.java
