Skip to content

Fix HttpObjectAggregator leaving connection stuck after 413 with AUTO…#16280

Merged
chrisvest merged 1 commit into
netty:4.1from
samlandfried:fix-413-stuck-connection
Mar 3, 2026
Merged

Fix HttpObjectAggregator leaving connection stuck after 413 with AUTO…#16280
chrisvest merged 1 commit into
netty:4.1from
samlandfried:fix-413-stuck-connection

Conversation

@samlandfried

@samlandfried samlandfried commented Feb 16, 2026

Copy link
Copy Markdown
Contributor

Motivation:

When HttpObjectAggregator.handleOversizedMessage() sends a 413 response with HTTP/1.1 keep-alive
enabled, it only closes the channel on write failure. On success, the channel is left open but no
channel.read() is called.

With AUTO_READ=false, this leaves the connection stuck -
subsequent requests sit unread in the kernel buffer until timeout.

Modification:

Always close the channel after sending 413, not just on write failure. This matches:

  1. The existing comment: "send back a 413 and close the connection"
  2. The behavior when keep-alive is disabled
  3. The behavior when the message is a FullHttpMessage

Closing is the correct behavior because after an oversized chunked message, there may be leftover data
in the TCP stream that cannot be reliably skipped in HTTP/1.1.

Result:

Connections are properly closed after 413, preventing stuck connections with AUTO_READ=false.

Repro:

The bug can only be reproed with a "real" channel. This test repros but isn't committed since the fix to simply close the channel is testable with a simpler EmbeddedChannel test

@Test
public void testOversizedRequestWithKeepAliveAndAutoReadFalse() throws InterruptedException {
    final CountDownLatch responseLatch = new CountDownLatch(1);
    final CountDownLatch secondRequestLatch = new CountDownLatch(1);
    final AtomicReference<HttpResponseStatus> statusRef = new AtomicReference<HttpResponseStatus>();

    NioEventLoopGroup group = new NioEventLoopGroup(2);
    try {
        ServerBootstrap sb = new ServerBootstrap();
        sb.group(group)
                .channel(NioServerSocketChannel.class)
                .childOption(ChannelOption.AUTO_READ, false)
                .childHandler(new ChannelInitializer<Channel>() {
                    @Override
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpServerCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(4));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpRequest>() {
                            @Override
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest msg) {
                                secondRequestLatch.countDown();
                            }
                        });
                        // Trigger the first read manually (AUTO_READ=false requires this)
                        ch.read();
                    }
                });

        Bootstrap cb = new Bootstrap();
        cb.group(group)
                .channel(NioSocketChannel.class)
                .handler(new ChannelInitializer<Channel>() {
                    @Override
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpClientCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(1024));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpResponse>() {
                            @Override
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpResponse msg) {
                                statusRef.set(msg.status());
                                responseLatch.countDown();
                            }
                        });
                    }
                });

        Channel serverChannel = sb.bind(new InetSocketAddress(0)).sync().channel();
        int port = ((InetSocketAddress) serverChannel.localAddress()).getPort();
        Channel clientChannel = cb.connect(new InetSocketAddress(NetUtil.LOCALHOST, port)).sync().channel();

        HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "/upload");
        HttpUtil.setContentLength(request, 5);
        clientChannel.writeAndFlush(request);
        clientChannel.writeAndFlush(new DefaultLastHttpContent(
                Unpooled.wrappedBuffer(new byte[]{1, 2, 3, 4, 5})));

        // Server should respond with 413
        assertTrue(responseLatch.await(5, SECONDS));
        assertEquals(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, statusRef.get());

        // Send a second request on the same connection. With the bug, the server never
        // calls ctx.read() after the 413, so this request hangs indefinitely.
        clientChannel.writeAndFlush(
                new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/next",
                        Unpooled.EMPTY_BUFFER));

        assertTrue(secondRequestLatch.await(5, SECONDS),
                "Second request was never read - channel is stuck after 413 with AUTO_READ=false");

        clientChannel.close().sync();
        serverChannel.close().sync();
    } finally {
        group.shutdownGracefully();
    }
}

@samlandfried samlandfried force-pushed the fix-413-stuck-connection branch from ca897c8 to 9cdc24b Compare February 16, 2026 21:47
@samlandfried samlandfried marked this pull request as draft February 17, 2026 02:10
@samlandfried samlandfried marked this pull request as ready for review February 17, 2026 02:10
@normanmaurer normanmaurer added needs-cherry-pick-4.2 This PR should be cherry-picked to 4.2 once merged. needs-cherry-pick-5.0 This PR should be cherry-picked to 5.0 once merged. labels Feb 18, 2026
if (!future.isSuccess()) {
logger.debug("Failed to send a 413 Request Entity Too Large.", future.cause());
ctx.close();
} else if (!HttpUtil.is100ContinueExpected(oversized)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samlandfried I think it would be better to call the HttpUtil.is100ContinueExpected(oversized) outside of the listener and store it in a boolean. Then use the boolean in the listener. The reason for this is that the listener might run when the oversized was already released.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this a little more, I don't think this solves the broader problem. If the Expect: 100-continue header is present, the channel will still be in the same stuck state. We could conditionally call ctx.read() if AUTO_READ is false, but I think that could read a partial message remaining in the buffer from this oversized one. I think the most reliable behavior would be to always close the channel, but there are some unit tests expecting the channel to remain open in this scenario. We could always close when auto read is disabled.

What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@normanmaurer forgot to tag you ☝️

@samlandfried samlandfried Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@normanmaurer You can ignore my response. I changed this to always close the channel for AUTO_READ false since calling ctx.read in this case would defeat the point of disabling auto read, and I confirmed this bug doesn't apply to AUTO_READ true. This addresses the oversized usage as well

Comment thread codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java Outdated
@samlandfried samlandfried marked this pull request as draft February 23, 2026 17:26
…_READ=false

When `HttpObjectAggregator` sends a 413 (Request Entity Too Large) response on a
keep-alive HTTP/1.1 connection, it neither closes the channel nor triggers the next
read. With `AUTO_READ=false`, this leaves the channel permanently stuck: subsequent
requests from the client sit unread in the kernel buffer indefinitely.

This change closes the channel when AUTO_READ=false so the next incoming
message is read appropriately. An alternate fix could be to call
`ctx.read()` instead, but that would be contradictory to the intention
of AUTO_READ=false.

The following test reproduces the bug but isn't committed since
the fix simply closes the channel which can be tested with a simpler
EmbeddedChannel test.

@test
public void testOversizedRequestWithKeepAliveAndAutoReadFalse() throws InterruptedException {
    final CountDownLatch responseLatch = new CountDownLatch(1);
    final CountDownLatch secondRequestLatch = new CountDownLatch(1);
    final AtomicReference<HttpResponseStatus> statusRef = new AtomicReference<HttpResponseStatus>();

    NioEventLoopGroup group = new NioEventLoopGroup(2);
    try {
        ServerBootstrap sb = new ServerBootstrap();
        sb.group(group)
                .channel(NioServerSocketChannel.class)
                .childOption(ChannelOption.AUTO_READ, false)
                .childHandler(new ChannelInitializer<Channel>() {
                    @OverRide
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpServerCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(4));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpRequest>() {
                            @OverRide
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest msg) {
                                secondRequestLatch.countDown();
                            }
                        });
                        // Trigger the first read manually (AUTO_READ=false requires this)
                        ch.read();
                    }
                });

        Bootstrap cb = new Bootstrap();
        cb.group(group)
                .channel(NioSocketChannel.class)
                .handler(new ChannelInitializer<Channel>() {
                    @OverRide
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpClientCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(1024));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpResponse>() {
                            @OverRide
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpResponse msg) {
                                statusRef.set(msg.status());
                                responseLatch.countDown();
                            }
                        });
                    }
                });

        Channel serverChannel = sb.bind(new InetSocketAddress(0)).sync().channel();
        int port = ((InetSocketAddress) serverChannel.localAddress()).getPort();
        Channel clientChannel = cb.connect(new InetSocketAddress(NetUtil.LOCALHOST, port)).sync().channel();

        // Send an oversized request (Content-Length: 5 > limit of 4), keep-alive on by default.
        // Don't sync() - the HttpClientCodec encoder uses pooled buffers which have a JDK
        // version incompatibility in this environment. We rely on the response latch instead.
        HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "/upload");
        HttpUtil.setContentLength(request, 5);
        clientChannel.writeAndFlush(request);
        clientChannel.writeAndFlush(new DefaultLastHttpContent(
                Unpooled.wrappedBuffer(new byte[]{1, 2, 3, 4, 5})));

        // Server should respond with 413
        assertTrue(responseLatch.await(5, SECONDS));
        assertEquals(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, statusRef.get());

        // Send a second request on the same connection. With the bug, the server never
        // calls ctx.read() after the 413, so this request hangs indefinitely.
        clientChannel.writeAndFlush(
                new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/next",
                        Unpooled.EMPTY_BUFFER));

        assertTrue(secondRequestLatch.await(5, SECONDS),
                "Second request was never read - channel is stuck after 413 with AUTO_READ=false");

        clientChannel.close().sync();
        serverChannel.close().sync();
    } finally {
        group.shutdownGracefully();
    }
}
@samlandfried samlandfried force-pushed the fix-413-stuck-connection branch from 293337d to c383083 Compare February 24, 2026 17:13
@samlandfried samlandfried marked this pull request as ready for review February 24, 2026 17:19
@chrisvest chrisvest added this to the 4.1.132.Final milestone Feb 25, 2026
@chrisvest chrisvest merged commit 442a8cf into netty:4.1 Mar 3, 2026
18 checks passed
@chrisvest

Copy link
Copy Markdown
Member

Thanks!

@netty-project-bot

Copy link
Copy Markdown
Contributor

Could not create auto-port PR.
Got conflicts when cherry-picking onto 4.2.

chrisvest pushed a commit to chrisvest/netty that referenced this pull request Mar 3, 2026
netty#16280)

### Motivation:

When HttpObjectAggregator.handleOversizedMessage() sends a 413 response
with HTTP/1.1 keep-alive
enabled, it only closes the channel on write failure. On success, the
channel is left open but no
channel.read() is called.

With AUTO_READ=false, this leaves the connection stuck -
subsequent requests sit unread in the kernel buffer until timeout.

### Modification:

Always close the channel after sending 413, not just on write failure.
This matches:
1. The existing comment: "send back a 413 and close the connection"
2. The behavior when keep-alive is disabled
3. The behavior when the message is a FullHttpMessage

Closing is the correct behavior because after an oversized chunked
message, there may be leftover data
in the TCP stream that cannot be reliably skipped in HTTP/1.1.

### Result:

Connections are properly closed after 413, preventing stuck connections
with AUTO_READ=false.

### Repro:

The bug can only be reproed with a "real" channel. This test repros but
isn't committed since the fix to simply close the channel is testable
with a simpler EmbeddedChannel test

```
@test
public void testOversizedRequestWithKeepAliveAndAutoReadFalse() throws InterruptedException {
    final CountDownLatch responseLatch = new CountDownLatch(1);
    final CountDownLatch secondRequestLatch = new CountDownLatch(1);
    final AtomicReference<HttpResponseStatus> statusRef = new AtomicReference<HttpResponseStatus>();

    NioEventLoopGroup group = new NioEventLoopGroup(2);
    try {
        ServerBootstrap sb = new ServerBootstrap();
        sb.group(group)
                .channel(NioServerSocketChannel.class)
                .childOption(ChannelOption.AUTO_READ, false)
                .childHandler(new ChannelInitializer<Channel>() {
                    @OverRide
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpServerCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(4));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpRequest>() {
                            @OverRide
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest msg) {
                                secondRequestLatch.countDown();
                            }
                        });
                        // Trigger the first read manually (AUTO_READ=false requires this)
                        ch.read();
                    }
                });

        Bootstrap cb = new Bootstrap();
        cb.group(group)
                .channel(NioSocketChannel.class)
                .handler(new ChannelInitializer<Channel>() {
                    @OverRide
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpClientCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(1024));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpResponse>() {
                            @OverRide
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpResponse msg) {
                                statusRef.set(msg.status());
                                responseLatch.countDown();
                            }
                        });
                    }
                });

        Channel serverChannel = sb.bind(new InetSocketAddress(0)).sync().channel();
        int port = ((InetSocketAddress) serverChannel.localAddress()).getPort();
        Channel clientChannel = cb.connect(new InetSocketAddress(NetUtil.LOCALHOST, port)).sync().channel();

        HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "/upload");
        HttpUtil.setContentLength(request, 5);
        clientChannel.writeAndFlush(request);
        clientChannel.writeAndFlush(new DefaultLastHttpContent(
                Unpooled.wrappedBuffer(new byte[]{1, 2, 3, 4, 5})));

        // Server should respond with 413
        assertTrue(responseLatch.await(5, SECONDS));
        assertEquals(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, statusRef.get());

        // Send a second request on the same connection. With the bug, the server never
        // calls ctx.read() after the 413, so this request hangs indefinitely.
        clientChannel.writeAndFlush(
                new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/next",
                        Unpooled.EMPTY_BUFFER));

        assertTrue(secondRequestLatch.await(5, SECONDS),
                "Second request was never read - channel is stuck after 413 with AUTO_READ=false");

        clientChannel.close().sync();
        serverChannel.close().sync();
    } finally {
        group.shutdownGracefully();
    }
}
```

Co-authored-by: Sam Landfried <samlland@amazon.com>
(cherry picked from commit 442a8cf)
@chrisvest

Copy link
Copy Markdown
Member

4.2 port: #16401

netty-project-bot pushed a commit that referenced this pull request Mar 3, 2026
#16280)

### Motivation:

When HttpObjectAggregator.handleOversizedMessage() sends a 413 response
with HTTP/1.1 keep-alive
enabled, it only closes the channel on write failure. On success, the
channel is left open but no
channel.read() is called.

With AUTO_READ=false, this leaves the connection stuck -
subsequent requests sit unread in the kernel buffer until timeout.

### Modification:

Always close the channel after sending 413, not just on write failure.
This matches:
1. The existing comment: "send back a 413 and close the connection"
2. The behavior when keep-alive is disabled
3. The behavior when the message is a FullHttpMessage

Closing is the correct behavior because after an oversized chunked
message, there may be leftover data
in the TCP stream that cannot be reliably skipped in HTTP/1.1.

### Result:

Connections are properly closed after 413, preventing stuck connections
with AUTO_READ=false.

### Repro:

The bug can only be reproed with a "real" channel. This test repros but
isn't committed since the fix to simply close the channel is testable
with a simpler EmbeddedChannel test

```
@test
public void testOversizedRequestWithKeepAliveAndAutoReadFalse() throws InterruptedException {
    final CountDownLatch responseLatch = new CountDownLatch(1);
    final CountDownLatch secondRequestLatch = new CountDownLatch(1);
    final AtomicReference<HttpResponseStatus> statusRef = new AtomicReference<HttpResponseStatus>();

    NioEventLoopGroup group = new NioEventLoopGroup(2);
    try {
        ServerBootstrap sb = new ServerBootstrap();
        sb.group(group)
                .channel(NioServerSocketChannel.class)
                .childOption(ChannelOption.AUTO_READ, false)
                .childHandler(new ChannelInitializer<Channel>() {
                    @OverRide
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpServerCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(4));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpRequest>() {
                            @OverRide
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest msg) {
                                secondRequestLatch.countDown();
                            }
                        });
                        // Trigger the first read manually (AUTO_READ=false requires this)
                        ch.read();
                    }
                });

        Bootstrap cb = new Bootstrap();
        cb.group(group)
                .channel(NioSocketChannel.class)
                .handler(new ChannelInitializer<Channel>() {
                    @OverRide
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpClientCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(1024));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpResponse>() {
                            @OverRide
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpResponse msg) {
                                statusRef.set(msg.status());
                                responseLatch.countDown();
                            }
                        });
                    }
                });

        Channel serverChannel = sb.bind(new InetSocketAddress(0)).sync().channel();
        int port = ((InetSocketAddress) serverChannel.localAddress()).getPort();
        Channel clientChannel = cb.connect(new InetSocketAddress(NetUtil.LOCALHOST, port)).sync().channel();

        HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "/upload");
        HttpUtil.setContentLength(request, 5);
        clientChannel.writeAndFlush(request);
        clientChannel.writeAndFlush(new DefaultLastHttpContent(
                Unpooled.wrappedBuffer(new byte[]{1, 2, 3, 4, 5})));

        // Server should respond with 413
        assertTrue(responseLatch.await(5, SECONDS));
        assertEquals(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, statusRef.get());

        // Send a second request on the same connection. With the bug, the server never
        // calls ctx.read() after the 413, so this request hangs indefinitely.
        clientChannel.writeAndFlush(
                new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/next",
                        Unpooled.EMPTY_BUFFER));

        assertTrue(secondRequestLatch.await(5, SECONDS),
                "Second request was never read - channel is stuck after 413 with AUTO_READ=false");

        clientChannel.close().sync();
        serverChannel.close().sync();
    } finally {
        group.shutdownGracefully();
    }
}
```

Co-authored-by: Sam Landfried <samlland@amazon.com>
(cherry picked from commit 442a8cf)
@netty-project-bot

Copy link
Copy Markdown
Contributor

Auto-port PR for 5.0: #16402

@github-actions github-actions Bot removed the needs-cherry-pick-5.0 This PR should be cherry-picked to 5.0 once merged. label Mar 3, 2026
@chrisvest chrisvest removed the needs-cherry-pick-4.2 This PR should be cherry-picked to 4.2 once merged. label Mar 3, 2026
chrisvest added a commit that referenced this pull request Mar 4, 2026
…#16280) (#16401)

### Motivation:

When HttpObjectAggregator.handleOversizedMessage() sends a 413 response
with HTTP/1.1 keep-alive
enabled, it only closes the channel on write failure. On success, the
channel is left open but no
channel.read() is called.

With AUTO_READ=false, this leaves the connection stuck - subsequent
requests sit unread in the kernel buffer until timeout.

### Modification:

Always close the channel after sending 413, not just on write failure.
This matches:
1. The existing comment: "send back a 413 and close the connection"
2. The behavior when keep-alive is disabled
3. The behavior when the message is a FullHttpMessage

Closing is the correct behavior because after an oversized chunked
message, there may be leftover data
in the TCP stream that cannot be reliably skipped in HTTP/1.1.

### Result:

Connections are properly closed after 413, preventing stuck connections
with AUTO_READ=false.

### Repro:

The bug can only be reproed with a "real" channel. This test repros but
isn't committed since the fix to simply close the channel is testable
with a simpler EmbeddedChannel test

```
@test
public void testOversizedRequestWithKeepAliveAndAutoReadFalse() throws InterruptedException {
    final CountDownLatch responseLatch = new CountDownLatch(1);
    final CountDownLatch secondRequestLatch = new CountDownLatch(1);
    final AtomicReference<HttpResponseStatus> statusRef = new AtomicReference<HttpResponseStatus>();

    NioEventLoopGroup group = new NioEventLoopGroup(2);
    try {
        ServerBootstrap sb = new ServerBootstrap();
        sb.group(group)
                .channel(NioServerSocketChannel.class)
                .childOption(ChannelOption.AUTO_READ, false)
                .childHandler(new ChannelInitializer<Channel>() {
                    @OverRide
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpServerCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(4));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpRequest>() {
                            @OverRide
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest msg) {
                                secondRequestLatch.countDown();
                            }
                        });
                        // Trigger the first read manually (AUTO_READ=false requires this)
                        ch.read();
                    }
                });

        Bootstrap cb = new Bootstrap();
        cb.group(group)
                .channel(NioSocketChannel.class)
                .handler(new ChannelInitializer<Channel>() {
                    @OverRide
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpClientCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(1024));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpResponse>() {
                            @OverRide
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpResponse msg) {
                                statusRef.set(msg.status());
                                responseLatch.countDown();
                            }
                        });
                    }
                });

        Channel serverChannel = sb.bind(new InetSocketAddress(0)).sync().channel();
        int port = ((InetSocketAddress) serverChannel.localAddress()).getPort();
        Channel clientChannel = cb.connect(new InetSocketAddress(NetUtil.LOCALHOST, port)).sync().channel();

        HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "/upload");
        HttpUtil.setContentLength(request, 5);
        clientChannel.writeAndFlush(request);
        clientChannel.writeAndFlush(new DefaultLastHttpContent(
                Unpooled.wrappedBuffer(new byte[]{1, 2, 3, 4, 5})));

        // Server should respond with 413
        assertTrue(responseLatch.await(5, SECONDS));
        assertEquals(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, statusRef.get());

        // Send a second request on the same connection. With the bug, the server never
        // calls ctx.read() after the 413, so this request hangs indefinitely.
        clientChannel.writeAndFlush(
                new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/next",
                        Unpooled.EMPTY_BUFFER));

        assertTrue(secondRequestLatch.await(5, SECONDS),
                "Second request was never read - channel is stuck after 413 with AUTO_READ=false");

        clientChannel.close().sync();
        serverChannel.close().sync();
    } finally {
        group.shutdownGracefully();
    }
}
```


(cherry picked from commit 442a8cf)

Co-authored-by: Sam Landfried <slandfried@bignerdranch.com>
Co-authored-by: Sam Landfried <samlland@amazon.com>
normanmaurer added a commit that referenced this pull request Mar 6, 2026
…r 413 with AUTO… (#16402)

Auto-port of #16280 to 5.0
Cherry-picked commit: 442a8cf

---
### Motivation:

When HttpObjectAggregator.handleOversizedMessage() sends a 413 response
with HTTP/1.1 keep-alive
enabled, it only closes the channel on write failure. On success, the
channel is left open but no
channel.read() is called.

With AUTO_READ=false, this leaves the connection stuck -
subsequent requests sit unread in the kernel buffer until timeout.

### Modification:

Always close the channel after sending 413, not just on write failure.
This matches:
1. The existing comment: "send back a 413 and close the connection"
2. The behavior when keep-alive is disabled
3. The behavior when the message is a FullHttpMessage

Closing is the correct behavior because after an oversized chunked
message, there may be leftover data
in the TCP stream that cannot be reliably skipped in HTTP/1.1.

### Result:

Connections are properly closed after 413, preventing stuck connections
with AUTO_READ=false.

### Repro:

The bug can only be reproed with a "real" channel. This test repros but
isn't committed since the fix to simply close the channel is testable
with a simpler EmbeddedChannel test

```
@test
public void testOversizedRequestWithKeepAliveAndAutoReadFalse() throws InterruptedException {
    final CountDownLatch responseLatch = new CountDownLatch(1);
    final CountDownLatch secondRequestLatch = new CountDownLatch(1);
    final AtomicReference<HttpResponseStatus> statusRef = new AtomicReference<HttpResponseStatus>();

    NioEventLoopGroup group = new NioEventLoopGroup(2);
    try {
        ServerBootstrap sb = new ServerBootstrap();
        sb.group(group)
                .channel(NioServerSocketChannel.class)
                .childOption(ChannelOption.AUTO_READ, false)
                .childHandler(new ChannelInitializer<Channel>() {
                    @OverRide
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpServerCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(4));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpRequest>() {
                            @OverRide
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest msg) {
                                secondRequestLatch.countDown();
                            }
                        });
                        // Trigger the first read manually (AUTO_READ=false requires this)
                        ch.read();
                    }
                });

        Bootstrap cb = new Bootstrap();
        cb.group(group)
                .channel(NioSocketChannel.class)
                .handler(new ChannelInitializer<Channel>() {
                    @OverRide
                    protected void initChannel(Channel ch) {
                        ch.pipeline().addLast(new HttpClientCodec());
                        ch.pipeline().addLast(new HttpObjectAggregator(1024));
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpResponse>() {
                            @OverRide
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpResponse msg) {
                                statusRef.set(msg.status());
                                responseLatch.countDown();
                            }
                        });
                    }
                });

        Channel serverChannel = sb.bind(new InetSocketAddress(0)).sync().channel();
        int port = ((InetSocketAddress) serverChannel.localAddress()).getPort();
        Channel clientChannel = cb.connect(new InetSocketAddress(NetUtil.LOCALHOST, port)).sync().channel();

        HttpRequest request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "/upload");
        HttpUtil.setContentLength(request, 5);
        clientChannel.writeAndFlush(request);
        clientChannel.writeAndFlush(new DefaultLastHttpContent(
                Unpooled.wrappedBuffer(new byte[]{1, 2, 3, 4, 5})));

        // Server should respond with 413
        assertTrue(responseLatch.await(5, SECONDS));
        assertEquals(HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, statusRef.get());

        // Send a second request on the same connection. With the bug, the server never
        // calls ctx.read() after the 413, so this request hangs indefinitely.
        clientChannel.writeAndFlush(
                new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/next",
                        Unpooled.EMPTY_BUFFER));

        assertTrue(secondRequestLatch.await(5, SECONDS),
                "Second request was never read - channel is stuck after 413 with AUTO_READ=false");

        clientChannel.close().sync();
        serverChannel.close().sync();
    } finally {
        group.shutdownGracefully();
    }
}
```

---------

Co-authored-by: Sam Landfried <slandfried@bignerdranch.com>
Co-authored-by: Sam Landfried <samlland@amazon.com>
Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants