From 6235734dabb97ce61e39fe1b1e04786838e8541c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Thu, 18 Jan 2018 10:43:39 +0100 Subject: [PATCH] improve API, separate individual parameter setters + add 2 last --- .../ipc/netty/http/server/HttpServer.java | 4 +- .../netty/http/server/HttpServerOptions.java | 121 ++++++++++-- .../http/server/HttpServerOptionsTest.java | 182 +++++++++--------- 3 files changed, 192 insertions(+), 115 deletions(-) diff --git a/src/main/java/reactor/ipc/netty/http/server/HttpServer.java b/src/main/java/reactor/ipc/netty/http/server/HttpServer.java index 0e57c26760..49b74da996 100644 --- a/src/main/java/reactor/ipc/netty/http/server/HttpServer.java +++ b/src/main/java/reactor/ipc/netty/http/server/HttpServer.java @@ -257,7 +257,9 @@ public void accept(ChannelPipeline p, ContextHandler c) { p.addLast(NettyPipeline.HttpCodec, new HttpServerCodec( options.httpCodecMaxInitialLineLength(), options.httpCodecMaxHeaderSize(), - options.httpCodecMaxChunkSize())); + options.httpCodecMaxChunkSize(), + options.httpCodecValidateHeaders(), + options.httpCodecInitialBufferSize())); if(options.minCompressionResponseSize() >= 0) { p.addLast(NettyPipeline.CompressionHandler, new CompressionHandler(options.minCompressionResponseSize())); diff --git a/src/main/java/reactor/ipc/netty/http/server/HttpServerOptions.java b/src/main/java/reactor/ipc/netty/http/server/HttpServerOptions.java index 223b38fee7..6414638ac2 100644 --- a/src/main/java/reactor/ipc/netty/http/server/HttpServerOptions.java +++ b/src/main/java/reactor/ipc/netty/http/server/HttpServerOptions.java @@ -41,6 +41,8 @@ public static HttpServerOptions.Builder builder() { private final int maxInitialLineLength; private final int maxHeaderSize; private final int maxChunkSize; + private final int initialBufferSize; + private final boolean validateHeaders; private HttpServerOptions(HttpServerOptions.Builder builder) { super(builder); @@ -48,6 +50,8 @@ private HttpServerOptions(HttpServerOptions.Builder builder) { this.maxInitialLineLength = builder.maxInitialLineLength; this.maxHeaderSize = builder.maxHeaderSize; this.maxChunkSize = builder.maxChunkSize; + this.validateHeaders = builder.validateHeaders; + this.initialBufferSize = builder.initialBufferSize; } /** @@ -90,6 +94,25 @@ public int httpCodecMaxChunkSize() { return maxChunkSize; } + /** + * Returns the HTTP validate headers flag. + * + * @return true if the HTTP codec validates headers, false otherwise + * @see io.netty.handler.codec.http.HttpServerCodec + */ + public boolean httpCodecValidateHeaders() { + return validateHeaders; + } + /** + * Returns the configured HTTP codec initial buffer size. + * + * @return the configured HTTP codec initial buffer size + * @see io.netty.handler.codec.http.HttpServerCodec + */ + public int httpCodecInitialBufferSize() { + return initialBufferSize; + } + @Override public HttpServerOptions duplicate() { return builder().from(this).build(); @@ -123,10 +146,19 @@ public String toString() { } public static final class Builder extends ServerOptions.Builder { + + public static final int DEFAULT_MAX_INITIAL_LINE_LENGTH = 4096; + public static final int DEFAULT_MAX_HEADER_SIZE = 8192; + public static final int DEFAULT_MAX_CHUNK_SIZE = 8192; + public static final boolean DEFAULT_VALIDATE_HEADERS = true; + public static final int DEFAULT_INITIAL_BUFFER_SIZE = 128; + private int minCompressionResponseSize = -1; - private int maxInitialLineLength = 4096; - private int maxHeaderSize = 8192; - private int maxChunkSize = 8192; + private int maxInitialLineLength = DEFAULT_MAX_INITIAL_LINE_LENGTH; + private int maxHeaderSize = DEFAULT_MAX_HEADER_SIZE; + private int maxChunkSize = DEFAULT_MAX_CHUNK_SIZE; + private boolean validateHeaders = DEFAULT_VALIDATE_HEADERS; + private int initialBufferSize = DEFAULT_INITIAL_BUFFER_SIZE; private Builder(){ super(new ServerBootstrap()); @@ -162,28 +194,75 @@ public final Builder compression(int minResponseSize) { } /** - * Configure the {@link io.netty.handler.codec.http.HttpServerCodec HTTP codec} - * maximum initial HTTP line length, header size and chunk size. - *

- * Negative or zero values are not valid, but will be interpreted as "don't change - * the current configuration for that field": on first call the Netty defaults of - * {@code (4096,8192,8192)} will be used. + * Configure the maximum length that can be decoded for the HTTP request's initial + * line. Defaults to {@code #DEFAULT_MAX_INITIAL_LINE_LENGTH}. * - * @param maxInitialLineLength the HTTP initial line maximum length. Use 0 to ignore/keep default. - * @param maxHeaderSize the HTTP header maximum size. Use 0 to ignore/keep default. - * @param maxChunkSize the HTTP chunk maximum size. Use 0 to ignore/keep default. - * @return {@code this} + * @param value the value for the maximum initial line length (strictly positive) + * @return this option builder for further configuration */ - public final Builder httpCodecOptions(int maxInitialLineLength, int maxHeaderSize, int maxChunkSize) { - if (maxInitialLineLength > 0) { - this.maxInitialLineLength = maxInitialLineLength; + public final Builder maxInitialLineLength(int value) { + if (value <= 0) { + throw new IllegalArgumentException( + "maxInitialLineLength must be strictly positive"); } - if (maxHeaderSize > 0) { - this.maxHeaderSize = maxHeaderSize; + this.maxInitialLineLength = value; + return get(); + } + + /** + * Configure the maximum header size that can be decoded for the HTTP request. + * Defaults to {@link #DEFAULT_MAX_HEADER_SIZE}. + * + * @param value the value for the maximum header size (strictly positive) + * @return this option builder for further configuration + */ + public final Builder maxHeaderSize(int value) { + if (value <= 0) { + throw new IllegalArgumentException("maxHeaderSize must be strictly positive"); } - if (maxChunkSize > 0) { - this.maxChunkSize = maxChunkSize; + this.maxHeaderSize = value; + return get(); + } + + /** + * Configure the maximum chunk size that can be decoded for the HTTP request. + * Defaults to {@link #DEFAULT_MAX_CHUNK_SIZE}. + * + * @param value the value for the maximum chunk size (strictly positive) + * @return this option builder for further configuration + */ + public final Builder maxChunkSize(int value) { + if (value <= 0) { + throw new IllegalArgumentException("maxChunkSize must be strictly positive"); + } + this.maxChunkSize = value; + return get(); + } + + /** + * Configure whether or not to validate headers when decoding requests. Defaults to + * #DEFAULT_VALIDATE_HEADERS. + * + * @param validate true to validate headers, false otherwise + * @return this option builder for further configuration + */ + public final Builder validateHeaders(boolean validate) { + this.validateHeaders = validate; + return get(); + } + + /** + * Configure the initial buffer size for HTTP request decoding. Defaults to + * {@link #DEFAULT_INITIAL_BUFFER_SIZE}. + * + * @param value the initial buffer size to use (strictly positive) + * @return + */ + public final Builder initialBufferSize(int value) { + if (value <= 0) { + throw new IllegalArgumentException("initialBufferSize must be strictly positive"); } + this.initialBufferSize = value; return get(); } @@ -199,6 +278,8 @@ public final Builder from(HttpServerOptions options) { this.maxInitialLineLength = options.maxInitialLineLength; this.maxHeaderSize = options.maxHeaderSize; this.maxChunkSize = options.maxChunkSize; + this.validateHeaders = options.validateHeaders; + this.initialBufferSize = options.initialBufferSize; return get(); } diff --git a/src/test/java/reactor/ipc/netty/http/server/HttpServerOptionsTest.java b/src/test/java/reactor/ipc/netty/http/server/HttpServerOptionsTest.java index ce65a43f5a..fe69be5de9 100644 --- a/src/test/java/reactor/ipc/netty/http/server/HttpServerOptionsTest.java +++ b/src/test/java/reactor/ipc/netty/http/server/HttpServerOptionsTest.java @@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static reactor.ipc.netty.http.server.HttpServerOptions.Builder.*; public class HttpServerOptionsTest { @@ -49,148 +50,139 @@ public void minResponseForCompressionPositive() { } @Test - public void httpCodecSizesModified() { + public void maxInitialLineLength() { HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, 456, 789); + builder.maxInitialLineLength(123); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(123); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(456); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(789); - } + HttpServerOptions conf = builder.build(); - @Test - public void httpCodecSizesDefaults() { - HttpServerOptions.Builder builder = HttpServerOptions.builder(); + assertThat(conf.httpCodecMaxInitialLineLength()).as("initial line length").isEqualTo(123); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(4096); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(8192); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(8192); + assertThat(conf.httpCodecMaxHeaderSize()).as("default header size").isEqualTo(DEFAULT_MAX_HEADER_SIZE); + assertThat(conf.httpCodecMaxChunkSize()).as("default chunk size").isEqualTo(DEFAULT_MAX_CHUNK_SIZE); + assertThat(conf.httpCodecValidateHeaders()).as("default validate headers").isEqualTo(DEFAULT_VALIDATE_HEADERS); + assertThat(conf.httpCodecInitialBufferSize()).as("default initial buffer sizez").isEqualTo(DEFAULT_INITIAL_BUFFER_SIZE); } @Test - public void httpCodecSizesLineNegativeDefaults() { + public void maxInitialLineLengthBadValues() { HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(-1, 456, 789); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(4096); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(456); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(789); - } - @Test - public void httpCodecSizesLineZeroDefaults() { - HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(0, 456, 789); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.maxInitialLineLength(0)) + .as("rejects 0") + .withMessage("maxInitialLineLength must be strictly positive"); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(4096); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(456); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(789); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.maxInitialLineLength(-1)) + .as("rejects negative") + .withMessage("maxInitialLineLength must be strictly positive"); } @Test - public void httpCodecSizesLineNegativeIgnored() { + public void maxHeaderSize() { HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, 456, 789) - .httpCodecOptions(-1, 1, 2); + builder.maxHeaderSize(123); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(123); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(1); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(2); - } + HttpServerOptions conf = builder.build(); - @Test - public void httpCodecSizesLineZeroIgnored() { - HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, 456, 789) - .httpCodecOptions(0, 1, 2); + assertThat(conf.httpCodecMaxHeaderSize()).as("header size").isEqualTo(123); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(123); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(1); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(2); + assertThat(conf.httpCodecMaxInitialLineLength()).as("default initial line length").isEqualTo(DEFAULT_MAX_INITIAL_LINE_LENGTH); + assertThat(conf.httpCodecMaxChunkSize()).as("default chunk size").isEqualTo(DEFAULT_MAX_CHUNK_SIZE); + assertThat(conf.httpCodecValidateHeaders()).as("default validate headers").isEqualTo(DEFAULT_VALIDATE_HEADERS); + assertThat(conf.httpCodecInitialBufferSize()).as("default initial buffer sizez").isEqualTo(DEFAULT_INITIAL_BUFFER_SIZE); } @Test - public void httpCodecSizesHeaderNegativeDefaults() { + public void maxHeaderSizeBadValues() { HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, -1, 789); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(123); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(8192); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(789); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.maxHeaderSize(0)) + .as("rejects 0") + .withMessage("maxHeaderSize must be strictly positive"); + + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.maxHeaderSize(-1)) + .as("rejects negative") + .withMessage("maxHeaderSize must be strictly positive"); } @Test - public void httpCodecSizesHeaderZeroDefaults() { + public void maxChunkSize() { HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, 0, 789); + builder.maxChunkSize(123); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(123); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(8192); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(789); - } + HttpServerOptions conf = builder.build(); - @Test - public void httpCodecSizesHeaderNegativeIgnored() { - HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, 456, 789) - .httpCodecOptions(1, -1, 2); + assertThat(conf.httpCodecMaxChunkSize()).as("chunk size").isEqualTo(123); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(1); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(456); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(2); + assertThat(conf.httpCodecMaxInitialLineLength()).as("default initial line length").isEqualTo(DEFAULT_MAX_INITIAL_LINE_LENGTH); + assertThat(conf.httpCodecMaxHeaderSize()).as("default header size").isEqualTo(DEFAULT_MAX_HEADER_SIZE); + assertThat(conf.httpCodecValidateHeaders()).as("default validate headers").isEqualTo(DEFAULT_VALIDATE_HEADERS); + assertThat(conf.httpCodecInitialBufferSize()).as("default initial buffer sizez").isEqualTo(DEFAULT_INITIAL_BUFFER_SIZE); } @Test - public void httpCodecSizesHeaderZeroIgnored() { + public void maxChunkSizeBadValues() { HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, 456, 789) - .httpCodecOptions(1, 0, 2); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(1); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(456); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(2); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.maxChunkSize(0)) + .as("rejects 0") + .withMessage("maxChunkSize must be strictly positive"); + + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.maxChunkSize(-1)) + .as("rejects negative") + .withMessage("maxChunkSize must be strictly positive"); } @Test - public void httpCodecSizesChunkNegativeDefaults() { + public void validateHeaders() { HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, 456, -1); + builder.validateHeaders(false); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(123); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(456); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(8192); - } + HttpServerOptions conf = builder.build(); - @Test - public void httpCodecSizesChunkZeroDefaults() { - HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, 456, 0); + assertThat(conf.httpCodecValidateHeaders()).as("validate headers").isFalse(); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(123); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(456); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(8192); + assertThat(conf.httpCodecMaxInitialLineLength()).as("default initial line length").isEqualTo(DEFAULT_MAX_INITIAL_LINE_LENGTH); + assertThat(conf.httpCodecMaxHeaderSize()).as("default header size").isEqualTo(DEFAULT_MAX_HEADER_SIZE); + assertThat(conf.httpCodecMaxChunkSize()).as("default chunk size").isEqualTo(DEFAULT_MAX_CHUNK_SIZE); + assertThat(conf.httpCodecInitialBufferSize()).as("default initial buffer sizez").isEqualTo(DEFAULT_INITIAL_BUFFER_SIZE); } @Test - public void httpCodecSizesChunkNegativeIgnored() { + public void initialBufferSize() { HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, 456, 789) - .httpCodecOptions(1, 2, -1); + builder.initialBufferSize(123); + + HttpServerOptions conf = builder.build(); + + assertThat(conf.httpCodecInitialBufferSize()).as("initial buffer size").isEqualTo(123); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(1); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(2); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(789); + assertThat(conf.httpCodecMaxInitialLineLength()).as("default initial line length").isEqualTo(DEFAULT_MAX_INITIAL_LINE_LENGTH); + assertThat(conf.httpCodecMaxHeaderSize()).as("default header size").isEqualTo(DEFAULT_MAX_HEADER_SIZE); + assertThat(conf.httpCodecMaxChunkSize()).as("default chunk size").isEqualTo(DEFAULT_MAX_CHUNK_SIZE); + assertThat(conf.httpCodecValidateHeaders()).as("default validate headers").isEqualTo(DEFAULT_VALIDATE_HEADERS); } @Test - public void httpCodecSizesChunkZeroIgnored() { + public void initialBufferSizeBadValues() { HttpServerOptions.Builder builder = HttpServerOptions.builder(); - builder.httpCodecOptions(123, 456, 789) - .httpCodecOptions(1, 2, 0); - assertThat(builder.build().httpCodecMaxInitialLineLength()).isEqualTo(1); - assertThat(builder.build().httpCodecMaxHeaderSize()).isEqualTo(2); - assertThat(builder.build().httpCodecMaxChunkSize()).isEqualTo(789); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.initialBufferSize(0)) + .as("rejects 0") + .withMessage("initialBufferSize must be strictly positive"); + + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.initialBufferSize(-1)) + .as("rejects negative") + .withMessage("initialBufferSize must be strictly positive"); } @Test @@ -249,25 +241,27 @@ public void asDetailedStringHttpCodecSizes() { //changed line length assertThat(HttpServerOptions.builder() - .httpCodecOptions(123, 0, -1) + .maxInitialLineLength(123) .build().asDetailedString()) .endsWith(", httpCodecSizes={initialLine=123,header=8192,chunk=8192}"); //changed header size assertThat(HttpServerOptions.builder() - .httpCodecOptions(0, 123, -1) + .maxHeaderSize(123) .build().asDetailedString()) .endsWith(", httpCodecSizes={initialLine=4096,header=123,chunk=8192}"); //changed chunk size assertThat(HttpServerOptions.builder() - .httpCodecOptions(0, -1, 123) + .maxChunkSize(123) .build().asDetailedString()) .endsWith(", httpCodecSizes={initialLine=4096,header=8192,chunk=123}"); //changed all sizes assertThat(HttpServerOptions.builder() - .httpCodecOptions(123, 456, 789) + .maxInitialLineLength(123) + .maxHeaderSize(456) + .maxChunkSize(789) .build().asDetailedString()) .endsWith(", httpCodecSizes={initialLine=123,header=456,chunk=789}"); }