Skip to content

Commit

Permalink
Return "431 Request Header Fields Too Large" for long headers on HTTP…
Browse files Browse the repository at this point in the history
…/1 (#4655)

* Return "431 Request Header Fields Too Large" for long headers on HTTP/1

Motivation:

Netty HTTP/1 codec raises `TooLongHttpHeaderException` if headers exceed
the max length limit. However, `Http1RequestDecoder` does not take
account into `TooLongHttpLineException` and returns "400 Bad Request"
instead.

Modifications:

- Make `Http1RequestDecoder` return `431 Request Header Fields Too Large`
  if a Netty HttpRequest fails with `TooLongHttpHeaderException`

Result:

- "431 Request Header Fields Too Large" is now returned instead of
  `400 Bad Request` if the size of header exceeds
  `ServerBuilder.http1MaxHeaderSize()`.
- Fixes #4609

* Fix the error message
  • Loading branch information
ikhoon committed Feb 9, 2023
1 parent e971022 commit a43cfa2
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import io.netty.handler.codec.http.HttpUtil;
import io.netty.handler.codec.http.HttpVersion;
import io.netty.handler.codec.http.LastHttpContent;
import io.netty.handler.codec.http.TooLongHttpHeaderException;
import io.netty.handler.codec.http.TooLongHttpLineException;
import io.netty.handler.codec.http2.Http2CodecUtil;
import io.netty.handler.codec.http2.Http2Error;
Expand Down Expand Up @@ -151,6 +152,9 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
final Throwable cause = nettyReq.decoderResult().cause();
if (cause instanceof TooLongHttpLineException) {
fail(id, null, HttpStatus.REQUEST_URI_TOO_LONG, "Too Long URI", cause);
} else if (cause instanceof TooLongHttpHeaderException) {
fail(id, null, HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE,
"Request Header Fields Too Large", cause);
} else {
fail(id, null, HttpStatus.BAD_REQUEST, "Decoder failure", cause);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright 2023 LINE Corporation
*
* LINE Corporation 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:
*
* https://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.linecorp.armeria.server;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import com.google.common.base.Strings;

import com.linecorp.armeria.client.BlockingWebClient;
import com.linecorp.armeria.common.AggregatedHttpResponse;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.HttpStatus;
import com.linecorp.armeria.common.SessionProtocol;
import com.linecorp.armeria.testing.junit5.server.ServerExtension;

class LongHttp1HeadersTest {

@RegisterExtension
static ServerExtension server = new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) {
sb.http1MaxHeaderSize(100);

sb.service("/", (ctx, req) -> HttpResponse.of("OK"));
}
};

@Test
void shouldReturn431WithLongHeadersForHttp1() {
final BlockingWebClient client = BlockingWebClient.of(server.uri(SessionProtocol.H1C));
final AggregatedHttpResponse response = client.prepare()
.get("/")
.header("x-long", Strings.repeat("1", 100))
.execute();
assertThat(response.status()).isEqualTo(HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE);
}
}

0 comments on commit a43cfa2

Please sign in to comment.