Skip to content

Commit

Permalink
sourcing maxInitialLineLength through the config and propagating it t…
Browse files Browse the repository at this point in the history
…o netty's configuration (#2561)

developing feature and testing

Co-authored-by: hdriviere <[email protected]>
  • Loading branch information
hdriviere and hdriviere authored Dec 20, 2023
1 parent f8f5b3c commit fa99f66
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 3 deletions.
1 change: 1 addition & 0 deletions zio-http/src/main/scala/zio/http/ConnectionPool.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ trait ConnectionPool[Connection] {
location: URL.Location.Absolute,
proxy: Option[Proxy],
sslOptions: ClientSSLConfig,
maxInitialLineLength: Int,
maxHeaderSize: Int,
decompression: Decompression,
idleTimeout: Option[Duration],
Expand Down
7 changes: 7 additions & 0 deletions zio-http/src/main/scala/zio/http/Server.scala
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ object Server {
requestDecompression: Decompression,
responseCompression: Option[ResponseCompressionConfig],
requestStreaming: RequestStreaming,
maxInitialLineLength: Int,
maxHeaderSize: Int,
logWarningOnFatalError: Boolean,
gracefulShutdownTimeout: Duration,
Expand Down Expand Up @@ -112,6 +113,8 @@ object Server {
*/
def logWarningOnFatalError(enable: Boolean): Config = self.copy(logWarningOnFatalError = enable)

def maxInitialLineLength(initialLineLength: Int): Config = self.copy(maxInitialLineLength = initialLineLength)

/**
* Configure the server to use `maxHeaderSize` value when encode/decode
* headers.
Expand Down Expand Up @@ -169,6 +172,7 @@ object Server {
Decompression.config.nested("request-decompression").withDefault(Config.default.requestDecompression) ++
ResponseCompressionConfig.config.nested("response-compression").optional ++
RequestStreaming.config.nested("request-streaming").withDefault(Config.default.requestStreaming) ++
zio.Config.int("max-initial-line-length").withDefault(Config.default.maxInitialLineLength) ++
zio.Config.int("max-header-size").withDefault(Config.default.maxHeaderSize) ++
zio.Config.boolean("log-warning-on-fatal-error").withDefault(Config.default.logWarningOnFatalError) ++
zio.Config.duration("graceful-shutdown-timeout").withDefault(Config.default.gracefulShutdownTimeout) ++
Expand All @@ -183,6 +187,7 @@ object Server {
requestDecompression,
responseCompression,
requestStreaming,
maxInitialLineLength,
maxHeaderSize,
logWarningOnFatalError,
gracefulShutdownTimeout,
Expand All @@ -196,6 +201,7 @@ object Server {
requestDecompression = requestDecompression,
responseCompression = responseCompression,
requestStreaming = requestStreaming,
maxInitialLineLength = maxInitialLineLength,
maxHeaderSize = maxHeaderSize,
logWarningOnFatalError = logWarningOnFatalError,
gracefulShutdownTimeout = gracefulShutdownTimeout,
Expand All @@ -211,6 +217,7 @@ object Server {
requestDecompression = Decompression.No,
responseCompression = None,
requestStreaming = RequestStreaming.Disabled(1024 * 100),
maxInitialLineLength = 4096,
maxHeaderSize = 8192,
logWarningOnFatalError = true,
gracefulShutdownTimeout = 10.seconds,
Expand Down
8 changes: 8 additions & 0 deletions zio-http/src/main/scala/zio/http/ZClient.scala
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,7 @@ object ZClient {
ssl: Option[ClientSSLConfig],
proxy: Option[zio.http.Proxy],
connectionPool: ConnectionPoolConfig,
maxInitialLineLength: Int,
maxHeaderSize: Int,
requestDecompression: Decompression,
localAddress: Option[InetSocketAddress],
Expand All @@ -557,6 +558,8 @@ object ZClient {
def disabledConnectionPool: Config =
self.copy(connectionPool = ConnectionPoolConfig.Disabled)

def maxInitialLineLength(initialLineLength: Int): Config = self.copy(maxInitialLineLength = initialLineLength)

/**
* Configure the client to use `maxHeaderSize` value when encode/decode
* headers.
Expand Down Expand Up @@ -590,6 +593,7 @@ object ZClient {
ClientSSLConfig.config.nested("ssl").optional.withDefault(Config.default.ssl) ++
zio.http.Proxy.config.nested("proxy").optional.withDefault(Config.default.proxy) ++
ConnectionPoolConfig.config.nested("connection-pool").withDefault(Config.default.connectionPool) ++
zio.Config.int("max-initial-line-length").withDefault(Config.default.maxInitialLineLength) ++
zio.Config.int("max-header-size").withDefault(Config.default.maxHeaderSize) ++
Decompression.config.nested("request-decompression").withDefault(Config.default.requestDecompression) ++
zio.Config.boolean("add-user-agent-header").withDefault(Config.default.addUserAgentHeader) ++
Expand All @@ -600,6 +604,7 @@ object ZClient {
ssl,
proxy,
connectionPool,
maxInitialLineLength,
maxHeaderSize,
requestDecompression,
addUserAgentHeader,
Expand All @@ -610,6 +615,7 @@ object ZClient {
ssl = ssl,
proxy = proxy,
connectionPool = connectionPool,
maxInitialLineLength = maxInitialLineLength,
maxHeaderSize = maxHeaderSize,
requestDecompression = requestDecompression,
addUserAgentHeader = addUserAgentHeader,
Expand All @@ -622,6 +628,7 @@ object ZClient {
ssl = None,
proxy = None,
connectionPool = ConnectionPoolConfig.Fixed(10),
maxInitialLineLength = 4096,
maxHeaderSize = 8192,
requestDecompression = Decompression.No,
localAddress = None,
Expand Down Expand Up @@ -708,6 +715,7 @@ object ZClient {
location,
clientConfig.proxy,
clientConfig.ssl.getOrElse(ClientSSLConfig.Default),
clientConfig.maxInitialLineLength,
clientConfig.maxHeaderSize,
clientConfig.requestDecompression,
clientConfig.idleTimeout,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ object NettyConnectionPool {
location: URL.Location.Absolute,
proxy: Option[Proxy],
sslOptions: ClientSSLConfig,
maxInitialLineLength: Int,
maxHeaderSize: Int,
decompression: Decompression,
idleTimeout: Option[Duration],
Expand Down Expand Up @@ -92,7 +93,7 @@ object NettyConnectionPool {
// This way, if the server closes the connection before the whole response has been sent,
// we get an error. (We can also handle the channelInactive callback, but since for now
// we always buffer the whole HTTP response we can letty Netty take care of this)
pipeline.addLast(Names.HttpClientCodec, new HttpClientCodec(4096, maxHeaderSize, 8192, true))
pipeline.addLast(Names.HttpClientCodec, new HttpClientCodec(maxInitialLineLength, maxHeaderSize, 8192, true))

// HttpContentDecompressor
if (decompression.enabled)
Expand Down Expand Up @@ -135,6 +136,7 @@ object NettyConnectionPool {
location: Location.Absolute,
proxy: Option[Proxy],
sslOptions: ClientSSLConfig,
maxInitialLineLength: Int,
maxHeaderSize: Int,
decompression: Decompression,
idleTimeout: Option[Duration],
Expand All @@ -147,6 +149,7 @@ object NettyConnectionPool {
location,
proxy,
sslOptions,
maxInitialLineLength,
maxHeaderSize,
decompression,
idleTimeout,
Expand All @@ -166,6 +169,7 @@ object NettyConnectionPool {
location: Location.Absolute,
proxy: Option[Proxy],
sslOptions: ClientSSLConfig,
maxInitialLineLength: Int,
maxHeaderSize: Int,
decompression: Decompression,
idleTimeout: Option[Duration],
Expand All @@ -179,14 +183,26 @@ object NettyConnectionPool {
location: Location.Absolute,
proxy: Option[Proxy],
sslOptions: ClientSSLConfig,
maxInitialLineLength: Int,
maxHeaderSize: Int,
decompression: Decompression,
idleTimeout: Option[Duration],
connectionTimeout: Option[Duration],
localAddress: Option[InetSocketAddress] = None,
)(implicit trace: Trace): ZIO[Scope, Throwable, JChannel] =
pool
.get(PoolKey(location, proxy, sslOptions, maxHeaderSize, decompression, idleTimeout, connectionTimeout))
.get(
PoolKey(
location,
proxy,
sslOptions,
maxInitialLineLength,
maxHeaderSize,
decompression,
idleTimeout,
connectionTimeout,
),
)

override def invalidate(channel: JChannel)(implicit trace: Trace): ZIO[Any, Nothing, Unit] =
pool.invalidate(channel)
Expand Down Expand Up @@ -243,6 +259,7 @@ object NettyConnectionPool {
key.location,
key.proxy,
key.sslOptions,
key.maxInitialLineLength,
key.maxHeaderSize,
key.decompression,
key.idleTimeout,
Expand Down Expand Up @@ -287,6 +304,7 @@ object NettyConnectionPool {
key.location,
key.proxy,
key.sslOptions,
key.maxInitialLineLength,
key.maxHeaderSize,
key.decompression,
key.idleTimeout,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private[zio] final case class ServerChannelInitializer(
// Instead of ServerCodec, we should use Decoder and Encoder separately to have more granular control over performance.
pipeline.addLast(
Names.HttpRequestDecoder,
new HttpRequestDecoder(DEFAULT_MAX_INITIAL_LINE_LENGTH, cfg.maxHeaderSize, DEFAULT_MAX_CHUNK_SIZE, false),
new HttpRequestDecoder(cfg.maxInitialLineLength, cfg.maxHeaderSize, DEFAULT_MAX_CHUNK_SIZE, false),
)
pipeline.addLast(Names.HttpResponseEncoder, new HttpResponseEncoder())

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright 2021 - 2023 Sporta Technologies PVT LTD & the ZIO HTTP contributors.
*
* 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 zio.http

import zio.test.TestAspect.withLiveClock
import zio.test._
import zio.{Scope, ZLayer}

object NettyMaxInitialLineLength extends ZIOHttpSpec {
val minimalInitialLineLength: Int = "GET / HTTP/1.1".getBytes.length

def extractStatus(response: Response): Status = response.status

private val serverConfig: Server.Config =
Server.Config.default.onAnyOpenPort.copy(maxInitialLineLength = minimalInitialLineLength)

override def spec: Spec[TestEnvironment with Scope, Any] =
test("should get a failure instead of an empty body") {
val app = Handler
.fromFunctionZIO[Request] { request =>
request.body.asString.map { body =>
val responseBody = if (body.isEmpty) "<empty>" else body
Response.text(responseBody)
} // this should not be run, as the request is invalid
}
.sandbox
.toHttpApp
for {
port <- Server.install(app)
url = URL
.decode(s"http://localhost:$port/a%20looooooooooooooooooooooooooooong%20query%20parameter")
.toOption
.get
headers = Headers.empty

res <- Client.request(Request(url = url, headers = headers, body = Body.fromString("some-body")))
data <- res.body.asString
} yield assertTrue(extractStatus(res) == Status.InternalServerError, data == "")
}.provide(
Client.default,
Server.live,
ZLayer.succeed(serverConfig),
Scope.default,
) @@ withLiveClock
}

0 comments on commit fa99f66

Please sign in to comment.