Skip to content

Commit c9f5c3b

Browse files
committed
Netty & OkHttp : Added Allow header on 405
405 response: include Allow: POST http: added Allow header for 405
1 parent 5a54372 commit c9f5c3b

File tree

4 files changed

+77
-20
lines changed

4 files changed

+77
-20
lines changed

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

Lines changed: 22 additions & 2 deletions
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,15 @@ private void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers
480482
}
481483

482484
if (!HTTP_METHOD.contentEquals(headers.method())) {
483-
respondWithHttpError(ctx, streamId, 405, Status.Code.INTERNAL,
484-
String.format("Method '%s' is not supported", headers.method()));
485+
Http2Headers allowHeaders = new DefaultHttp2Headers();
486+
allowHeaders.add(HttpHeaderNames.ALLOW, HTTP_METHOD);
487+
respondWithHttpError(
488+
ctx,
489+
streamId,
490+
405,
491+
Status.Code.INTERNAL,
492+
String.format("Method '%s' is not supported", headers.method()),
493+
allowHeaders);
485494
return;
486495
}
487496

@@ -869,6 +878,16 @@ public boolean visit(Http2Stream stream) throws Http2Exception {
869878

870879
private void respondWithHttpError(
871880
ChannelHandlerContext ctx, int streamId, int code, Status.Code statusCode, String msg) {
881+
respondWithHttpError(ctx, streamId, code, statusCode, msg, EmptyHttp2Headers.INSTANCE);
882+
}
883+
884+
private void respondWithHttpError(
885+
ChannelHandlerContext ctx,
886+
int streamId,
887+
int code,
888+
Status.Code statusCode,
889+
String msg,
890+
Http2Headers allowHeaders) {
872891
Metadata metadata = new Metadata();
873892
metadata.put(InternalStatus.CODE_KEY, statusCode.toStatus());
874893
metadata.put(InternalStatus.MESSAGE_KEY, msg);
@@ -880,6 +899,7 @@ private void respondWithHttpError(
880899
for (int i = 0; i < serialized.length; i += 2) {
881900
headers.add(new AsciiString(serialized[i], false), new AsciiString(serialized[i + 1], false));
882901
}
902+
headers.add(allowHeaders);
883903
encoder().writeHeaders(ctx, streamId, headers, 0, false, ctx.newPromise());
884904
ByteBuf msgBuf = ByteBufUtil.writeUtf8(ctx.alloc(), msg);
885905
encoder().writeData(ctx, streamId, msgBuf, 0, true, ctx.newPromise());

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

Lines changed: 8 additions & 5 deletions
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;
@@ -538,11 +539,13 @@ public void headersWithInvalidMethodShouldFail() throws Exception {
538539
.path(new AsciiString("/foo/bar"));
539540
ByteBuf headersFrame = headersFrame(STREAM_ID, headers);
540541
channelRead(headersFrame);
541-
Http2Headers responseHeaders = new DefaultHttp2Headers()
542-
.set(InternalStatus.CODE_KEY.name(), String.valueOf(Code.INTERNAL.value()))
543-
.set(InternalStatus.MESSAGE_KEY.name(), "Method 'FAKE' is not supported")
544-
.status("" + 405)
545-
.set(CONTENT_TYPE_HEADER, "text/plain; charset=utf-8");
542+
Http2Headers responseHeaders =
543+
new DefaultHttp2Headers()
544+
.set(InternalStatus.CODE_KEY.name(), String.valueOf(Code.INTERNAL.value()))
545+
.set(InternalStatus.MESSAGE_KEY.name(), "Method 'FAKE' is not supported")
546+
.status("" + 405)
547+
.set(HttpHeaderNames.ALLOW, "POST")
548+
.set(CONTENT_TYPE_HEADER, "text/plain; charset=utf-8");
546549

547550
verifyWrite()
548551
.writeHeaders(

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

Lines changed: 23 additions & 2 deletions
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,14 @@ public void headers(boolean outFinished,
772775
}
773776

774777
if (!POST_METHOD.equals(httpMethod)) {
775-
respondWithHttpError(streamId, inFinished, 405, Status.Code.INTERNAL,
776-
"HTTP Method is not supported: " + asciiString(httpMethod));
778+
List<Header> allowHeaders = Lists.newArrayList(new Header(ALLOW, POST_METHOD));
779+
respondWithHttpError(
780+
streamId,
781+
inFinished,
782+
405,
783+
Status.Code.INTERNAL,
784+
"HTTP Method is not supported: " + asciiString(httpMethod),
785+
allowHeaders);
777786
return;
778787
}
779788

@@ -1066,11 +1075,23 @@ private void streamError(int streamId, ErrorCode errorCode, String reason) {
10661075

10671076
private void respondWithHttpError(
10681077
int streamId, boolean inFinished, int httpCode, Status.Code statusCode, String msg) {
1078+
respondWithHttpError(
1079+
streamId, inFinished, httpCode, statusCode, msg, Collections.emptyList());
1080+
}
1081+
1082+
private void respondWithHttpError(
1083+
int streamId,
1084+
boolean inFinished,
1085+
int httpCode,
1086+
Status.Code statusCode,
1087+
String msg,
1088+
List<Header> allowHeaders) {
10691089
Metadata metadata = new Metadata();
10701090
metadata.put(InternalStatus.CODE_KEY, statusCode.toStatus());
10711091
metadata.put(InternalStatus.MESSAGE_KEY, msg);
10721092
List<Header> headers =
10731093
Headers.createHttpResponseHeaders(httpCode, "text/plain; charset=utf-8", metadata);
1094+
headers.addAll(allowHeaders);
10741095
Buffer data = new Buffer().writeUtf8(msg);
10751096

10761097
synchronized (lock) {

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

Lines changed: 24 additions & 11 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;
@@ -920,7 +922,9 @@ public void httpGet_failsWith405() throws Exception {
920922
TE_HEADER));
921923
clientFrameWriter.flush();
922924

923-
verifyHttpError(1, 405, Status.Code.INTERNAL, "HTTP Method is not supported: GET");
925+
List<Header> extraHeaders = Lists.newArrayList(new Header("allow", "POST"));
926+
verifyHttpError(
927+
1, 405, Status.Code.INTERNAL, "HTTP Method is not supported: GET", extraHeaders);
924928

925929
shutdownAndTerminate(/*lastStreamId=*/ 1);
926930
}
@@ -972,11 +976,13 @@ public void httpErrorsAdhereToFlowControl() throws Exception {
972976
clientFrameWriter.flush();
973977

974978
String errorDescription = "HTTP Method is not supported: GET";
975-
List<Header> responseHeaders = Arrays.asList(
976-
new Header(":status", "405"),
977-
new Header("content-type", "text/plain; charset=utf-8"),
978-
new Header("grpc-status", "" + Status.Code.INTERNAL.value()),
979-
new Header("grpc-message", errorDescription));
979+
List<Header> responseHeaders =
980+
Arrays.asList(
981+
new Header(":status", "405"),
982+
new Header("content-type", "text/plain; charset=utf-8"),
983+
new Header("grpc-status", "" + Status.Code.INTERNAL.value()),
984+
new Header("grpc-message", errorDescription),
985+
new Header("allow", "POST"));
980986
assertThat(clientFrameReader.nextFrame(clientFramesRead)).isTrue();
981987
verify(clientFramesRead)
982988
.headers(false, false, 1, -1, responseHeaders, HeadersMode.HTTP_20_HEADERS);
@@ -1398,11 +1404,18 @@ private void pingPong() throws IOException {
13981404

13991405
private void verifyHttpError(
14001406
int streamId, int httpCode, Status.Code grpcCode, String errorDescription) throws Exception {
1401-
List<Header> responseHeaders = Arrays.asList(
1402-
new Header(":status", "" + httpCode),
1403-
new Header("content-type", "text/plain; charset=utf-8"),
1404-
new Header("grpc-status", "" + grpcCode.value()),
1405-
new Header("grpc-message", errorDescription));
1407+
verifyHttpError(streamId, httpCode, grpcCode, errorDescription, Collections.emptyList());
1408+
}
1409+
1410+
private void verifyHttpError(
1411+
int streamId, int httpCode, Status.Code grpcCode, String errorDescription,
1412+
List<Header> extraHeaders) throws Exception {
1413+
List<Header> responseHeaders = Lists.newArrayList(
1414+
new Header(":status", "" + httpCode),
1415+
new Header("content-type", "text/plain; charset=utf-8"),
1416+
new Header("grpc-status", "" + grpcCode.value()),
1417+
new Header("grpc-message", errorDescription));
1418+
responseHeaders.addAll(extraHeaders);
14061419
assertThat(clientFrameReader.nextFrame(clientFramesRead)).isTrue();
14071420
verify(clientFramesRead)
14081421
.headers(false, false, streamId, -1, responseHeaders, HeadersMode.HTTP_20_HEADERS);

0 commit comments

Comments
 (0)