Skip to content

Commit e0f4682

Browse files
authored
Do not send RST_STREAM when UnprocessedRequestException is raised (#6157)
Motivation: When an `UnprocessedRequestException` is raised, the client should not send an `RST_STREAM`. Modifications: - Updated the `Http2ResponseDecoder` to avoid sending `RST_STREAM` when `UnprocessedRequestException` occurs. Result: - The client does not send an `RST_STREAM` when an `UnprocessedRequestException` is raised.
1 parent 85e40fe commit e0f4682

File tree

2 files changed

+74
-1
lines changed

2 files changed

+74
-1
lines changed

core/src/main/java/com/linecorp/armeria/client/Http2ResponseDecoder.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,22 @@ private void onWrapperCompleted(HttpResponseWrapper resWrapper, int id, @Nullabl
9292
resWrapper.onSubscriptionCancelled(cause);
9393

9494
if (cause != null) {
95+
if (cause instanceof UnprocessedRequestException ||
96+
cause instanceof ClosedStreamException) {
97+
return;
98+
}
99+
final int streamId = idToStreamId(id);
100+
final Http2Stream stream = conn.stream(streamId);
101+
if (stream == null || !stream.isHeadersSent()) {
102+
return;
103+
}
95104
// Removing the response and decrementing `unfinishedResponses` isn't done immediately
96105
// here. Instead, we rely on `Http2ResponseDecoder#onStreamClosed` to decrement
97106
// `unfinishedResponses` after Netty decrements `numActiveStreams` in `DefaultHttp2Connection`
98107
// so that `unfinishedResponses` is never greater than `numActiveStreams`.
99108

100109
// Reset the stream.
101-
final int streamId = idToStreamId(id);
110+
102111
final int lastStreamId = conn.local().lastStreamKnownByPeer();
103112
if (lastStreamId < 0 || // Did not receive a GOAWAY yet or
104113
streamId <= lastStreamId) { // received a GOAWAY and the request's streamId <= lastStreamId
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright 2025 LINE Corporation
3+
*
4+
* LINE Corporation licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
package com.linecorp.armeria.client;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
20+
21+
import java.time.Duration;
22+
import java.util.concurrent.CompletableFuture;
23+
24+
import org.junit.jupiter.api.Test;
25+
import org.junit.jupiter.api.extension.RegisterExtension;
26+
27+
import com.google.common.base.Strings;
28+
29+
import com.linecorp.armeria.common.AggregatedHttpResponse;
30+
import com.linecorp.armeria.common.HttpMethod;
31+
import com.linecorp.armeria.common.HttpResponse;
32+
import com.linecorp.armeria.common.HttpStatus;
33+
import com.linecorp.armeria.common.RequestHeaders;
34+
import com.linecorp.armeria.server.ServerBuilder;
35+
import com.linecorp.armeria.testing.junit5.server.ServerExtension;
36+
37+
import io.netty.handler.codec.http2.Http2Exception.HeaderListSizeException;
38+
39+
class HeaderListSizeExceptionTest {
40+
41+
@RegisterExtension
42+
static ServerExtension server = new ServerExtension() {
43+
@Override
44+
protected void configure(ServerBuilder sb) {
45+
sb.service("/", (ctx, req) -> HttpResponse.delayed(
46+
() -> HttpResponse.of("OK"), Duration.ofMillis(100)));
47+
}
48+
};
49+
50+
@Test
51+
void doNotSendRstStreamWhenHeaderListSizeExceptionIsRaised() throws InterruptedException {
52+
final CompletableFuture<AggregatedHttpResponse> future = server.webClient().get("/").aggregate();
53+
final String a = Strings.repeat("aa", 10000);
54+
final RequestHeaders headers = RequestHeaders.of(HttpMethod.GET, "/", "foo", "bar",
55+
"baz", a);
56+
assertThatThrownBy(() -> server.webClient().execute(headers).aggregate().join())
57+
.hasCauseInstanceOf(UnprocessedRequestException.class)
58+
.cause()
59+
.hasCauseInstanceOf(HeaderListSizeException.class);
60+
// If the client sends RST_STREAM with invalid stream ID, the server will send GOAWAY back thus
61+
// the first request will be failed with ClosedSessionException.
62+
assertThat(future.join().status()).isSameAs(HttpStatus.OK);
63+
}
64+
}

0 commit comments

Comments
 (0)