core,netty,okhttp: strip outbound headers with reserved names (#3098)

These will be stripped:
CONTENT_TYPE, TE, USER_AGENT
This commit is contained in:
zpencer 2017-09-14 15:26:36 -07:00 committed by GitHub
parent 2d711687f9
commit bda67784c0
7 changed files with 156 additions and 7 deletions

View File

@ -100,6 +100,12 @@ public final class GrpcUtil {
public static final Metadata.Key<String> CONTENT_TYPE_KEY =
Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER);
/**
* {@link io.grpc.Metadata.Key} for the Transfer encoding.
*/
public static final Metadata.Key<String> TE_HEADER =
Metadata.Key.of("te", Metadata.ASCII_STRING_MARSHALLER);
/**
* {@link io.grpc.Metadata.Key} for the Content-Type request/response header.
*/

View File

@ -29,7 +29,6 @@ import io.grpc.Metadata;
import io.grpc.MethodDescriptor;
import io.grpc.Status;
import io.grpc.internal.AbstractClientStream;
import io.grpc.internal.GrpcUtil;
import io.grpc.internal.Http2ClientStreamTransportState;
import io.grpc.internal.StatsTraceContext;
import io.grpc.internal.WritableBuffer;
@ -121,7 +120,6 @@ class NettyClientStream extends AbstractClientStream {
} else {
httpMethod = Utils.HTTP_METHOD;
}
headers.discardAll(GrpcUtil.USER_AGENT_KEY);
Http2Headers http2Headers = Utils.convertClientHeaders(headers, scheme, defaultPath,
authority, httpMethod, userAgent);

View File

@ -17,7 +17,6 @@
package io.grpc.netty;
import static io.grpc.internal.GrpcUtil.CONTENT_TYPE_KEY;
import static io.grpc.internal.GrpcUtil.USER_AGENT_KEY;
import static io.grpc.internal.TransportFrameUtil.toHttp2Headers;
import static io.grpc.internal.TransportFrameUtil.toRawSerializedHeaders;
import static io.netty.util.CharsetUtil.UTF_8;
@ -55,9 +54,9 @@ class Utils {
public static final AsciiString HTTP = AsciiString.of("http");
public static final AsciiString CONTENT_TYPE_HEADER = AsciiString.of(CONTENT_TYPE_KEY.name());
public static final AsciiString CONTENT_TYPE_GRPC = AsciiString.of(GrpcUtil.CONTENT_TYPE_GRPC);
public static final AsciiString TE_HEADER = AsciiString.of("te");
public static final AsciiString TE_HEADER = AsciiString.of(GrpcUtil.TE_HEADER.name());
public static final AsciiString TE_TRAILERS = AsciiString.of(GrpcUtil.TE_TRAILERS);
public static final AsciiString USER_AGENT = AsciiString.of(USER_AGENT_KEY.name());
public static final AsciiString USER_AGENT = AsciiString.of(GrpcUtil.USER_AGENT_KEY.name());
public static final Resource<EventLoopGroup> DEFAULT_BOSS_EVENT_LOOP_GROUP =
new DefaultEventLoopGroupResource(1, "grpc-default-boss-ELG");
@ -108,6 +107,11 @@ class Utils {
Preconditions.checkNotNull(authority, "authority");
Preconditions.checkNotNull(method, "method");
// Discard any application supplied duplicates of the reserved headers
headers.discardAll(CONTENT_TYPE_KEY);
headers.discardAll(GrpcUtil.TE_HEADER);
headers.discardAll(GrpcUtil.USER_AGENT_KEY);
return GrpcHttp2OutboundHeaders.clientRequestHeaders(
toHttp2Headers(headers),
authority,
@ -118,6 +122,11 @@ class Utils {
}
public static Http2Headers convertServerHeaders(Metadata headers) {
// Discard any application supplied duplicates of the reserved headers
headers.discardAll(CONTENT_TYPE_KEY);
headers.discardAll(GrpcUtil.TE_HEADER);
headers.discardAll(GrpcUtil.USER_AGENT_KEY);
return GrpcHttp2OutboundHeaders.serverResponseHeaders(toHttp2Headers(headers));
}

View File

@ -19,10 +19,16 @@ package io.grpc.netty;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertSame;
import io.grpc.Metadata;
import io.grpc.Status;
import io.grpc.internal.GrpcUtil;
import io.netty.channel.ConnectTimeoutException;
import io.netty.handler.codec.http2.DefaultHttp2Headers;
import io.netty.handler.codec.http2.Http2Error;
import io.netty.handler.codec.http2.Http2Exception;
import io.netty.handler.codec.http2.Http2Headers;
import io.netty.util.AsciiString;
import java.util.Map;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@ -30,6 +36,10 @@ import org.junit.runners.JUnit4;
/** Unit tests for {@link Utils}. */
@RunWith(JUnit4.class)
public class UtilsTest {
private final Metadata.Key<String> userKey =
Metadata.Key.of("user-key", Metadata.ASCII_STRING_MARSHALLER);
private final String userValue = "user-value";
@Test
public void testStatusFromThrowable() {
Status s = Status.CANCELLED.withDescription("msg");
@ -43,6 +53,69 @@ public class UtilsTest {
assertStatusEquals(Status.UNKNOWN.withCause(t), Utils.statusFromThrowable(t));
}
@Test
public void convertClientHeaders_sanitizes() {
Metadata metaData = new Metadata();
// Intentionally being explicit here rather than relying on any pre-defined lists of headers,
// since the goal of this test is to validate the correctness of such lists in the first place.
metaData.put(GrpcUtil.CONTENT_TYPE_KEY, "to-be-removed");
metaData.put(GrpcUtil.USER_AGENT_KEY, "to-be-removed");
metaData.put(GrpcUtil.TE_HEADER, "to-be-removed");
metaData.put(userKey, userValue);
String scheme = "https";
String userAgent = "user-agent";
String method = "POST";
String authority = "authority";
String path = "//testService/test";
Http2Headers output =
Utils.convertClientHeaders(
metaData,
new AsciiString(scheme),
new AsciiString(path),
new AsciiString(authority),
new AsciiString(method),
new AsciiString(userAgent));
DefaultHttp2Headers headers = new DefaultHttp2Headers();
for (Map.Entry<CharSequence, CharSequence> entry : output) {
headers.add(entry.getKey(), entry.getValue());
}
// 7 reserved headers, 1 user header
assertEquals(7 + 1, headers.size());
// Check the 3 reserved headers that are non pseudo
// Users can not create pseudo headers keys so no need to check for them here
assertEquals(GrpcUtil.CONTENT_TYPE_GRPC,
headers.get(GrpcUtil.CONTENT_TYPE_KEY.name()).toString());
assertEquals(userAgent, headers.get(GrpcUtil.USER_AGENT_KEY.name()).toString());
assertEquals(GrpcUtil.TE_TRAILERS, headers.get(GrpcUtil.TE_HEADER.name()).toString());
// Check the user header is in tact
assertEquals(userValue, headers.get(userKey.name()).toString());
}
@Test
public void convertServerHeaders_sanitizes() {
Metadata metaData = new Metadata();
// Intentionally being explicit here rather than relying on any pre-defined lists of headers,
// since the goal of this test is to validate the correctness of such lists in the first place.
metaData.put(GrpcUtil.CONTENT_TYPE_KEY, "to-be-removed");
metaData.put(GrpcUtil.TE_HEADER, "to-be-removed");
metaData.put(GrpcUtil.USER_AGENT_KEY, "to-be-removed");
metaData.put(userKey, userValue);
Http2Headers output = Utils.convertServerHeaders(metaData);
DefaultHttp2Headers headers = new DefaultHttp2Headers();
for (Map.Entry<CharSequence, CharSequence> entry : output) {
headers.add(entry.getKey(), entry.getValue());
}
// 2 reserved headers, 1 user header
assertEquals(2 + 1, headers.size());
assertEquals(Utils.CONTENT_TYPE_GRPC, headers.get(GrpcUtil.CONTENT_TYPE_KEY.name()));
}
private static void assertStatusEquals(Status expected, Status actual) {
assertEquals(expected.getCode(), actual.getCode());
assertEquals(expected.getDescription(), actual.getDescription());

View File

@ -52,6 +52,11 @@ class Headers {
Preconditions.checkNotNull(defaultPath, "defaultPath");
Preconditions.checkNotNull(authority, "authority");
// Discard any application supplied duplicates of the reserved headers
headers.discardAll(GrpcUtil.CONTENT_TYPE_KEY);
headers.discardAll(GrpcUtil.TE_HEADER);
headers.discardAll(GrpcUtil.USER_AGENT_KEY);
// 7 is the number of explicit add calls below.
List<Header> okhttpHeaders = new ArrayList<Header>(7 + InternalMetadata.headerCount(headers));

View File

@ -25,7 +25,6 @@ import io.grpc.Metadata;
import io.grpc.MethodDescriptor;
import io.grpc.Status;
import io.grpc.internal.AbstractClientStream;
import io.grpc.internal.GrpcUtil;
import io.grpc.internal.Http2ClientStreamTransportState;
import io.grpc.internal.StatsTraceContext;
import io.grpc.internal.WritableBuffer;
@ -127,7 +126,6 @@ class OkHttpClientStream extends AbstractClientStream {
useGet = true;
defaultPath += "?" + BaseEncoding.base64().encode(payload);
}
metadata.discardAll(GrpcUtil.USER_AGENT_KEY);
synchronized (state.lock) {
state.streamReady(metadata, defaultPath);
}

View File

@ -0,0 +1,60 @@
/*
* Copyright 2017, gRPC Authors All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.grpc.okhttp;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import io.grpc.Metadata;
import io.grpc.internal.GrpcUtil;
import io.grpc.okhttp.internal.framed.Header;
import java.util.List;
import org.junit.Test;
public class HeadersTest {
@Test
public void createRequestHeaders_sanitizes() {
Metadata metaData = new Metadata();
// Intentionally being explicit here rather than relying on any pre-defined lists of headers,
// since the goal of this test is to validate the correctness of such lists in the first place.
metaData.put(GrpcUtil.CONTENT_TYPE_KEY, "to-be-removed");
metaData.put(GrpcUtil.USER_AGENT_KEY, "to-be-removed");
metaData.put(GrpcUtil.TE_HEADER, "to-be-removed");
Metadata.Key<String> userKey = Metadata.Key.of("user-key", Metadata.ASCII_STRING_MARSHALLER);
String userValue = "user-value";
metaData.put(userKey, userValue);
String path = "//testServerice/test";
String authority = "localhost";
String userAgent = "useragent";
List<Header> headers = Headers.createRequestHeaders(metaData, path, authority, userAgent);
// 7 reserved headers, 1 user header
assertEquals(7 + 1, headers.size());
// Check the 3 reserved headers that are non pseudo
// Users can not create pseudo headers keys so no need to check for them here
assertThat(headers).contains(Headers.CONTENT_TYPE_HEADER);
assertThat(headers).contains(new Header(GrpcUtil.USER_AGENT_KEY.name(), userAgent));
assertThat(headers).contains(new Header(GrpcUtil.TE_HEADER.name(), GrpcUtil.TE_TRAILERS));
// Check the user header is in tact
assertThat(headers).contains(new Header(userKey.name(), userValue));
}
}