From 2b116ef2cdf59f862334668fff39c00fa59a3c71 Mon Sep 17 00:00:00 2001 From: zhangkun Date: Wed, 3 Dec 2014 20:28:00 -0800 Subject: [PATCH] Encode binary headers with Base64 on the wire, and requires all binary headers have "-bin" suffix in their names. Split Metadata.Marshaller into BinaryMarshaller and AsciiMarshaller. ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=81306135 --- .../stubby/auth/OAuth2ChannelInterceptor.java | 2 +- .../java/com/google/net/stubby/Metadata.java | 276 +++++++++--------- .../java/com/google/net/stubby/Status.java | 22 +- .../google/net/stubby/proto/ProtoUtils.java | 16 +- .../stubby/transport/Http2ClientStream.java | 20 +- .../google/net/stubby/transport/HttpUtil.java | 5 +- .../stubby/transport/TransportFrameUtil.java | 98 +++++++ .../net/stubby/ClientInterceptorsTest.java | 2 +- .../com/google/net/stubby/MetadataTest.java | 33 +-- .../transport/TransportFrameUtilTest.java | 102 +++++++ .../net/stubby/transport/netty/Utils.java | 9 +- .../net/stubby/transport/okhttp/Headers.java | 7 +- .../net/stubby/transport/okhttp/Utils.java | 3 +- 13 files changed, 368 insertions(+), 227 deletions(-) create mode 100644 core/src/test/java/com/google/net/stubby/transport/TransportFrameUtilTest.java diff --git a/auth/src/main/java/com/google/net/stubby/auth/OAuth2ChannelInterceptor.java b/auth/src/main/java/com/google/net/stubby/auth/OAuth2ChannelInterceptor.java index 3f11af43d8..8ab2ef3530 100644 --- a/auth/src/main/java/com/google/net/stubby/auth/OAuth2ChannelInterceptor.java +++ b/auth/src/main/java/com/google/net/stubby/auth/OAuth2ChannelInterceptor.java @@ -15,7 +15,7 @@ import javax.inject.Provider; /** Client interceptor that authenticates all calls with OAuth2. */ public class OAuth2ChannelInterceptor implements ClientInterceptor { private static final Metadata.Key AUTHORIZATION = - Metadata.Key.of("Authorization", Metadata.STRING_MARSHALLER); + Metadata.Key.of("Authorization", Metadata.ASCII_STRING_MARSHALLER); private final OAuth2AccessTokenProvider accessTokenProvider; private final Provider authorizationHeaderProvider diff --git a/core/src/main/java/com/google/net/stubby/Metadata.java b/core/src/main/java/com/google/net/stubby/Metadata.java index 5c69c19216..4f32e182c3 100644 --- a/core/src/main/java/com/google/net/stubby/Metadata.java +++ b/core/src/main/java/com/google/net/stubby/Metadata.java @@ -1,12 +1,10 @@ package com.google.net.stubby; import static com.google.common.base.Charsets.US_ASCII; -import static com.google.common.base.Charsets.UTF_8; import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.Iterables; -import com.google.common.collect.Iterators; import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; @@ -28,6 +26,11 @@ import javax.annotation.concurrent.NotThreadSafe; @NotThreadSafe public abstract class Metadata { + /** + * All binary headers should have this suffix in their names. Vice versa. + */ + public static final String BINARY_HEADER_SUFFIX = "-bin"; + /** * Interleave keys and values into a single iterator. */ @@ -60,65 +63,38 @@ public abstract class Metadata { } /** - * Simple metadata marshaller that encodes strings as either UTF-8 or ASCII bytes. + * Simple metadata marshaller that encodes strings as is. + * + *

This should be used with ASCII strings that only contain printable characters and space. + * Otherwise the output may be considered invalid and discarded by the transport. */ - public static final Marshaller STRING_MARSHALLER = - new Marshaller() { + public static final AsciiMarshaller ASCII_STRING_MARSHALLER = + new AsciiMarshaller() { @Override - public byte[] toBytes(String value) { - return value.getBytes(UTF_8); - } - - @Override - public String toAscii(String value) { + public String toAsciiString(String value) { return value; } @Override - public String parseBytes(byte[] serialized) { - return new String(serialized, UTF_8); - } - - @Override - public String parseAscii(String ascii) { - return ascii; + public String parseAsciiString(String serialized) { + return serialized; } }; /** - * Simple metadata marshaller that encodes an integer as a signed decimal string or as big endian - * binary with four bytes. + * Simple metadata marshaller that encodes an integer as a signed decimal string. */ - public static final Marshaller INTEGER_MARSHALLER = new Marshaller() { - @Override - public byte[] toBytes(Integer value) { - return new byte[] { - (byte) (value >>> 24), - (byte) (value >>> 16), - (byte) (value >>> 8), - (byte) (value >>> 0)}; - } + public static final AsciiMarshaller INTEGER_MARSHALLER = new AsciiMarshaller() { @Override - public String toAscii(Integer value) { + public String toAsciiString(Integer value) { return value.toString(); } @Override - public Integer parseBytes(byte[] serialized) { - if (serialized.length != 4) { - throw new IllegalArgumentException("Can only deserialize 4 bytes into an integer"); - } - return (serialized[0] << 24) - | (serialized[1] << 16) - | (serialized[2] << 8) - | serialized[3]; - } - - @Override - public Integer parseAscii(String ascii) { - return Integer.valueOf(ascii); + public Integer parseAsciiString(String serialized) { + return Integer.parseInt(serialized); } }; @@ -138,17 +114,6 @@ public abstract class Metadata { this.serializable = false; } - /** - * Constructor called by the transport layer when it receives ASCII metadata. - */ - private Metadata(String... asciiValues) { - store = LinkedListMultimap.create(); - for (int i = 0; i < asciiValues.length; i++) { - store.put(asciiValues[i], new MetadataEntry(asciiValues[++i])); - } - this.serializable = false; - } - /** * Constructor called by the application layer when it wants to send metadata. */ @@ -227,7 +192,13 @@ public abstract class Metadata { } /** - * Serialize all the metadata entries + * Serialize all the metadata entries. + * + *

It produces serialized names and values interleaved. result[i*2] are names, while + * result[i*2+1] are values. + * + *

Names are ASCII string bytes. If the name ends with "-bin", the value can be raw binary. + * Otherwise, the value must be printable ASCII characters or space. */ public byte[][] serialize() { Preconditions.checkState(serializable, "Can't serialize raw metadata"); @@ -240,20 +211,6 @@ public abstract class Metadata { return serialized; } - /** - * Serialize all the metadata entries - */ - public String[] serializeAscii() { - Preconditions.checkState(serializable, "Can't serialize received metadata"); - String[] serialized = new String[store.size() * 2]; - int i = 0; - for (Map.Entry entry : store.entries()) { - serialized[i++] = entry.getValue().key.name(); - serialized[i++] = entry.getValue().getSerializedAscii(); - } - return serialized; - } - /** * Perform a simple merge of two sets of metadata. *

@@ -301,20 +258,6 @@ public abstract class Metadata { super(headers); } - /** - * Called by the transport layer to create headers from their ASCII serialized values. - */ - public Headers(String... asciiValues) { - super(asciiValues); - } - - /** - * Called by the transport layer to create headers from their ASCII serialized values. - */ - public Headers(Iterable> mapEntries) { - super(Iterators.toArray(fromMapEntries(mapEntries), String.class)); - } - /** * Called by the application layer to construct headers prior to passing them to the * transport for serialization. @@ -377,20 +320,6 @@ public abstract class Metadata { super(headers); } - /** - * Called by the transport layer to create trailers from their ASCII serialized values. - */ - public Trailers(String... asciiValues) { - super(asciiValues); - } - - /** - * Called by the transport layer to create headers from their ASCII serialized values. - */ - public Trailers(Iterable> mapEntries) { - super(Iterators.toArray(fromMapEntries(mapEntries), String.class)); - } - /** * Called by the application layer to construct trailers prior to passing them to the * transport for serialization. @@ -401,57 +330,75 @@ public abstract class Metadata { /** - * Marshaller for metadata values. + * Marshaller for metadata values that are serialized into raw binary. */ - public static interface Marshaller { + public static interface BinaryMarshaller { /** * Serialize a metadata value to bytes. * @param value to serialize - * @return serialized version of value, or null if value cannot be transmitted. + * @return serialized version of value */ public byte[] toBytes(T value); - /** - * Serialize a metadata value to an ASCII string - * @param value to serialize - * @return serialized ascii version of value, or null if value cannot be transmitted. - */ - public String toAscii(T value); - /** * Parse a serialized metadata value from bytes. * @param serialized value of metadata to parse * @return a parsed instance of type T */ public T parseBytes(byte[] serialized); + } + + /** + * Marshaller for metadata values that are serialized into ASCII strings that contain only + * printable characters and space. + */ + public static interface AsciiMarshaller { + /** + * Serialize a metadata value to a ASCII string that contains only printable characters and + * space. + * + * @param value to serialize + * @return serialized version of value, or null if value cannot be transmitted. + */ + public String toAsciiString(T value); /** - * Parse a serialized metadata value from an ascii string. - * @param ascii string value of metadata to parse + * Parse a serialized metadata value from an ASCII string. + * @param serialized value of metadata to parse * @return a parsed instance of type T */ - public T parseAscii(String ascii); + public T parseAsciiString(String serialized); } /** * Key for metadata entries. Allows for parsing and serialization of metadata. */ - public static class Key { - public static Key of(String name, Marshaller marshaller) { - return new Key(name, marshaller); + public abstract static class Key { + + /** + * Creates a key for a binary header. + * + * @param name must end with {@link BINARY_HEADER_SUFFIX} + */ + public static Key of(String name, BinaryMarshaller marshaller) { + return new BinaryKey(name, marshaller); + } + + /** + * Creates a key for a ASCII header. + * + * @param name must not end with {@link BINARY_HEADER_SUFFIX} + */ + public static Key of(String name, AsciiMarshaller marshaller) { + return new AsciiKey(name, marshaller); } private final String name; private final byte[] asciiName; - private final Marshaller marshaller; - /** - * Keys have a name and a marshaller used for serialization. - */ - private Key(String name, Marshaller marshaller) { + private Key(String name) { this.name = Preconditions.checkNotNull(name, "name").toLowerCase().intern(); this.asciiName = this.name.getBytes(US_ASCII); - this.marshaller = Preconditions.checkNotNull(marshaller); } public String name() { @@ -463,10 +410,6 @@ public abstract class Metadata { return asciiName; } - public Marshaller getMarshaller() { - return marshaller; - } - @Override public boolean equals(Object o) { if (this == o) return true; @@ -484,6 +427,68 @@ public abstract class Metadata { public String toString() { return "Key{name='" + name + "'}"; } + + /** + * Serialize a metadata value to bytes. + * @param value to serialize + * @return serialized version of value + */ + abstract byte[] toBytes(T value); + + /** + * Parse a serialized metadata value from bytes. + * @param serialized value of metadata to parse + * @return a parsed instance of type T + */ + abstract T parseBytes(byte[] serialized); + } + + private static class BinaryKey extends Key { + private final BinaryMarshaller marshaller; + + /** + * Keys have a name and a binary marshaller used for serialization. + */ + private BinaryKey(String name, BinaryMarshaller marshaller) { + super(name); + Preconditions.checkArgument(name.endsWith(BINARY_HEADER_SUFFIX), + "Binary header is named " + name + ". It must end with " + BINARY_HEADER_SUFFIX); + this.marshaller = Preconditions.checkNotNull(marshaller); + } + + @Override + byte[] toBytes(T value) { + return marshaller.toBytes(value); + } + + @Override + T parseBytes(byte[] serialized) { + return marshaller.parseBytes(serialized); + } + } + + private static class AsciiKey extends Key { + private final AsciiMarshaller marshaller; + + /** + * Keys have a name and an ASCII marshaller used for serialization. + */ + private AsciiKey(String name, AsciiMarshaller marshaller) { + super(name); + Preconditions.checkArgument(!name.endsWith(BINARY_HEADER_SUFFIX), + "ASCII header is named " + name + ". It must not end with " + BINARY_HEADER_SUFFIX); + this.marshaller = Preconditions.checkNotNull(marshaller); + } + + @Override + byte[] toBytes(T value) { + return marshaller.toAsciiString(value).getBytes(US_ASCII); + } + + @Override + T parseBytes(byte[] serialized) { + return marshaller.parseAsciiString(new String(serialized, US_ASCII)); + } } private static class MetadataEntry { @@ -492,7 +497,6 @@ public abstract class Metadata { @SuppressWarnings("rawtypes") Key key; byte[] serializedBinary; - String serializedAscii; /** * Constructor used when application layer adds a parsed value. @@ -510,29 +514,20 @@ public abstract class Metadata { this.serializedBinary = serialized; } - /** - * Constructor used when reading a value from the transport. - */ - private MetadataEntry(String serializedAscii) { - this.serializedAscii = Preconditions.checkNotNull(serializedAscii); - } - @SuppressWarnings("unchecked") public T getParsed(Key key) { T value = (T) parsed; if (value != null) { if (this.key != key) { // Keys don't match so serialize using the old key - serializedBinary = this.key.getMarshaller().toBytes(value); + serializedBinary = this.key.toBytes(value); } else { return value; } } this.key = key; if (serializedBinary != null) { - value = key.getMarshaller().parseBytes(serializedBinary); - } else if (serializedAscii != null) { - value = key.getMarshaller().parseAscii(serializedAscii); + value = key.parseBytes(serializedBinary); } parsed = value; return value; @@ -542,16 +537,7 @@ public abstract class Metadata { public byte[] getSerialized() { return serializedBinary = serializedBinary == null - ? key.getMarshaller().toBytes(parsed) : - serializedBinary; - } - - @SuppressWarnings("unchecked") - public String getSerializedAscii() { - return serializedAscii = - serializedAscii == null - ? key.getMarshaller().toAscii(parsed) : - serializedAscii; + ? key.toBytes(parsed) : serializedBinary; } } } diff --git a/core/src/main/java/com/google/net/stubby/Status.java b/core/src/main/java/com/google/net/stubby/Status.java index fe5318ecb3..4209938362 100644 --- a/core/src/main/java/com/google/net/stubby/Status.java +++ b/core/src/main/java/com/google/net/stubby/Status.java @@ -1,7 +1,5 @@ package com.google.net.stubby; -import static com.google.common.base.Charsets.US_ASCII; - import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.base.Preconditions; @@ -210,7 +208,7 @@ public final class Status { * Key to bind status message to trailers. */ public static final Metadata.Key MESSAGE_KEY - = Metadata.Key.of("grpc-message", Metadata.STRING_MARSHALLER); + = Metadata.Key.of("grpc-message", Metadata.ASCII_STRING_MARSHALLER); /** * Extract an error {@link Status} from the causal chain of a {@link Throwable}. @@ -354,25 +352,15 @@ public final class Status { .toString(); } - private static class StatusCodeMarshaller implements Metadata.Marshaller { + private static class StatusCodeMarshaller implements Metadata.AsciiMarshaller { @Override - public byte[] toBytes(Status status) { - return toAscii(status).getBytes(US_ASCII); - } - - @Override - public String toAscii(Status status) { + public String toAsciiString(Status status) { return status.getCode().valueAscii(); } @Override - public Status parseBytes(byte[] serialized) { - return parseAscii(new String(serialized, US_ASCII)); - } - - @Override - public Status parseAscii(String ascii) { - return fromCodeValue(Integer.valueOf(ascii)); + public Status parseAsciiString(String serialized) { + return fromCodeValue(Integer.valueOf(serialized)); } } } diff --git a/core/src/main/java/com/google/net/stubby/proto/ProtoUtils.java b/core/src/main/java/com/google/net/stubby/proto/ProtoUtils.java index ae19766c8a..892c0f4865 100644 --- a/core/src/main/java/com/google/net/stubby/proto/ProtoUtils.java +++ b/core/src/main/java/com/google/net/stubby/proto/ProtoUtils.java @@ -1,6 +1,5 @@ package com.google.net.stubby.proto; -import com.google.common.io.BaseEncoding; import com.google.net.stubby.Marshaller; import com.google.net.stubby.Metadata; import com.google.net.stubby.Status; @@ -39,18 +38,14 @@ public class ProtoUtils { * Produce a metadata key for a generated protobuf type. */ public static Metadata.Key keyForProto(final T instance) { - return Metadata.Key.of(instance.getDescriptorForType().getFullName(), - new Metadata.Marshaller() { + return Metadata.Key.of( + instance.getDescriptorForType().getFullName() + Metadata.BINARY_HEADER_SUFFIX, + new Metadata.BinaryMarshaller() { @Override public byte[] toBytes(T value) { return value.toByteArray(); } - @Override - public String toAscii(T value) { - return BaseEncoding.base64().encode(value.toByteArray()); - } - @Override @SuppressWarnings("unchecked") public T parseBytes(byte[] serialized) { @@ -60,11 +55,6 @@ public class ProtoUtils { throw new IllegalArgumentException(ipbe); } } - - @Override - public T parseAscii(String ascii) { - return parseBytes(BaseEncoding.base64().decode(ascii)); - } }); } diff --git a/core/src/main/java/com/google/net/stubby/transport/Http2ClientStream.java b/core/src/main/java/com/google/net/stubby/transport/Http2ClientStream.java index f4b9c1a71d..38bf876120 100644 --- a/core/src/main/java/com/google/net/stubby/transport/Http2ClientStream.java +++ b/core/src/main/java/com/google/net/stubby/transport/Http2ClientStream.java @@ -19,26 +19,16 @@ public abstract class Http2ClientStream extends AbstractClientStream { /** * Metadata marshaller for HTTP status lines. */ - private static final Metadata.Marshaller HTTP_STATUS_LINE_MARSHALLER = - new Metadata.Marshaller() { + private static final Metadata.AsciiMarshaller HTTP_STATUS_LINE_MARSHALLER = + new Metadata.AsciiMarshaller() { @Override - public byte[] toBytes(Integer value) { - return value.toString().getBytes(Charsets.US_ASCII); - } - - @Override - public String toAscii(Integer value) { + public String toAsciiString(Integer value) { return value.toString(); } @Override - public Integer parseBytes(byte[] serialized) { - return parseAscii(new String(serialized, Charsets.US_ASCII)); - } - - @Override - public Integer parseAscii(String ascii) { - return Integer.parseInt(ascii.split(" ", 2)[0]); + public Integer parseAsciiString(String serialized) { + return Integer.parseInt(serialized.split(" ", 2)[0]); } }; diff --git a/core/src/main/java/com/google/net/stubby/transport/HttpUtil.java b/core/src/main/java/com/google/net/stubby/transport/HttpUtil.java index cced608c41..49dc6fccb3 100644 --- a/core/src/main/java/com/google/net/stubby/transport/HttpUtil.java +++ b/core/src/main/java/com/google/net/stubby/transport/HttpUtil.java @@ -14,7 +14,7 @@ public final class HttpUtil { * spec. */ public static final Metadata.Key CONTENT_TYPE = - Metadata.Key.of("content-type", Metadata.STRING_MARSHALLER); + Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER); /** * Content-Type used for GRPC-over-HTTP/2. @@ -29,7 +29,8 @@ public final class HttpUtil { /** * The TE header name. Defined here since it is not explicitly defined by the HTTP/2 spec. */ - public static final Metadata.Key TE = Metadata.Key.of("te", Metadata.STRING_MARSHALLER); + public static final Metadata.Key TE = Metadata.Key.of("te", + Metadata.ASCII_STRING_MARSHALLER); /** * The TE (transport encoding) header for requests over HTTP/2 diff --git a/core/src/main/java/com/google/net/stubby/transport/TransportFrameUtil.java b/core/src/main/java/com/google/net/stubby/transport/TransportFrameUtil.java index 021bee8402..a11f6bfba8 100644 --- a/core/src/main/java/com/google/net/stubby/transport/TransportFrameUtil.java +++ b/core/src/main/java/com/google/net/stubby/transport/TransportFrameUtil.java @@ -1,5 +1,14 @@ package com.google.net.stubby.transport; +import static com.google.common.base.Charsets.US_ASCII; + +import com.google.common.io.BaseEncoding; +import com.google.net.stubby.Metadata; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.logging.Logger; + import javax.annotation.Nullable; /** @@ -10,6 +19,12 @@ import javax.annotation.Nullable; */ public final class TransportFrameUtil { + private static final Logger logger = Logger.getLogger(TransportFrameUtil.class.getName()); + + private static final byte[] binaryHeaderSuffixBytes = + Metadata.BINARY_HEADER_SUFFIX.getBytes(US_ASCII); + + // Compression modes (lowest order 3 bits of frame flags) public static final byte NO_COMPRESS_FLAG = 0x0; public static final byte FLATE_FLAG = 0x1; @@ -57,5 +72,88 @@ public final class TransportFrameUtil { return path; } + /** + * Transform the given headers to a format where only spec-compliant ASCII characters are allowed. + * Binary header values are encoded by Base64 in the result. + * + * @return the interleaved keys and values. + */ + public static byte[][] toHttp2Headers(Metadata headers) { + byte[][] serializedHeaders = headers.serialize(); + ArrayList result = new ArrayList(); + for (int i = 0; i < serializedHeaders.length; i += 2) { + byte[] key = serializedHeaders[i]; + byte[] value = serializedHeaders[i + 1]; + if (endsWith(key, binaryHeaderSuffixBytes)) { + // Binary header. + result.add(key); + result.add(BaseEncoding.base64().encode(value).getBytes(US_ASCII)); + } else { + // Non-binary header. + // Filter out headers that contain non-spec-compliant ASCII characters. + // TODO(user): only do such check in development mode since it's expensive + if (isSpecCompliantAscii(value)) { + result.add(key); + result.add(value); + } else { + String keyString = new String(key, US_ASCII); + logger.warning("Metadata key=" + keyString + ", value=" + Arrays.toString(value) + + " contains invalid ASCII characters"); + } + } + } + return result.toArray(new byte[result.size()][]); + } + + /** + * Transform HTTP/2-compliant headers to the raw serialized format which can be deserialized by + * metadata marshallers. It decodes the Base64-encoded binary headers. + */ + public static byte[][] toRawSerializedHeaders(byte[][] http2Headers) { + byte[][] result = new byte[http2Headers.length][]; + for (int i = 0; i < http2Headers.length; i += 2) { + byte[] key = http2Headers[i]; + byte[] value = http2Headers[i + 1]; + result[i] = key; + if (endsWith(key, binaryHeaderSuffixBytes)) { + // Binary header + result[i + 1] = BaseEncoding.base64().decode(new String(value, US_ASCII)); + } else { + // Non-binary header + result[i + 1] = value; + } + } + return result; + } + + /** + * Returns true if subject ends with suffix. + */ + private static boolean endsWith(byte[] subject, byte[] suffix) { + int start = subject.length - suffix.length; + if (start < 0) { + return false; + } + for (int i = start; i < subject.length; i++) { + if (subject[i] != suffix[i - start]) { + return false; + } + } + return true; + } + + /** + * Returns true if subject contains only bytes that are spec-compliant ASCII characters and + * space. + */ + private static boolean isSpecCompliantAscii(byte[] subject) { + for (byte b : subject) { + if (b < 32 || b > 126) { + return false; + } + } + return true; + } + private TransportFrameUtil() {} } diff --git a/core/src/test/java/com/google/net/stubby/ClientInterceptorsTest.java b/core/src/test/java/com/google/net/stubby/ClientInterceptorsTest.java index 990f365fe0..63f3442230 100644 --- a/core/src/test/java/com/google/net/stubby/ClientInterceptorsTest.java +++ b/core/src/test/java/com/google/net/stubby/ClientInterceptorsTest.java @@ -128,7 +128,7 @@ public class ClientInterceptorsTest { @Test public void addOutboundHeaders() { - final Metadata.Key credKey = Metadata.Key.of("Cred", Metadata.STRING_MARSHALLER); + final Metadata.Key credKey = Metadata.Key.of("Cred", Metadata.ASCII_STRING_MARSHALLER); ClientInterceptor interceptor = new ClientInterceptor() { @Override public Call interceptCall(MethodDescriptor method, diff --git a/core/src/test/java/com/google/net/stubby/MetadataTest.java b/core/src/test/java/com/google/net/stubby/MetadataTest.java index 158f03c62e..566cd267b5 100644 --- a/core/src/test/java/com/google/net/stubby/MetadataTest.java +++ b/core/src/test/java/com/google/net/stubby/MetadataTest.java @@ -7,8 +7,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import com.google.common.primitives.Bytes; - import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -22,31 +20,22 @@ import java.util.Iterator; @RunWith(JUnit4.class) public class MetadataTest { - private static final Metadata.Marshaller FISH_MARSHALLER = new Metadata.Marshaller() { + private static final Metadata.BinaryMarshaller FISH_MARSHALLER = + new Metadata.BinaryMarshaller() { @Override public byte[] toBytes(Fish fish) { return fish.name.getBytes(UTF_8); } - @Override - public String toAscii(Fish value) { - return value.name; - } - @Override public Fish parseBytes(byte[] serialized) { return new Fish(new String(serialized, UTF_8)); } - - @Override - public Fish parseAscii(String ascii) { - return new Fish(ascii); - } }; private static final String LANCE = "lance"; private static final byte[] LANCE_BYTES = LANCE.getBytes(StandardCharsets.US_ASCII); - private static final Metadata.Key KEY = Metadata.Key.of("test", FISH_MARSHALLER); + private static final Metadata.Key KEY = Metadata.Key.of("test-bin", FISH_MARSHALLER); @Test public void testWriteParsed() { @@ -61,7 +50,7 @@ public class MetadataTest { assertFalse(fishes.hasNext()); byte[][] serialized = metadata.serialize(); assertEquals(2, serialized.length); - assertEquals(new String(serialized[0], StandardCharsets.US_ASCII), "test"); + assertEquals(new String(serialized[0], StandardCharsets.US_ASCII), "test-bin"); assertArrayEquals(LANCE_BYTES, serialized[1]); assertSame(lance, metadata.get(KEY)); // Serialized instance should be cached too @@ -112,14 +101,8 @@ public class MetadataTest { } @Test - public void integerMarshallerBytesIsBigEndian() { - assertEquals(Bytes.asList(new byte[] {0x12, 0x34, 0x56, 0x78}), - Bytes.asList(Metadata.INTEGER_MARSHALLER.toBytes(0x12345678))); - } - - @Test - public void integerMarshallerAsciiIsDecimal() { - assertEquals("12345678", Metadata.INTEGER_MARSHALLER.toAscii(12345678)); + public void integerMarshallerIsDecimal() { + assertEquals("12345678", Metadata.INTEGER_MARSHALLER.toAsciiString(12345678)); } @Test @@ -132,8 +115,8 @@ public class MetadataTest { } private void roundTripInteger(Integer i) { - assertEquals(i, Metadata.INTEGER_MARSHALLER.parseBytes(Metadata.INTEGER_MARSHALLER.toBytes(i))); - assertEquals(i, Metadata.INTEGER_MARSHALLER.parseAscii(Metadata.INTEGER_MARSHALLER.toAscii(i))); + assertEquals(i, Metadata.INTEGER_MARSHALLER.parseAsciiString( + Metadata.INTEGER_MARSHALLER.toAsciiString(i))); } private static class Fish { diff --git a/core/src/test/java/com/google/net/stubby/transport/TransportFrameUtilTest.java b/core/src/test/java/com/google/net/stubby/transport/TransportFrameUtilTest.java new file mode 100644 index 0000000000..56a1f7e96c --- /dev/null +++ b/core/src/test/java/com/google/net/stubby/transport/TransportFrameUtilTest.java @@ -0,0 +1,102 @@ +package com.google.net.stubby.transport; + +import static com.google.common.base.Charsets.US_ASCII; +import static com.google.common.base.Charsets.UTF_8; +import static com.google.net.stubby.Metadata.ASCII_STRING_MARSHALLER; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; + +import com.google.common.io.BaseEncoding; +import com.google.net.stubby.Metadata.BinaryMarshaller; +import com.google.net.stubby.Metadata.Headers; +import com.google.net.stubby.Metadata.Key; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +import java.util.Arrays; + +/** Unit tests for {@link TransportFrameUtil}. */ +@RunWith(JUnit4.class) +public class TransportFrameUtilTest { + + private static final String NONCOMPLIANT_ASCII_STRING = new String(new char[]{1, 2, 3}); + + private static final String COMPLIANT_ASCII_STRING = "Kyle"; + + private static final BinaryMarshaller UTF8_STRING_MARSHALLER = + new BinaryMarshaller() { + @Override + public byte[] toBytes(String value) { + return value.getBytes(UTF_8); + } + + @Override + public String parseBytes(byte[] serialized) { + return new String(serialized, UTF_8); + } + }; + + private static final Key PLAIN_STRING = Key.of("plainstring", ASCII_STRING_MARSHALLER); + private static final Key BINARY_STRING = Key.of("string-bin", UTF8_STRING_MARSHALLER); + private static final Key BINARY_STRING_WITHOUT_SUFFIX = + Key.of("string", ASCII_STRING_MARSHALLER); + + @Test + public void testToHttp2Headers() { + Headers headers = new Headers(); + headers.put(PLAIN_STRING, COMPLIANT_ASCII_STRING); + headers.put(BINARY_STRING, NONCOMPLIANT_ASCII_STRING); + headers.put(BINARY_STRING_WITHOUT_SUFFIX, NONCOMPLIANT_ASCII_STRING); + byte[][] http2Headers = TransportFrameUtil.toHttp2Headers(headers); + // BINARY_STRING_WITHOUT_SUFFIX should not get in because it contains non-compliant ASCII + // characters but doesn't have "-bin" in the name. + byte[][] answer = new byte[][] { + "plainstring".getBytes(US_ASCII), COMPLIANT_ASCII_STRING.getBytes(US_ASCII), + "string-bin".getBytes(US_ASCII), + base64Encode(NONCOMPLIANT_ASCII_STRING.getBytes(US_ASCII))}; + assertEquals(answer.length, http2Headers.length); + // http2Headers may re-sort the keys, so we cannot compare it with the answer side-by-side. + for (int i = 0; i < answer.length; i += 2) { + assertContains(http2Headers, answer[i], answer[i + 1]); + } + } + + @Test(expected = IllegalArgumentException.class) + public void binaryHeaderWithoutSuffix() { + Key.of("plainstring", UTF8_STRING_MARSHALLER); + } + + @Test + public void testToAndFromHttp2Headers() { + Headers headers = new Headers(); + headers.put(PLAIN_STRING, COMPLIANT_ASCII_STRING); + headers.put(BINARY_STRING, NONCOMPLIANT_ASCII_STRING); + headers.put(BINARY_STRING_WITHOUT_SUFFIX, NONCOMPLIANT_ASCII_STRING); + byte[][] http2Headers = TransportFrameUtil.toHttp2Headers(headers); + byte[][] rawSerialized = TransportFrameUtil.toRawSerializedHeaders(http2Headers); + Headers recoveredHeaders = new Headers(rawSerialized); + assertEquals(COMPLIANT_ASCII_STRING, recoveredHeaders.get(PLAIN_STRING)); + assertEquals(NONCOMPLIANT_ASCII_STRING, recoveredHeaders.get(BINARY_STRING)); + assertNull(recoveredHeaders.get(BINARY_STRING_WITHOUT_SUFFIX)); + } + + private static void assertContains(byte[][] headers, byte[] key, byte[] value) { + String keyString = new String(key, US_ASCII); + for (int i = 0; i < headers.length; i += 2) { + if (Arrays.equals(headers[i], key)) { + assertArrayEquals("value for key=" + keyString, value, headers[i + 1]); + return; + } + } + fail("key=" + keyString + " not found"); + } + + private static byte[] base64Encode(byte[] input) { + return BaseEncoding.base64().encode(input).getBytes(US_ASCII); + } + +} diff --git a/netty/src/main/java/com/google/net/stubby/transport/netty/Utils.java b/netty/src/main/java/com/google/net/stubby/transport/netty/Utils.java index cee390d478..46289ca43c 100644 --- a/netty/src/main/java/com/google/net/stubby/transport/netty/Utils.java +++ b/netty/src/main/java/com/google/net/stubby/transport/netty/Utils.java @@ -5,6 +5,7 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.google.net.stubby.Metadata; import com.google.net.stubby.SharedResourceHolder.Resource; import com.google.net.stubby.transport.HttpUtil; +import com.google.net.stubby.transport.TransportFrameUtil; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufAllocator; @@ -78,7 +79,7 @@ class Utils { headerValues[i++] = entry.getKey().array(); headerValues[i++] = entry.getValue().array(); } - return headerValues; + return TransportFrameUtil.toRawSerializedHeaders(headerValues); } public static Http2Headers convertClientHeaders(Metadata.Headers headers, @@ -128,10 +129,10 @@ class Utils { private static Http2Headers convertMetadata(Metadata headers) { Preconditions.checkNotNull(headers, "headers"); Http2Headers http2Headers = new DefaultHttp2Headers(); - byte[][] serializedHeaders = headers.serialize(); - for (int i = 0; i < serializedHeaders.length; i++) { + byte[][] serializedHeaders = TransportFrameUtil.toHttp2Headers(headers); + for (int i = 0; i < serializedHeaders.length; i += 2) { http2Headers.add(new AsciiString(serializedHeaders[i], false), - new AsciiString(serializedHeaders[++i], false)); + new AsciiString(serializedHeaders[i + 1], false)); } return http2Headers; } diff --git a/okhttp/src/main/java/com/google/net/stubby/transport/okhttp/Headers.java b/okhttp/src/main/java/com/google/net/stubby/transport/okhttp/Headers.java index c5a019fd96..9474fb805e 100644 --- a/okhttp/src/main/java/com/google/net/stubby/transport/okhttp/Headers.java +++ b/okhttp/src/main/java/com/google/net/stubby/transport/okhttp/Headers.java @@ -4,6 +4,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.net.stubby.Metadata; import com.google.net.stubby.transport.HttpUtil; +import com.google.net.stubby.transport.TransportFrameUtil; import com.squareup.okhttp.internal.spdy.Header; @@ -48,10 +49,10 @@ public class Headers { okhttpHeaders.add(TE_HEADER); // Now add any application-provided headers. - byte[][] serializedHeaders = headers.serialize(); - for (int i = 0; i < serializedHeaders.length; i++) { + byte[][] serializedHeaders = TransportFrameUtil.toHttp2Headers(headers); + for (int i = 0; i < serializedHeaders.length; i += 2) { ByteString key = ByteString.of(serializedHeaders[i]); - ByteString value = ByteString.of(serializedHeaders[++i]); + ByteString value = ByteString.of(serializedHeaders[i + 1]); if (isApplicationHeader(key)) { okhttpHeaders.add(new Header(key, value)); } diff --git a/okhttp/src/main/java/com/google/net/stubby/transport/okhttp/Utils.java b/okhttp/src/main/java/com/google/net/stubby/transport/okhttp/Utils.java index 6a42970b8d..f734390988 100644 --- a/okhttp/src/main/java/com/google/net/stubby/transport/okhttp/Utils.java +++ b/okhttp/src/main/java/com/google/net/stubby/transport/okhttp/Utils.java @@ -1,6 +1,7 @@ package com.google.net.stubby.transport.okhttp; import com.google.net.stubby.Metadata; +import com.google.net.stubby.transport.TransportFrameUtil; import com.squareup.okhttp.internal.spdy.Header; @@ -29,7 +30,7 @@ class Utils { headerValues[i++] = header.name.toByteArray(); headerValues[i++] = header.value.toByteArray(); } - return headerValues; + return TransportFrameUtil.toRawSerializedHeaders(headerValues); } private Utils() {