Skip to content

Commit e60a179

Browse files
carl-mastrangelocarl-mastrangelo
authored
core: hide access to pseudo headers
Previously anyone could create metadata keys with a leading ":", due to not having a way to prevent it. This change effectively only allows internal code to make use of this.
1 parent 5713ebc commit e60a179

File tree

5 files changed

+57
-33
lines changed

5 files changed

+57
-33
lines changed

‎core/src/main/java/io/grpc/InternalMetadata.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package io.grpc;
1818

19+
import io.grpc.Metadata.AsciiMarshaller;
1920
import io.grpc.Metadata.Key;
2021
import java.nio.charset.Charset;
2122

@@ -44,7 +45,14 @@ public interface TrustedAsciiMarshaller<T> extends Metadata.TrustedAsciiMarshall
4445

4546
@Internal
4647
public static <T> Key<T> keyOf(String name, TrustedAsciiMarshaller<T> marshaller) {
47-
return Metadata.Key.of(name, marshaller);
48+
boolean isPseudo = name != null && !name.isEmpty() && name.charAt(0) == ':';
49+
return Metadata.Key.of(name, isPseudo, marshaller);
50+
}
51+
52+
@Internal
53+
public static <T> Key<T> keyOf(String name, AsciiMarshaller<T> marshaller) {
54+
boolean isPseudo = name != null && !name.isEmpty() && name.charAt(0) == ':';
55+
return Metadata.Key.of(name, isPseudo, marshaller);
4856
}
4957

5058
@Internal

‎core/src/main/java/io/grpc/Metadata.java‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -599,11 +599,15 @@ public static <T> Key<T> of(String name, BinaryMarshaller<T> marshaller) {
599599
* <b>not</b> end with {@link #BINARY_HEADER_SUFFIX}
600600
*/
601601
public static <T> Key<T> of(String name, AsciiMarshaller<T> marshaller) {
602-
return new AsciiKey<T>(name, marshaller);
602+
return of(name, false, marshaller);
603603
}
604604

605-
static <T> Key<T> of(String name, TrustedAsciiMarshaller<T> marshaller) {
606-
return new TrustedAsciiKey<T>(name, marshaller);
605+
static <T> Key<T> of(String name, boolean pseudo, AsciiMarshaller<T> marshaller) {
606+
return new AsciiKey<T>(name, pseudo, marshaller);
607+
}
608+
609+
static <T> Key<T> of(String name, boolean pseudo, TrustedAsciiMarshaller<T> marshaller) {
610+
return new TrustedAsciiKey<T>(name, pseudo, marshaller);
607611
}
608612

609613
private final String originalName;
@@ -626,13 +630,12 @@ private static BitSet generateValidTChars() {
626630
return valid;
627631
}
628632

629-
private static String validateName(String n) {
633+
private static String validateName(String n, boolean pseudo) {
630634
checkNotNull(n, "name");
631-
checkArgument(n.length() != 0, "token must have at least 1 tchar");
635+
checkArgument(!n.isEmpty(), "token must have at least 1 tchar");
632636
for (int i = 0; i < n.length(); i++) {
633637
char tChar = n.charAt(i);
634-
// TODO(notcarl): remove this hack once pseudo headers are properly handled
635-
if (tChar == ':' && i == 0) {
638+
if (pseudo && tChar == ':' && i == 0) {
636639
continue;
637640
}
638641

@@ -642,9 +645,9 @@ private static String validateName(String n) {
642645
return n;
643646
}
644647

645-
private Key(String name) {
648+
private Key(String name, boolean pseudo) {
646649
this.originalName = checkNotNull(name, "name");
647-
this.name = validateName(this.originalName.toLowerCase(Locale.ROOT));
650+
this.name = validateName(this.originalName.toLowerCase(Locale.ROOT), pseudo);
648651
this.nameBytes = this.name.getBytes(US_ASCII);
649652
}
650653

@@ -722,7 +725,7 @@ private static class BinaryKey<T> extends Key<T> {
722725

723726
/** Keys have a name and a binary marshaller used for serialization. */
724727
private BinaryKey(String name, BinaryMarshaller<T> marshaller) {
725-
super(name);
728+
super(name, false /* not pseudo */);
726729
checkArgument(
727730
name.endsWith(BINARY_HEADER_SUFFIX),
728731
"Binary header is named %s. It must end with %s",
@@ -747,8 +750,8 @@ private static class AsciiKey<T> extends Key<T> {
747750
private final AsciiMarshaller<T> marshaller;
748751

749752
/** Keys have a name and an ASCII marshaller used for serialization. */
750-
private AsciiKey(String name, AsciiMarshaller<T> marshaller) {
751-
super(name);
753+
private AsciiKey(String name, boolean pseudo, AsciiMarshaller<T> marshaller) {
754+
super(name, pseudo);
752755
Preconditions.checkArgument(
753756
!name.endsWith(BINARY_HEADER_SUFFIX),
754757
"ASCII header is named %s. Only binary headers may end with %s",
@@ -772,8 +775,8 @@ private static final class TrustedAsciiKey<T> extends Key<T> {
772775
private final TrustedAsciiMarshaller<T> marshaller;
773776

774777
/** Keys have a name and an ASCII marshaller used for serialization. */
775-
private TrustedAsciiKey(String name, TrustedAsciiMarshaller<T> marshaller) {
776-
super(name);
778+
private TrustedAsciiKey(String name, boolean pseudo, TrustedAsciiMarshaller<T> marshaller) {
779+
super(name, pseudo);
777780
Preconditions.checkArgument(
778781
!name.endsWith(BINARY_HEADER_SUFFIX),
779782
"ASCII header is named %s. Only binary headers may end with %s",

‎core/src/main/java/io/grpc/Status.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ public static Status fromCode(Code code) {
346346
* Key to bind status code to trailing metadata.
347347
*/
348348
static final Metadata.Key<Status> CODE_KEY
349-
= Metadata.Key.of("grpc-status", new StatusCodeMarshaller());
349+
= Metadata.Key.of("grpc-status", false /* not pseudo */, new StatusCodeMarshaller());
350350

351351
/**
352352
* Marshals status messages for ({@link #MESSAGE_KEY}. gRPC does not use binary coding of
@@ -377,7 +377,7 @@ public static Status fromCode(Code code) {
377377
* Key to bind status message to trailing metadata.
378378
*/
379379
static final Metadata.Key<String> MESSAGE_KEY =
380-
Metadata.Key.of("grpc-message", STATUS_MESSAGE_MARSHALLER);
380+
Metadata.Key.of("grpc-message", false /* not pseudo */, STATUS_MESSAGE_MARSHALLER);
381381

382382
/**
383383
* Extract an error {@link Status} from the causal chain of a {@link Throwable}.

‎core/src/test/java/io/grpc/MetadataTest.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ public Fish parseBytes(byte[] serialized) {
6363
private static final byte[] LANCE_BYTES = LANCE.getBytes(US_ASCII);
6464
private static final Metadata.Key<Fish> KEY = Metadata.Key.of("test-bin", FISH_MARSHALLER);
6565

66+
@Test
67+
public void noPseudoHeaders() {
68+
thrown.expect(IllegalArgumentException.class);
69+
thrown.expectMessage("Invalid character");
70+
71+
Metadata.Key.of(":test-bin", FISH_MARSHALLER);
72+
}
73+
6674
@Test
6775
public void testMutations() {
6876
Fish lance = new Fish(LANCE);

‎core/src/test/java/io/grpc/internal/Http2ClientStreamTransportStateTest.java‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import static org.mockito.Mockito.never;
2626
import static org.mockito.Mockito.verify;
2727

28+
import io.grpc.InternalMetadata;
2829
import io.grpc.Metadata;
2930
import io.grpc.Status;
3031
import io.grpc.Status.Code;
@@ -40,6 +41,10 @@
4041
/** Unit tests for {@link Http2ClientStreamTransportState}. */
4142
@RunWith(JUnit4.class)
4243
public class Http2ClientStreamTransportStateTest {
44+
45+
private final Metadata.Key<String> testStatusMashaller =
46+
InternalMetadata.keyOf(":status", Metadata.ASCII_STRING_MARSHALLER);
47+
4348
@Mock private ClientStreamListener mockListener;
4449
@Captor private ArgumentCaptor<Status> statusCaptor;
4550

@@ -53,7 +58,7 @@ public void transportHeadersReceived_notifiesListener() {
5358
BaseTransportState state = new BaseTransportState();
5459
state.setListener(mockListener);
5560
Metadata headers = new Metadata();
56-
headers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "200");
61+
headers.put(testStatusMashaller, "200");
5762
headers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER),
5863
"application/grpc");
5964
state.transportHeadersReceived(headers);
@@ -67,7 +72,7 @@ public void transportHeadersReceived_doesntRequire200() {
6772
BaseTransportState state = new BaseTransportState();
6873
state.setListener(mockListener);
6974
Metadata headers = new Metadata();
70-
headers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "500");
75+
headers.put(testStatusMashaller, "500");
7176
headers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER),
7277
"application/grpc");
7378
state.transportHeadersReceived(headers);
@@ -96,7 +101,7 @@ public void transportHeadersReceived_wrongContentType_200() {
96101
BaseTransportState state = new BaseTransportState();
97102
state.setListener(mockListener);
98103
Metadata headers = new Metadata();
99-
headers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "200");
104+
headers.put(testStatusMashaller, "200");
100105
headers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER), "text/html");
101106
state.transportHeadersReceived(headers);
102107
state.transportDataReceived(ReadableBuffers.empty(), true);
@@ -112,7 +117,7 @@ public void transportHeadersReceived_wrongContentType_401() {
112117
BaseTransportState state = new BaseTransportState();
113118
state.setListener(mockListener);
114119
Metadata headers = new Metadata();
115-
headers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "401");
120+
headers.put(testStatusMashaller, "401");
116121
headers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER), "text/html");
117122
state.transportHeadersReceived(headers);
118123
state.transportDataReceived(ReadableBuffers.empty(), true);
@@ -130,14 +135,14 @@ public void transportHeadersReceived_handles_1xx() {
130135
state.setListener(mockListener);
131136

132137
Metadata infoHeaders = new Metadata();
133-
infoHeaders.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "100");
138+
infoHeaders.put(testStatusMashaller, "100");
134139
state.transportHeadersReceived(infoHeaders);
135140
Metadata infoHeaders2 = new Metadata();
136-
infoHeaders2.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "101");
141+
infoHeaders2.put(testStatusMashaller, "101");
137142
state.transportHeadersReceived(infoHeaders2);
138143

139144
Metadata headers = new Metadata();
140-
headers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "200");
145+
headers.put(testStatusMashaller, "200");
141146
headers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER),
142147
"application/grpc");
143148
state.transportHeadersReceived(headers);
@@ -151,7 +156,7 @@ public void transportHeadersReceived_twice() {
151156
BaseTransportState state = new BaseTransportState();
152157
state.setListener(mockListener);
153158
Metadata headers = new Metadata();
154-
headers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "200");
159+
headers.put(testStatusMashaller, "200");
155160
headers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER),
156161
"application/grpc");
157162
state.transportHeadersReceived(headers);
@@ -170,7 +175,7 @@ public void transportHeadersReceived_unknownAndTwiceLogsSecondHeaders() {
170175
BaseTransportState state = new BaseTransportState();
171176
state.setListener(mockListener);
172177
Metadata headers = new Metadata();
173-
headers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "200");
178+
headers.put(testStatusMashaller, "200");
174179
headers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER), "text/html");
175180
state.transportHeadersReceived(headers);
176181
Metadata headersAgain = new Metadata();
@@ -201,7 +206,7 @@ public void transportDataReceived_debugData() {
201206
BaseTransportState state = new BaseTransportState();
202207
state.setListener(mockListener);
203208
Metadata headers = new Metadata();
204-
headers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "200");
209+
headers.put(testStatusMashaller, "200");
205210
headers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER), "text/html");
206211
state.transportHeadersReceived(headers);
207212
String testString = "This is a test";
@@ -216,7 +221,7 @@ public void transportTrailersReceived_notifiesListener() {
216221
BaseTransportState state = new BaseTransportState();
217222
state.setListener(mockListener);
218223
Metadata trailers = new Metadata();
219-
trailers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "200");
224+
trailers.put(testStatusMashaller, "200");
220225
trailers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER),
221226
"application/grpc");
222227
trailers.put(Metadata.Key.of("grpc-status", Metadata.ASCII_STRING_MARSHALLER), "0");
@@ -231,7 +236,7 @@ public void transportTrailersReceived_afterHeaders() {
231236
BaseTransportState state = new BaseTransportState();
232237
state.setListener(mockListener);
233238
Metadata headers = new Metadata();
234-
headers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "200");
239+
headers.put(testStatusMashaller, "200");
235240
headers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER),
236241
"application/grpc");
237242
state.transportHeadersReceived(headers);
@@ -248,7 +253,7 @@ public void transportTrailersReceived_observesStatus() {
248253
BaseTransportState state = new BaseTransportState();
249254
state.setListener(mockListener);
250255
Metadata trailers = new Metadata();
251-
trailers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "200");
256+
trailers.put(testStatusMashaller, "200");
252257
trailers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER),
253258
"application/grpc");
254259
trailers.put(Metadata.Key.of("grpc-status", Metadata.ASCII_STRING_MARSHALLER), "1");
@@ -263,7 +268,7 @@ public void transportTrailersReceived_missingStatusUsesHttpStatus() {
263268
BaseTransportState state = new BaseTransportState();
264269
state.setListener(mockListener);
265270
Metadata trailers = new Metadata();
266-
trailers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "401");
271+
trailers.put(testStatusMashaller, "401");
267272
trailers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER),
268273
"application/grpc");
269274
state.transportTrailersReceived(trailers);
@@ -308,12 +313,12 @@ public void transportTrailersReceived_missingStatusAfterHeadersIgnoresHttpStatus
308313
BaseTransportState state = new BaseTransportState();
309314
state.setListener(mockListener);
310315
Metadata headers = new Metadata();
311-
headers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "200");
316+
headers.put(testStatusMashaller, "200");
312317
headers.put(Metadata.Key.of("content-type", Metadata.ASCII_STRING_MARSHALLER),
313318
"application/grpc");
314319
state.transportHeadersReceived(headers);
315320
Metadata trailers = new Metadata();
316-
trailers.put(Metadata.Key.of(":status", Metadata.ASCII_STRING_MARSHALLER), "401");
321+
trailers.put(testStatusMashaller, "401");
317322
state.transportTrailersReceived(trailers);
318323

319324
verify(mockListener).headersRead(headers);

0 commit comments

Comments
 (0)