Skip to content

Commit f4ed575

Browse files
JoeCquptAgraVator
JoeCqupt
authored and
AgraVator
committed
netty, okhttp: Add allow header for response code 405 (#12334)
Fixes #12329
1 parent 339ef99 commit f4ed575

File tree

4 files changed

+43
-7
lines changed

4 files changed

+43
-7
lines changed

‎netty/src/main/java/io/grpc/netty/NettyServerHandler.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import io.netty.channel.ChannelFutureListener;
6161
import io.netty.channel.ChannelHandlerContext;
6262
import io.netty.channel.ChannelPromise;
63+
import io.netty.handler.codec.http.HttpHeaderNames;
6364
import io.netty.handler.codec.http2.DecoratingHttp2ConnectionEncoder;
6465
import io.netty.handler.codec.http2.DecoratingHttp2FrameWriter;
6566
import io.netty.handler.codec.http2.DefaultHttp2Connection;
@@ -70,6 +71,7 @@
7071
import io.netty.handler.codec.http2.DefaultHttp2Headers;
7172
import io.netty.handler.codec.http2.DefaultHttp2LocalFlowController;
7273
import io.netty.handler.codec.http2.DefaultHttp2RemoteFlowController;
74+
import io.netty.handler.codec.http2.EmptyHttp2Headers;
7375
import io.netty.handler.codec.http2.Http2Connection;
7476
import io.netty.handler.codec.http2.Http2ConnectionAdapter;
7577
import io.netty.handler.codec.http2.Http2ConnectionDecoder;
@@ -480,8 +482,10 @@ private void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers
480482
}
481483

482484
if (!HTTP_METHOD.contentEquals(headers.method())) {
485+
Http2Headers extraHeaders = new DefaultHttp2Headers();
486+
extraHeaders.add(HttpHeaderNames.ALLOW, HTTP_METHOD);
483487
respondWithHttpError(ctx, streamId, 405, Status.Code.INTERNAL,
484-
String.format("Method '%s' is not supported", headers.method()));
488+
String.format("Method '%s' is not supported", headers.method()), extraHeaders);
485489
return;
486490
}
487491

@@ -869,6 +873,12 @@ public boolean visit(Http2Stream stream) throws Http2Exception {
869873

870874
private void respondWithHttpError(
871875
ChannelHandlerContext ctx, int streamId, int code, Status.Code statusCode, String msg) {
876+
respondWithHttpError(ctx, streamId, code, statusCode, msg, EmptyHttp2Headers.INSTANCE);
877+
}
878+
879+
private void respondWithHttpError(
880+
ChannelHandlerContext ctx, int streamId, int code, Status.Code statusCode, String msg,
881+
Http2Headers extraHeaders) {
872882
Metadata metadata = new Metadata();
873883
metadata.put(InternalStatus.CODE_KEY, statusCode.toStatus());
874884
metadata.put(InternalStatus.MESSAGE_KEY, msg);
@@ -880,6 +890,7 @@ private void respondWithHttpError(
880890
for (int i = 0; i < serialized.length; i += 2) {
881891
headers.add(new AsciiString(serialized[i], false), new AsciiString(serialized[i + 1], false));
882892
}
893+
headers.add(extraHeaders);
883894
encoder().writeHeaders(ctx, streamId, headers, 0, false, ctx.newPromise());
884895
ByteBuf msgBuf = ByteBufUtil.writeUtf8(ctx.alloc(), msg);
885896
encoder().writeData(ctx, streamId, msgBuf, 0, true, ctx.newPromise());

‎netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
import io.netty.channel.ChannelFuture;
7979
import io.netty.channel.ChannelHandlerContext;
8080
import io.netty.channel.ChannelPromise;
81+
import io.netty.handler.codec.http.HttpHeaderNames;
8182
import io.netty.handler.codec.http2.DefaultHttp2Headers;
8283
import io.netty.handler.codec.http2.Http2CodecUtil;
8384
import io.netty.handler.codec.http2.Http2Error;
@@ -542,7 +543,8 @@ public void headersWithInvalidMethodShouldFail() throws Exception {
542543
.set(InternalStatus.CODE_KEY.name(), String.valueOf(Code.INTERNAL.value()))
543544
.set(InternalStatus.MESSAGE_KEY.name(), "Method 'FAKE' is not supported")
544545
.status("" + 405)
545-
.set(CONTENT_TYPE_HEADER, "text/plain; charset=utf-8");
546+
.set(CONTENT_TYPE_HEADER, "text/plain; charset=utf-8")
547+
.set(HttpHeaderNames.ALLOW, HTTP_METHOD);
546548

547549
verifyWrite()
548550
.writeHeaders(

‎okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static io.grpc.okhttp.OkHttpServerBuilder.MAX_CONNECTION_IDLE_NANOS_DISABLED;
2121

2222
import com.google.common.base.Preconditions;
23+
import com.google.common.collect.Lists;
2324
import com.google.common.util.concurrent.Futures;
2425
import com.google.common.util.concurrent.ListenableFuture;
2526
import com.google.errorprone.annotations.concurrent.GuardedBy;
@@ -52,6 +53,7 @@
5253
import java.io.IOException;
5354
import java.net.Socket;
5455
import java.net.SocketException;
56+
import java.util.Collections;
5557
import java.util.List;
5658
import java.util.Locale;
5759
import java.util.Map;
@@ -91,6 +93,7 @@ final class OkHttpServerTransport implements ServerTransport,
9193
private static final ByteString TE_TRAILERS = ByteString.encodeUtf8("trailers");
9294
private static final ByteString CONTENT_TYPE = ByteString.encodeUtf8("content-type");
9395
private static final ByteString CONTENT_LENGTH = ByteString.encodeUtf8("content-length");
96+
private static final ByteString ALLOW = ByteString.encodeUtf8("allow");
9497

9598
private final Config config;
9699
private final Variant variant = new Http2();
@@ -772,8 +775,9 @@ public void headers(boolean outFinished,
772775
}
773776

774777
if (!POST_METHOD.equals(httpMethod)) {
778+
List<Header> extraHeaders = Lists.newArrayList(new Header(ALLOW, POST_METHOD));
775779
respondWithHttpError(streamId, inFinished, 405, Status.Code.INTERNAL,
776-
"HTTP Method is not supported: " + asciiString(httpMethod));
780+
"HTTP Method is not supported: " + asciiString(httpMethod), extraHeaders);
777781
return;
778782
}
779783

@@ -1066,11 +1070,19 @@ private void streamError(int streamId, ErrorCode errorCode, String reason) {
10661070

10671071
private void respondWithHttpError(
10681072
int streamId, boolean inFinished, int httpCode, Status.Code statusCode, String msg) {
1073+
respondWithHttpError(streamId, inFinished, httpCode, statusCode, msg,
1074+
Collections.emptyList());
1075+
}
1076+
1077+
private void respondWithHttpError(
1078+
int streamId, boolean inFinished, int httpCode, Status.Code statusCode, String msg,
1079+
List<Header> extraHeaders) {
10691080
Metadata metadata = new Metadata();
10701081
metadata.put(InternalStatus.CODE_KEY, statusCode.toStatus());
10711082
metadata.put(InternalStatus.MESSAGE_KEY, msg);
10721083
List<Header> headers =
10731084
Headers.createHttpResponseHeaders(httpCode, "text/plain; charset=utf-8", metadata);
1085+
headers.addAll(extraHeaders);
10741086
Buffer data = new Buffer().writeUtf8(msg);
10751087

10761088
synchronized (lock) {

‎okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import static org.mockito.Mockito.timeout;
3535
import static org.mockito.Mockito.verify;
3636

37+
import com.google.common.collect.Lists;
3738
import com.google.common.io.ByteStreams;
3839
import io.grpc.Attributes;
3940
import io.grpc.InternalChannelz.SocketStats;
@@ -62,6 +63,7 @@
6263
import java.util.ArrayDeque;
6364
import java.util.ArrayList;
6465
import java.util.Arrays;
66+
import java.util.Collections;
6567
import java.util.Deque;
6668
import java.util.List;
6769
import java.util.concurrent.CountDownLatch;
@@ -919,8 +921,9 @@ public void httpGet_failsWith405() throws Exception {
919921
CONTENT_TYPE_HEADER,
920922
TE_HEADER));
921923
clientFrameWriter.flush();
922-
923-
verifyHttpError(1, 405, Status.Code.INTERNAL, "HTTP Method is not supported: GET");
924+
List<Header> extraHeaders = Lists.newArrayList(new Header("allow", "POST"));
925+
verifyHttpError(1, 405, Status.Code.INTERNAL, "HTTP Method is not supported: GET",
926+
extraHeaders);
924927

925928
shutdownAndTerminate(/*lastStreamId=*/ 1);
926929
}
@@ -976,7 +979,8 @@ public void httpErrorsAdhereToFlowControl() throws Exception {
976979
new Header(":status", "405"),
977980
new Header("content-type", "text/plain; charset=utf-8"),
978981
new Header("grpc-status", "" + Status.Code.INTERNAL.value()),
979-
new Header("grpc-message", errorDescription));
982+
new Header("grpc-message", errorDescription),
983+
new Header("allow", "POST"));
980984
assertThat(clientFrameReader.nextFrame(clientFramesRead)).isTrue();
981985
verify(clientFramesRead)
982986
.headers(false, false, 1, -1, responseHeaders, HeadersMode.HTTP_20_HEADERS);
@@ -1398,11 +1402,18 @@ private void pingPong() throws IOException {
13981402

13991403
private void verifyHttpError(
14001404
int streamId, int httpCode, Status.Code grpcCode, String errorDescription) throws Exception {
1401-
List<Header> responseHeaders = Arrays.asList(
1405+
verifyHttpError(streamId, httpCode, grpcCode, errorDescription, Collections.emptyList());
1406+
}
1407+
1408+
private void verifyHttpError(
1409+
int streamId, int httpCode, Status.Code grpcCode, String errorDescription,
1410+
List<Header> extraHeaders) throws Exception {
1411+
List<Header> responseHeaders = Lists.newArrayList(
14021412
new Header(":status", "" + httpCode),
14031413
new Header("content-type", "text/plain; charset=utf-8"),
14041414
new Header("grpc-status", "" + grpcCode.value()),
14051415
new Header("grpc-message", errorDescription));
1416+
responseHeaders.addAll(extraHeaders);
14061417
assertThat(clientFrameReader.nextFrame(clientFramesRead)).isTrue();
14071418
verify(clientFramesRead)
14081419
.headers(false, false, streamId, -1, responseHeaders, HeadersMode.HTTP_20_HEADERS);

0 commit comments

Comments
 (0)