Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ineffective HTTP initial line length attribute. #472

Merged
merged 1 commit into from
Oct 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -65,8 +65,8 @@ public boolean compress() {
*
* @return maximum length of initial line
*/
public int maxInitialLineLength() {
return maxInitialLineLength;
public int maxInitialLength() {
return maxInitialLength;
}

/**
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -189,7 +189,7 @@ public static class Builder<T extends Builder<T>> {
protected Integer bossThreadsCount;
protected Integer workerThreadsCount;
protected Integer nioAcceptorBacklog;
protected Integer maxInitialLineLength;
protected Integer maxInitialLength;
protected Integer maxHeaderSize;
protected Integer maxChunkSize;
protected Integer requestTimeoutMs;
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down