Merge pull request #3021 from ctiller/reject-the-stuffs
Outlaw illegal metadata characters
diff --git a/Makefile b/Makefile
index 48ecea9..6d857c5 100644
--- a/Makefile
+++ b/Makefile
@@ -791,6 +791,7 @@
fling_stream_test: $(BINDIR)/$(CONFIG)/fling_stream_test
fling_test: $(BINDIR)/$(CONFIG)/fling_test
gen_hpack_tables: $(BINDIR)/$(CONFIG)/gen_hpack_tables
+gen_legal_metadata_characters: $(BINDIR)/$(CONFIG)/gen_legal_metadata_characters
gpr_cmdline_test: $(BINDIR)/$(CONFIG)/gpr_cmdline_test
gpr_env_test: $(BINDIR)/$(CONFIG)/gpr_env_test
gpr_file_test: $(BINDIR)/$(CONFIG)/gpr_file_test
@@ -3384,7 +3385,7 @@
tools: tools_c tools_cxx
-tools_c: privatelibs_c $(BINDIR)/$(CONFIG)/gen_hpack_tables $(BINDIR)/$(CONFIG)/grpc_create_jwt $(BINDIR)/$(CONFIG)/grpc_fetch_oauth2 $(BINDIR)/$(CONFIG)/grpc_print_google_default_creds_token $(BINDIR)/$(CONFIG)/grpc_verify_jwt
+tools_c: privatelibs_c $(BINDIR)/$(CONFIG)/gen_hpack_tables $(BINDIR)/$(CONFIG)/gen_legal_metadata_characters $(BINDIR)/$(CONFIG)/grpc_create_jwt $(BINDIR)/$(CONFIG)/grpc_fetch_oauth2 $(BINDIR)/$(CONFIG)/grpc_print_google_default_creds_token $(BINDIR)/$(CONFIG)/grpc_verify_jwt
tools_cxx: privatelibs_cxx
@@ -7120,6 +7121,35 @@
endif
+GEN_LEGAL_METADATA_CHARACTERS_SRC = \
+ tools/codegen/core/gen_legal_metadata_characters.c \
+
+GEN_LEGAL_METADATA_CHARACTERS_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(GEN_LEGAL_METADATA_CHARACTERS_SRC))))
+ifeq ($(NO_SECURE),true)
+
+# You can't build secure targets if you don't have OpenSSL.
+
+$(BINDIR)/$(CONFIG)/gen_legal_metadata_characters: openssl_dep_error
+
+else
+
+$(BINDIR)/$(CONFIG)/gen_legal_metadata_characters: $(GEN_LEGAL_METADATA_CHARACTERS_OBJS)
+ $(E) "[LD] Linking $@"
+ $(Q) mkdir -p `dirname $@`
+ $(Q) $(LD) $(LDFLAGS) $(GEN_LEGAL_METADATA_CHARACTERS_OBJS) $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/gen_legal_metadata_characters
+
+endif
+
+$(OBJDIR)/$(CONFIG)/tools/codegen/core/gen_legal_metadata_characters.o:
+deps_gen_legal_metadata_characters: $(GEN_LEGAL_METADATA_CHARACTERS_OBJS:.o=.dep)
+
+ifneq ($(NO_SECURE),true)
+ifneq ($(NO_DEPS),true)
+-include $(GEN_LEGAL_METADATA_CHARACTERS_OBJS:.o=.dep)
+endif
+endif
+
+
GPR_CMDLINE_TEST_SRC = \
test/core/support/cmdline_test.c \
diff --git a/build.json b/build.json
index 54cae41..1efaafd 100644
--- a/build.json
+++ b/build.json
@@ -1149,6 +1149,15 @@
]
},
{
+ "name": "gen_legal_metadata_characters",
+ "build": "tool",
+ "language": "c",
+ "src": [
+ "tools/codegen/core/gen_legal_metadata_characters.c"
+ ],
+ "deps": []
+ },
+ {
"name": "gpr_cmdline_test",
"build": "test",
"language": "c",
diff --git a/src/core/channel/compress_filter.h b/src/core/channel/compress_filter.h
index 0917e81..415459b 100644
--- a/src/core/channel/compress_filter.h
+++ b/src/core/channel/compress_filter.h
@@ -36,7 +36,7 @@
#include "src/core/channel/channel_stack.h"
-#define GRPC_COMPRESS_REQUEST_ALGORITHM_KEY "internal:grpc-encoding-request"
+#define GRPC_COMPRESS_REQUEST_ALGORITHM_KEY "grpc-internal-encoding-request"
/** Compression filter for outgoing data.
*
diff --git a/src/core/surface/call.c b/src/core/surface/call.c
index 33f277d..4426bbb 100644
--- a/src/core/surface/call.c
+++ b/src/core/surface/call.c
@@ -1046,10 +1046,11 @@
(const gpr_uint8 *)md->value,
md->value_length, 1);
if (!grpc_mdstr_is_legal_header(l->md->key)) {
- gpr_log(GPR_ERROR, "attempt to send invalid metadata key");
+ gpr_log(GPR_ERROR, "attempt to send invalid metadata key: %s",
+ grpc_mdstr_as_c_string(l->md->key));
return 0;
} else if (!grpc_mdstr_is_bin_suffixed(l->md->key) &&
- !grpc_mdstr_is_legal_header(l->md->value)) {
+ !grpc_mdstr_is_legal_nonbin_header(l->md->value)) {
gpr_log(GPR_ERROR, "attempt to send invalid metadata value");
return 0;
}
diff --git a/src/core/transport/metadata.c b/src/core/transport/metadata.c
index f92e87e..3fd21a2 100644
--- a/src/core/transport/metadata.c
+++ b/src/core/transport/metadata.c
@@ -681,16 +681,34 @@
void grpc_mdctx_unlock(grpc_mdctx *ctx) { unlock(ctx); }
-int grpc_mdstr_is_legal_header(grpc_mdstr *s) {
- /* TODO(ctiller): consider caching this, or computing it on construction */
+static int conforms_to(grpc_mdstr *s, const gpr_uint8 *legal_bits) {
const gpr_uint8 *p = GPR_SLICE_START_PTR(s->slice);
const gpr_uint8 *e = GPR_SLICE_END_PTR(s->slice);
for (; p != e; p++) {
- if (*p < 32 || *p > 126) return 0;
+ int idx = *p;
+ int byte = idx / 8;
+ int bit = idx % 8;
+ if ((legal_bits[byte] & (1 << bit)) == 0) return 0;
}
return 1;
}
+int grpc_mdstr_is_legal_header(grpc_mdstr *s) {
+ static const gpr_uint8 legal_header_bits[256 / 8] = {
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0xff, 0x03, 0x00, 0x00, 0x00,
+ 0x80, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ return conforms_to(s, legal_header_bits);
+}
+
+int grpc_mdstr_is_legal_nonbin_header(grpc_mdstr *s) {
+ static const gpr_uint8 legal_header_bits[256 / 8] = {
+ 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ return conforms_to(s, legal_header_bits);
+}
+
int grpc_mdstr_is_bin_suffixed(grpc_mdstr *s) {
/* TODO(ctiller): consider caching this */
return grpc_is_binary_header((const char *)GPR_SLICE_START_PTR(s->slice),
diff --git a/src/core/transport/metadata.h b/src/core/transport/metadata.h
index a7af49b..eb17747 100644
--- a/src/core/transport/metadata.h
+++ b/src/core/transport/metadata.h
@@ -154,6 +154,7 @@
const char *grpc_mdstr_as_c_string(grpc_mdstr *s);
int grpc_mdstr_is_legal_header(grpc_mdstr *s);
+int grpc_mdstr_is_legal_nonbin_header(grpc_mdstr *s);
int grpc_mdstr_is_bin_suffixed(grpc_mdstr *s);
/* Batch mode metadata functions.
diff --git a/src/python/grpcio_test/grpc_test/_adapter/_links_test.py b/src/python/grpcio_test/grpc_test/_adapter/_links_test.py
index c4686b3..4077b8e 100644
--- a/src/python/grpcio_test/grpc_test/_adapter/_links_test.py
+++ b/src/python/grpcio_test/grpc_test/_adapter/_links_test.py
@@ -46,8 +46,8 @@
# TODO(nathaniel): End-to-end metadata testing.
def _transform_metadata(unused_metadata):
return (
- ('one unused key', 'one unused value'),
- ('another unused key', 'another unused value'),
+ ('one_unused_key', 'one unused value'),
+ ('another_unused_key', 'another unused value'),
)
diff --git a/src/python/grpcio_test/grpc_test/_cython/adapter_low_test.py b/src/python/grpcio_test/grpc_test/_cython/adapter_low_test.py
index 9bab930..f1bec23 100644
--- a/src/python/grpcio_test/grpc_test/_cython/adapter_low_test.py
+++ b/src/python/grpcio_test/grpc_test/_cython/adapter_low_test.py
@@ -76,7 +76,7 @@
CLIENT_METADATA_BIN_VALUE = b'\0'*1000
SERVER_INITIAL_METADATA_KEY = 'init_me_me_me'
SERVER_INITIAL_METADATA_VALUE = 'whodawha?'
- SERVER_TRAILING_METADATA_KEY = 'California_is_in_a_drought'
+ SERVER_TRAILING_METADATA_KEY = 'california_is_in_a_drought'
SERVER_TRAILING_METADATA_VALUE = 'zomg it is'
SERVER_STATUS_CODE = _types.StatusCode.OK
SERVER_STATUS_DETAILS = 'our work is never over'
diff --git a/src/python/grpcio_test/grpc_test/_links/_transmission_test.py b/src/python/grpcio_test/grpc_test/_links/_transmission_test.py
index 02ddd51..db011bc 100644
--- a/src/python/grpcio_test/grpc_test/_links/_transmission_test.py
+++ b/src/python/grpcio_test/grpc_test/_links/_transmission_test.py
@@ -66,9 +66,9 @@
def create_invocation_initial_metadata(self):
return (
- ('first invocation initial metadata key', 'just a string value'),
- ('second invocation initial metadata key', '0123456789'),
- ('third invocation initial metadata key-bin', '\x00\x57' * 100),
+ ('first_invocation_initial_metadata_key', 'just a string value'),
+ ('second_invocation_initial_metadata_key', '0123456789'),
+ ('third_invocation_initial_metadata_key-bin', '\x00\x57' * 100),
)
def create_invocation_terminal_metadata(self):
@@ -76,16 +76,16 @@
def create_service_initial_metadata(self):
return (
- ('first service initial metadata key', 'just another string value'),
- ('second service initial metadata key', '9876543210'),
- ('third service initial metadata key-bin', '\x00\x59\x02' * 100),
+ ('first_service_initial_metadata_key', 'just another string value'),
+ ('second_service_initial_metadata_key', '9876543210'),
+ ('third_service_initial_metadata_key-bin', '\x00\x59\x02' * 100),
)
def create_service_terminal_metadata(self):
return (
- ('first service terminal metadata key', 'yet another string value'),
- ('second service terminal metadata key', 'abcdefghij'),
- ('third service terminal metadata key-bin', '\x00\x37' * 100),
+ ('first_service_terminal_metadata_key', 'yet another string value'),
+ ('second_service_terminal_metadata_key', 'abcdefghij'),
+ ('third_service_terminal_metadata_key-bin', '\x00\x37' * 100),
)
def create_invocation_completion(self):
diff --git a/tools/codegen/core/gen_legal_metadata_characters.c b/tools/codegen/core/gen_legal_metadata_characters.c
new file mode 100644
index 0000000..5c290f2
--- /dev/null
+++ b/tools/codegen/core/gen_legal_metadata_characters.c
@@ -0,0 +1,73 @@
+/*
+ *
+ * Copyright 2015, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+/* generates constant table for metadata.c */
+
+#include <stdio.h>
+#include <string.h>
+
+static unsigned char legal_bits[256 / 8];
+
+static void legal(int x) {
+ int byte = x / 8;
+ int bit = x % 8;
+ legal_bits[byte] |= 1 << bit;
+}
+
+static void dump(void) {
+ int i;
+
+ printf("static const gpr_uint8 legal_header_bits[256/8] = ");
+ for (i = 0; i < 256 / 8; i++)
+ printf("%c 0x%02x", i ? ',' : '{', legal_bits[i]);
+ printf(" };\n");
+}
+
+static void clear(void) { memset(legal_bits, 0, sizeof(legal_bits)); }
+
+int main(void) {
+ int i;
+
+ clear();
+ for (i = 'a'; i <= 'z'; i++) legal(i);
+ for (i = '0'; i <= '9'; i++) legal(i);
+ legal('-');
+ legal('_');
+ dump();
+
+ clear();
+ for (i = 32; i <= 126; i++) legal(i);
+ dump();
+
+ return 0;
+}
diff --git a/tools/run_tests/jobset.py b/tools/run_tests/jobset.py
index 2a86319..79dde55 100755
--- a/tools/run_tests/jobset.py
+++ b/tools/run_tests/jobset.py
@@ -174,6 +174,7 @@
for k, v in add_env.iteritems():
env[k] = v
self._start = time.time()
+ print spec.cmdline
self._process = subprocess.Popen(args=spec.cmdline,
stderr=subprocess.STDOUT,
stdout=self._tempfile,
diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json
index c7df23a..7b57096 100644
--- a/tools/run_tests/sources_and_headers.json
+++ b/tools/run_tests/sources_and_headers.json
@@ -238,6 +238,15 @@
]
},
{
+ "deps": [],
+ "headers": [],
+ "language": "c",
+ "name": "gen_legal_metadata_characters",
+ "src": [
+ "tools/codegen/core/gen_legal_metadata_characters.c"
+ ]
+ },
+ {
"deps": [
"gpr",
"gpr_test_util"
diff --git a/vsprojects/Grpc.mak b/vsprojects/Grpc.mak
index 2893b72..e11653f 100644
--- a/vsprojects/Grpc.mak
+++ b/vsprojects/Grpc.mak
@@ -183,6 +183,14 @@
echo Running gen_hpack_tables
$(OUT_DIR)\gen_hpack_tables.exe
+gen_legal_metadata_characters.exe: $(OUT_DIR)
+ echo Building gen_legal_metadata_characters
+ $(CC) $(CFLAGS) /Fo:$(OUT_DIR)\ $(REPO_ROOT)\tools\codegen\core\gen_legal_metadata_characters.c
+ $(LINK) $(LFLAGS) /OUT:"$(OUT_DIR)\gen_legal_metadata_characters.exe" $(LIBS) $(OUT_DIR)\gen_legal_metadata_characters.obj
+gen_legal_metadata_characters: gen_legal_metadata_characters.exe
+ echo Running gen_legal_metadata_characters
+ $(OUT_DIR)\gen_legal_metadata_characters.exe
+
gpr_cmdline_test.exe: build_gpr_test_util build_gpr $(OUT_DIR)
echo Building gpr_cmdline_test
$(CC) $(CFLAGS) /Fo:$(OUT_DIR)\ $(REPO_ROOT)\test\core\support\cmdline_test.c