From 9d2263394219d47493e162a411345771357380c6 Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Mon, 7 Oct 2019 17:25:08 +0100 Subject: [PATCH] Fix ineffective HTTP initial line length attribute. --- .../com/hotels/styx/client/HttpConfig.java | 20 ++--- .../netty/connectionpool/NettyConnection.java | 2 +- .../hotels/styx/admin/AdminServerConfig.java | 4 +- .../styx/proxy/ProxyConnectorFactory.java | 2 +- .../hotels/styx/proxy/ProxyServerConfig.java | 6 +- .../styx/server/netty/NettyServerConfig.java | 16 ++-- .../support/configuration/ProxyConfig.scala | 2 +- .../support/configuration/StyxConfig.scala | 4 +- .../hotels/styx/config/HttpSettingsSpec.kt | 88 +++++++++++++++++++ .../styx/resiliency/OriginResourcesSpec.kt | 7 +- 10 files changed, 121 insertions(+), 30 deletions(-) create mode 100644 system-tests/ft-suite/src/test/kotlin/com/hotels/styx/config/HttpSettingsSpec.kt diff --git a/components/client/src/main/java/com/hotels/styx/client/HttpConfig.java b/components/client/src/main/java/com/hotels/styx/client/HttpConfig.java index dee6dcc745..b94be59200 100644 --- a/components/client/src/main/java/com/hotels/styx/client/HttpConfig.java +++ b/components/client/src/main/java/com/hotels/styx/client/HttpConfig.java @@ -27,7 +27,7 @@ */ public final class HttpConfig { private boolean compress; - private final int maxInitialLineLength; + private final int maxInitialLength; private final int maxHeadersSize; private final int maxChunkSize; private int maxContentLength = 65536; @@ -36,15 +36,15 @@ public final class HttpConfig { @JsonCreator HttpConfig(@JsonProperty("maxChunkSize") Integer maxChunkSize, @JsonProperty("maxHeaderSize") Integer maxHeadersSize, - @JsonProperty("maxInitialLength") Integer maxInitialLineLength) { + @JsonProperty("maxInitialLength") Integer maxInitialLength) { this.maxChunkSize = firstNonNull(maxChunkSize, 8192); this.maxHeadersSize = firstNonNull(maxHeadersSize, 8192); - this.maxInitialLineLength = firstNonNull(maxInitialLineLength, 4096); + this.maxInitialLength = firstNonNull(maxInitialLength, 4096); } private HttpConfig(Builder builder) { this.compress = builder.compress; - this.maxInitialLineLength = builder.maxInitialLineLength; + this.maxInitialLength = builder.maxInitialLength; this.maxHeadersSize = builder.maxHeadersSize; this.maxChunkSize = builder.maxChunkSize; this.maxContentLength = builder.maxContentLength; @@ -65,8 +65,8 @@ public boolean compress() { * * @return maximum length of initial line */ - public int maxInitialLineLength() { - return maxInitialLineLength; + public int maxInitialLength() { + return maxInitialLength; } /** @@ -128,7 +128,7 @@ public static HttpConfig defaultHttpConfig() { */ public static final class Builder { private boolean compress; - private int maxInitialLineLength = 4096; + private int maxInitialLength = 4096; private int maxHeadersSize = 8192; private int maxChunkSize = 8192; private int maxContentLength = 65536; @@ -162,11 +162,11 @@ public Builder setMaxChunkSize(int maxChunkSize) { /** * Set the maximum length in bytes of the initial line of an HTTP message, e.g. {@code GET http://example.org/ HTTP/1.1}. * - * @param maxInitialLineLength maximum length in bytes of the initial line + * @param maxInitialLength maximum length in bytes of the initial line * @return this builder */ - public Builder setMaxInitialLineLength(int maxInitialLineLength) { - this.maxInitialLineLength = maxInitialLineLength; + public Builder setMaxInitialLength(int maxInitialLength) { + this.maxInitialLength = maxInitialLength; return this; } diff --git a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnection.java b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnection.java index 6590dcf2e3..e718a3d358 100644 --- a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnection.java +++ b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnection.java @@ -84,7 +84,7 @@ private static void addChannelHandlers(Channel channel, HttpConfig httpConfig, S pipeline.addLast("ssl", sslHandler); } - pipeline.addLast("http-codec", new HttpClientCodec(httpConfig.maxInitialLineLength(), httpConfig.maxHeadersSize(), httpConfig.maxChunkSize())); + pipeline.addLast("http-codec", new HttpClientCodec(httpConfig.maxInitialLength(), httpConfig.maxHeadersSize(), httpConfig.maxChunkSize())); if (httpConfig.compress()) { pipeline.addLast("decompressor", new HttpContentDecompressor()); } diff --git a/components/proxy/src/main/java/com/hotels/styx/admin/AdminServerConfig.java b/components/proxy/src/main/java/com/hotels/styx/admin/AdminServerConfig.java index 77ac5d140c..7e1e24f247 100644 --- a/components/proxy/src/main/java/com/hotels/styx/admin/AdminServerConfig.java +++ b/components/proxy/src/main/java/com/hotels/styx/admin/AdminServerConfig.java @@ -99,8 +99,8 @@ public Builder setNioAcceptorBacklog(Integer nioAcceptorBacklog) { } @Override - public Builder setMaxInitialLineLength(Integer maxInitialLineLength) { - return super.setMaxInitialLineLength(maxInitialLineLength); + public Builder setMaxInitialLength(Integer maxInitialLength) { + return super.setMaxInitialLength(maxInitialLength); } @Override diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyConnectorFactory.java b/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyConnectorFactory.java index acf8fa436f..09ee9b1bc4 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyConnectorFactory.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyConnectorFactory.java @@ -153,7 +153,7 @@ public void configure(Channel channel, HttpHandler httpPipeline) { .addLast("channel-stats", channelStatsHandler) // Http Server Codec - .addLast("http-server-codec", new HttpServerCodec(serverConfig.maxInitialLineLength(), serverConfig.maxHeaderSize(), serverConfig.maxChunkSize(), true)) + .addLast("http-server-codec", new HttpServerCodec(serverConfig.maxInitialLength(), serverConfig.maxHeaderSize(), serverConfig.maxChunkSize(), true)) // idle-handler and timeout-handler must be before aggregator. Otherwise // timeout handler cannot see the incoming HTTP chunks. diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyServerConfig.java b/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyServerConfig.java index 25a64b4aff..bac0760687 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyServerConfig.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/ProxyServerConfig.java @@ -71,9 +71,9 @@ public Builder setNioAcceptorBacklog(Integer nioAcceptorBacklog) { return this; } - @JsonProperty("maxInitialLineLength") - public Builder setMaxInitialLineLength(Integer maxInitialLineLength) { - builder.setMaxInitialLineLength(maxInitialLineLength); + @JsonProperty("maxInitialLength") + public Builder setMaxInitialLength(Integer maxInitialLength) { + builder.setMaxInitialLength(maxInitialLength); return this; } diff --git a/components/server/src/main/java/com/hotels/styx/server/netty/NettyServerConfig.java b/components/server/src/main/java/com/hotels/styx/server/netty/NettyServerConfig.java index b967cbdf07..bd3e916562 100644 --- a/components/server/src/main/java/com/hotels/styx/server/netty/NettyServerConfig.java +++ b/components/server/src/main/java/com/hotels/styx/server/netty/NettyServerConfig.java @@ -43,7 +43,7 @@ public class NettyServerConfig { private int workerThreadsCount = HALF_OF_AVAILABLE_PROCESSORS; private int nioAcceptorBacklog = 1024; - private int maxInitialLineLength = 4096; + private int maxInitialLength = 4096; private int maxHeaderSize = 8192; private int maxChunkSize = 8192; private int requestTimeoutMs = 12000; @@ -65,7 +65,7 @@ protected NettyServerConfig(Builder builder) { this.bossThreadsCount = firstNonNull(builder.bossThreadsCount, HALF_OF_AVAILABLE_PROCESSORS); this.workerThreadsCount = firstNonNull(builder.workerThreadsCount, HALF_OF_AVAILABLE_PROCESSORS); this.nioAcceptorBacklog = firstNonNull(builder.nioAcceptorBacklog, 1024); - this.maxInitialLineLength = firstNonNull(builder.maxInitialLineLength, 4096); + this.maxInitialLength = firstNonNull(builder.maxInitialLength, 4096); this.maxHeaderSize = firstNonNull(builder.maxHeaderSize, 8192); this.maxChunkSize = firstNonNull(builder.maxChunkSize, 8192); this.requestTimeoutMs = firstNonNull(builder.requestTimeoutMs, 12000); @@ -128,8 +128,8 @@ public int nioAcceptorBacklog() { * * @return maximum length of initial line */ - public int maxInitialLineLength() { - return this.maxInitialLineLength; + public int maxInitialLength() { + return this.maxInitialLength; } /** @@ -189,7 +189,7 @@ public static class Builder> { protected Integer bossThreadsCount; protected Integer workerThreadsCount; protected Integer nioAcceptorBacklog; - protected Integer maxInitialLineLength; + protected Integer maxInitialLength; protected Integer maxHeaderSize; protected Integer maxChunkSize; protected Integer requestTimeoutMs; @@ -220,9 +220,9 @@ public T setNioAcceptorBacklog(Integer nioAcceptorBacklog) { return (T) this; } - @JsonProperty("maxInitialLineLength") - public T setMaxInitialLineLength(Integer maxInitialLineLength) { - this.maxInitialLineLength = maxInitialLineLength; + @JsonProperty("maxInitialLength") + public T setMaxInitialLength(Integer maxInitialLength) { + this.maxInitialLength = maxInitialLength; return (T) this; } diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/support/configuration/ProxyConfig.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/support/configuration/ProxyConfig.scala index dfad5b5a4a..3682703d48 100644 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/support/configuration/ProxyConfig.scala +++ b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/support/configuration/ProxyConfig.scala @@ -23,7 +23,7 @@ case class ProxyConfig(connectors: Connectors = Connectors(HttpConnectorConfig() bossThreadCount: Int = 1, workerThreadsCount: Int = 1, nioAcceptorBacklog: Int = proxyServerDefaults.nioAcceptorBacklog(), - maxInitialLineLength: Int = proxyServerDefaults.maxInitialLineLength(), + maxInitialLength: Int = proxyServerDefaults.maxInitialLength(), maxHeaderSize: Int = proxyServerDefaults.maxHeaderSize(), maxChunkSize: Int = proxyServerDefaults.maxChunkSize(), requestTimeoutMillis: Int = proxyServerDefaults.requestTimeoutMillis(), diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/support/configuration/StyxConfig.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/support/configuration/StyxConfig.scala index 6ed1ffae98..8a8b2a36c5 100644 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/support/configuration/StyxConfig.scala +++ b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/support/configuration/StyxConfig.scala @@ -80,7 +80,7 @@ case class StyxConfig(proxyConfig: ProxyConfig = ProxyConfig(), .setMaxChunkSize(proxyConfig.maxChunkSize) .setMaxConnectionsCount(proxyConfig.maxConnectionsCount) .setMaxHeaderSize(proxyConfig.maxHeaderSize) - .setMaxInitialLineLength(proxyConfig.maxInitialLineLength) + .setMaxInitialLength(proxyConfig.maxInitialLength) .setNioAcceptorBacklog(proxyConfig.nioAcceptorBacklog) .setRequestTimeoutMillis(proxyConfig.requestTimeoutMillis) .setClientWorkerThreadsCount(proxyConfig.clientWorkerThreadsCount) @@ -112,7 +112,7 @@ case class StyxConfig(proxyConfig: ProxyConfig = ProxyConfig(), .setMaxChunkSize(proxyConfig.maxChunkSize) .setMaxConnectionsCount(proxyConfig.maxConnectionsCount) .setMaxHeaderSize(proxyConfig.maxHeaderSize) - .setMaxInitialLineLength(proxyConfig.maxInitialLineLength) + .setMaxInitialLength(proxyConfig.maxInitialLength) .setNioAcceptorBacklog(proxyConfig.nioAcceptorBacklog) .setRequestTimeoutMillis(proxyConfig.requestTimeoutMillis) .setClientWorkerThreadsCount(proxyConfig.clientWorkerThreadsCount) diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/config/HttpSettingsSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/config/HttpSettingsSpec.kt new file mode 100644 index 0000000000..38c0eed63b --- /dev/null +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/config/HttpSettingsSpec.kt @@ -0,0 +1,88 @@ +/* + Copyright (C) 2013-2019 Expedia Inc. + + Licensed 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 com.hotels.styx.config + +import com.hotels.styx.api.HttpHeaderNames.HOST +import com.hotels.styx.api.HttpRequest +import com.hotels.styx.api.HttpResponseStatus.OK +import com.hotels.styx.api.HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE +import com.hotels.styx.client.StyxHttpClient +import com.hotels.styx.support.StyxServerProvider +import com.hotels.styx.support.proxyHttpHostHeader +import com.hotels.styx.support.wait +import io.kotlintest.Spec +import io.kotlintest.matchers.boolean.shouldBeTrue +import io.kotlintest.shouldBe +import io.kotlintest.specs.FeatureSpec +import java.nio.charset.StandardCharsets.UTF_8 + +class HttpSettingsSpec : FeatureSpec() { + + init { + feature("Initial Line Length") { + styxServer.restart() + + scenario("Accepts requests within max initial length") { + val response = client.send(HttpRequest.get("/a/" + "b".repeat(80)) + .header(HOST, styxServer().proxyHttpHostHeader()) + .build()) + .wait()!! + + response.status() shouldBe OK + response.bodyAs(UTF_8) shouldBe "Test origin." + + } + + scenario("Rejects requests exceeding the initial line length") { + val response = client.send(HttpRequest.get("/a/" + "b".repeat(95)) + .header(HOST, styxServer().proxyHttpHostHeader()) + .build())!! + .wait()!! + + response.status() shouldBe REQUEST_ENTITY_TOO_LARGE + response.bodyAs(UTF_8) shouldBe "Request Entity Too Large" + response.header("X-Styx-Info").isPresent.shouldBeTrue() + } + } + } + + val client: StyxHttpClient = StyxHttpClient.Builder().build() + + val styxServer = StyxServerProvider(""" + proxy: + connectors: + http: + port: 0 + maxInitialLength: 100 + + admin: + connectors: + http: + port: 0 + + httpPipeline: + type: StaticResponseHandler + config: + status: 200 + content: "Test origin." + """.trimIndent()) + + + override fun afterSpec(spec: Spec) { + styxServer.stop() + } + +} diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt index 4f5b9282cf..b725103462 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt @@ -168,8 +168,11 @@ class OriginResourcesSpec : StringSpec() { - { id: "$prefix", host: "localhost:${mockServer.port()}" } """.trimIndent() - fun configurationApplied(prefix: String) = client.send(get(prefix).header(HOST, styxServer().proxyHttpHostHeader()).build()) - .wait(debug = false) + fun configurationApplied(prefix: String) = client.send( + get(prefix) + .header(HOST, styxServer().proxyHttpHostHeader()) + .build())!! + .wait(debug = false)!! .status() == OK }