Skip to content

Commit 0763a39

Browse files
authored
Remove content-length header in gRPC trailers-only responses (#6511)
Motivation: When a `content-length` header is mistakenly set for a gRPC trailers-only response, it must be removed. Modifications: - Remove content-length header in `statusToTrailers` method. - A simple fix, like removing the header from all responses with an empty body, is not feasible because it would break valid use cases, such as responses to HEAD requests, which correctly include a `content-length` header but no body. Result: - The gRPC service now sends spec-compliant trailers-only responses without a `content-length` header.
1 parent 9bc31ae commit 0763a39

File tree

2 files changed

+110
-0
lines changed

2 files changed

+110
-0
lines changed

grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/AbstractServerCall.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,7 @@ protected final HttpObject responseTrailers(ServiceRequestContext ctx, Status st
571571
public static HttpHeaders statusToTrailers(
572572
ServiceRequestContext ctx, HttpHeadersBuilder trailersBuilder,
573573
Status status, @Nullable Metadata metadata) {
574+
trailersBuilder.endOfStream(true);
574575
try {
575576
MetadataUtil.fillHeaders(metadata, trailersBuilder);
576577
} catch (Exception e) {
@@ -594,6 +595,8 @@ public static HttpHeaders statusToTrailers(
594595
final HttpHeaders additionalTrailers = ctx.additionalResponseTrailers();
595596
ctx.mutateAdditionalResponseTrailers(HttpHeadersBuilder::clear);
596597
trailersBuilder.add(additionalTrailers);
598+
// Remove content-length just in case it was set mistakenly.
599+
trailersBuilder.remove(HttpHeaderNames.CONTENT_LENGTH);
597600
return trailersBuilder.build();
598601
}
599602

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/*
2+
* Copyright 2022 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+
17+
package com.linecorp.armeria.server.grpc;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
21+
22+
import java.util.concurrent.atomic.AtomicReference;
23+
24+
import org.junit.jupiter.api.Test;
25+
import org.junit.jupiter.api.extension.RegisterExtension;
26+
27+
import com.linecorp.armeria.client.grpc.GrpcClients;
28+
import com.linecorp.armeria.common.CommonPools;
29+
import com.linecorp.armeria.common.HttpHeaderNames;
30+
import com.linecorp.armeria.common.HttpResponse;
31+
import com.linecorp.armeria.common.ResponseHeaders;
32+
import com.linecorp.armeria.internal.common.grpc.TestServiceImpl;
33+
import com.linecorp.armeria.server.ServerBuilder;
34+
import com.linecorp.armeria.testing.junit5.server.ServerExtension;
35+
36+
import io.grpc.Metadata;
37+
import io.grpc.ServerCall;
38+
import io.grpc.ServerCall.Listener;
39+
import io.grpc.ServerCallHandler;
40+
import io.grpc.ServerInterceptor;
41+
import io.grpc.Status;
42+
import io.grpc.StatusRuntimeException;
43+
import testing.grpc.Messages.SimpleRequest;
44+
import testing.grpc.TestServiceGrpc.TestServiceBlockingStub;
45+
46+
class InvalidContentLengthHeaderTest {
47+
48+
private static final Metadata.Key<String> CONTENT_LENGTH =
49+
Metadata.Key.of("content-length", Metadata.ASCII_STRING_MARSHALLER);
50+
51+
private static final AtomicReference<ResponseHeaders> responseHeadersCapture = new AtomicReference<>();
52+
53+
@RegisterExtension
54+
static ServerExtension server = new ServerExtension() {
55+
@Override
56+
protected void configure(ServerBuilder sb) {
57+
final ServerInterceptor interceptor = new ServerInterceptor() {
58+
@Override
59+
public <I, O> Listener<I> interceptCall(ServerCall<I, O> call,
60+
Metadata requestHeaders,
61+
ServerCallHandler<I, O> next) {
62+
final Metadata metadata = new Metadata();
63+
metadata.put(CONTENT_LENGTH, "555");
64+
call.close(Status.UNAUTHENTICATED, metadata);
65+
return new Listener<I>() {};
66+
}
67+
};
68+
69+
final GrpcService grpcService =
70+
GrpcService.builder()
71+
.addService(new TestServiceImpl(CommonPools.blockingTaskExecutor()))
72+
.intercept(interceptor)
73+
.build();
74+
75+
sb.service(grpcService);
76+
sb.decorator((delegate, ctx, req) -> {
77+
final HttpResponse res = delegate.serve(ctx, req);
78+
// Capture response headers.
79+
return res.mapHeaders(headers -> {
80+
responseHeadersCapture.set(headers);
81+
return headers;
82+
});
83+
});
84+
}
85+
};
86+
87+
@Test
88+
void contentLengthRemovedFromAdditionalTrailers() {
89+
final TestServiceBlockingStub client =
90+
GrpcClients.newClient(server.httpUri(),
91+
TestServiceBlockingStub.class);
92+
93+
assertThatThrownBy(() -> {
94+
client.unaryCall(SimpleRequest.newBuilder().setResponseSize(100).build());
95+
}).isInstanceOf(StatusRuntimeException.class)
96+
.hasMessageContaining("UNAUTHENTICATED");
97+
98+
final ResponseHeaders serverResponseHeaders = responseHeadersCapture.get();
99+
assertThat(serverResponseHeaders).isNotNull();
100+
101+
// The mistakenly set content-length (500) is removed by statusToTrailers() and is set to 0
102+
// in ArmeriaHttpUtil.maybeUpdateContentLengthAndEndOfStream.
103+
final String contentLength = serverResponseHeaders.get(HttpHeaderNames.CONTENT_LENGTH);
104+
assertThat(contentLength).isEqualTo("0");
105+
assertThat(serverResponseHeaders.isEndOfStream()).isTrue();
106+
}
107+
}

0 commit comments

Comments
 (0)