Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

[metadata] Allow non application/grpc content-type values (#35824)

Browse Source 
Fixes #33935

The gRPC spec is not clear on how to handle headers where the "content-type" is not "application/grpc" or similar. We should be safe and not reject such headers.

Closes #35824

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35824 from yashykt:Fix33935 3d89af87ec
PiperOrigin-RevId: 604742049
pull/35833/head
Yash Tibrewal 12 months ago committed by Copybara-Service
parent 387c894117
commit 242e9185a3
3 changed files with 115 additions and 13 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 6
      src/core/lib/transport/metadata_batch.cc
  2. 109
      test/core/bad_client/tests/simple_request.cc
  3. 13
      test/core/bad_client/tests/simple_request_text_html_content_type.headers

6
src/core/lib/transport/metadata_batch.cc
Unescape View File

@ -64,7 +64,7 @@ absl::optional<absl::string_view> UnknownMap::GetStringValue(
} // namespace metadata_detail
ContentTypeMetadata::MementoType ContentTypeMetadata::ParseMemento(
Slice value, bool, MetadataParseErrorFn on_error) {
Slice value, bool, MetadataParseErrorFn /*on_error*/) {
auto out = kInvalid;
auto value_string = value.as_string_view();
if (value_string == "application/grpc") {
@ -76,7 +76,9 @@ ContentTypeMetadata::MementoType ContentTypeMetadata::ParseMemento(
} else if (value_string.empty()) {
out = kEmpty;
} else {
on_error("invalid value", value);
// We are intentionally not invoking on_error here since the spec is not
// clear on what the behavior should be here, so to avoid breaking anyone,
// we should continue to accept this.
}
return out;
}

109
test/core/bad_client/tests/simple_request.cc
Unescape View File

@ -81,6 +81,27 @@
"\x10\x0cgrpc-timeout\x02" \
"5S"
#define PFX_STR_TEXT_HTML_CONTENT_TYPE_HEADER \
"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n" \
"\x00\x00\x00\x04\x00\x00\x00\x00\x00" /* settings frame */ \
"\x00\x00\xdf\x01\x04\x00\x00\x00\x01" /* headers: generated from \
simple_request_text_html_content_type.headers \
in this directory */ \
"\x10\x05:path\x08/foo/bar" \
"\x10\x07:scheme\x04http" \
"\x10\x07:method\x04POST" \
"\x10\x04host\x09localhost" \
"\x10\x0c" \
"content-type\x09text/html" \
"\x10\x14grpc-accept-encoding\x15" \
"deflate,identity,gzip" \
"\x10\x02te\x08trailers" \
"\x10\x0auser-agent\"bad-client grpc-c/0.12.0.0 (linux)" \
"\x10\x0cgrpc-timeout\x03" \
"10S" \
"\x10\x0cgrpc-timeout\x02" \
"5S"
static void verifier(grpc_server* server, grpc_completion_queue* cq,
void* /*registered_method*/) {
grpc_call_error error;
@ -107,6 +128,67 @@ static void verifier(grpc_server* server, grpc_completion_queue* cq,
grpc_call_unref(s);
}
static void VerifyRpcDoesNotGetCanceled(grpc_server* server,
grpc_completion_queue* cq,
void* /*registered_method*/) {
grpc_call_error error;
grpc_call* s;
grpc_call_details call_details;
grpc_core::CqVerifier cqv(cq);
grpc_metadata_array request_metadata_recv;
int was_cancelled = 2;
grpc_call_details_init(&call_details);
grpc_metadata_array_init(&request_metadata_recv);
error = grpc_server_request_call(server, &s, &call_details,
&request_metadata_recv, cq, cq,
grpc_core::CqVerifier::tag(101));
GPR_ASSERT(GRPC_CALL_OK == error);
cqv.Expect(grpc_core::CqVerifier::tag(101), true);
cqv.Verify();
GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.host, "localhost"));
GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/foo/bar"));
grpc_op* op;
grpc_op ops[6];
// Send the initial metadata and the status from the server.
memset(ops, 0, sizeof(ops));
op = ops;
op->op = GRPC_OP_SEND_INITIAL_METADATA;
op->data.send_initial_metadata.count = 0;
op->flags = 0;
op->reserved = nullptr;
op++;
op->op = GRPC_OP_RECV_CLOSE_ON_SERVER;
op->data.recv_close_on_server.cancelled = &was_cancelled;
op->flags = 0;
op->reserved = nullptr;
op++;
op->op = GRPC_OP_SEND_STATUS_FROM_SERVER;
op->data.send_status_from_server.trailing_metadata_count = 0;
op->data.send_status_from_server.status = GRPC_STATUS_UNIMPLEMENTED;
grpc_slice status_details = grpc_slice_from_static_string("xyz");
op->data.send_status_from_server.status_details = &status_details;
op->flags = 0;
op->reserved = nullptr;
op++;
error = grpc_call_start_batch(s, ops, static_cast<size_t>(op - ops),
grpc_core::CqVerifier::tag(103), nullptr);
GPR_ASSERT(GRPC_CALL_OK == error);
cqv.Expect(grpc_core::CqVerifier::tag(103), true);
cqv.Verify();
// If the call had an error, `was_cancelled` would be 1.
// GPR_ASSERT(was_cancelled == 1);
grpc_metadata_array_destroy(&request_metadata_recv);
grpc_call_details_destroy(&call_details);
grpc_call_unref(s);
}
static void failure_verifier(grpc_server* server, grpc_completion_queue* cq,
void* /*registered_method*/) {
while (grpc_core::Server::FromC(server)->HasOpenConnections()) {
@ -120,45 +202,50 @@ int main(int argc, char** argv) {
grpc::testing::TestEnvironment env(&argc, argv);
grpc_init();
/* basic request: check that things are working */
// basic request: check that things are working
GRPC_RUN_BAD_CLIENT_TEST(verifier, nullptr, PFX_STR, 0);
GRPC_RUN_BAD_CLIENT_TEST(verifier, nullptr, PFX_STR_UNUSUAL, 0);
GRPC_RUN_BAD_CLIENT_TEST(verifier, nullptr, PFX_STR_UNUSUAL2, 0);
/* push an illegal data frame */
// A basic request with a "content-type: text/html" header. The spec is
// not clear on what the behavior should be here, so to avoid breaking anyone,
// we should continue to accept this header.
GRPC_RUN_BAD_CLIENT_TEST(VerifyRpcDoesNotGetCanceled, nullptr,
PFX_STR_TEXT_HTML_CONTENT_TYPE_HEADER, 0);
// push an illegal data frame
GRPC_RUN_BAD_CLIENT_TEST(verifier, nullptr,
PFX_STR
"\x00\x00\x05\x00\x00\x00\x00\x00\x01"
"\x34\x00\x00\x00\x00",
0);
/* push a data frame with bad flags */
// push a data frame with bad flags
GRPC_RUN_BAD_CLIENT_TEST(verifier, nullptr,
PFX_STR "\x00\x00\x00\x00\x02\x00\x00\x00\x01", 0);
/* push a window update with a bad length */
// push a window update with a bad length
GRPC_RUN_BAD_CLIENT_TEST(failure_verifier, nullptr,
PFX_STR "\x00\x00\x01\x08\x00\x00\x00\x00\x01", 0);
/* push a window update with bad flags */
// push a window update with bad flags
GRPC_RUN_BAD_CLIENT_TEST(failure_verifier, nullptr,
PFX_STR "\x00\x00\x00\x08\x10\x00\x00\x00\x01", 0);
/* push a window update with bad data (0 is not legal window size increment)
*/
// push a window update with bad data (0 is not legal window size increment)
GRPC_RUN_BAD_CLIENT_TEST(failure_verifier, nullptr,
PFX_STR
"\x00\x00\x04\x08\x00\x00\x00\x00\x01"
"\x00\x00\x00\x00",
0);
/* push a short goaway */
// push a short goaway
GRPC_RUN_BAD_CLIENT_TEST(failure_verifier, nullptr,
PFX_STR "\x00\x00\x04\x07\x00\x00\x00\x00\x00", 0);
/* disconnect before sending goaway */
// disconnect before sending goaway
GRPC_RUN_BAD_CLIENT_TEST(failure_verifier, nullptr,
PFX_STR "\x00\x01\x12\x07\x00\x00\x00\x00\x00",
GRPC_BAD_CLIENT_DISCONNECT);
/* push a rst_stream with a bad length */
// push a rst_stream with a bad length
GRPC_RUN_BAD_CLIENT_TEST(failure_verifier, nullptr,
PFX_STR "\x00\x00\x01\x03\x00\x00\x00\x00\x01", 0);
/* push a rst_stream with bad flags */
// push a rst_stream with bad flags
GRPC_RUN_BAD_CLIENT_TEST(failure_verifier, nullptr,
PFX_STR "\x00\x00\x00\x03\x10\x00\x00\x00\x01", 0);

13
test/core/bad_client/tests/simple_request_text_html_content_type.headers
Unescape View File

@ -0,0 +1,13 @@
# headers used in simple_request.c
# use tools/codegen/core/gen_header_frame.py to generate the binary strings
# contained in the source code
:path: /foo/bar
:scheme: http
:method: POST
host: localhost
content-type: text/html
grpc-accept-encoding: deflate,identity,gzip
te: trailers
user-agent: bad-client grpc-c/0.12.0.0 (linux)
grpc-timeout: 10S
grpc-timeout: 5S
Write Preview
Loading…
Cancel
Save