From d74729d8c78ab50d0219f03e831f1aabcbad0b15 Mon Sep 17 00:00:00 2001 From: ctiller Date: Mon, 12 Jan 2015 13:31:36 -0800 Subject: [PATCH] Pre-allocate, check, and consume method, scheme, and content-type headers at the server. Verify that method, scheme, and te trailers are present, and error out if not. Complain if content-type doesn't match the format, but don't error out. This currently, for now, blindly allows all three schemes (grpc, http, https) without verification, although that will change once the C implementation finishes switching to http/https. Cloned from CL 82558661 by 'g4 patch'. Original change by klempner@klempner:grpc_metadata:808:citc on 2014/12/19 18:41:47. Change on 2015/01/12 by ctiller ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=83785251 --- src/core/channel/http_server_filter.c | 108 ++++++++++++++++++++++---- 1 file changed, 94 insertions(+), 14 deletions(-) diff --git a/src/core/channel/http_server_filter.c b/src/core/channel/http_server_filter.c index 9a20d79c61f..44eab43f09d 100644 --- a/src/core/channel/http_server_filter.c +++ b/src/core/channel/http_server_filter.c @@ -32,13 +32,26 @@ */ #include "src/core/channel/http_server_filter.h" + +#include #include -typedef struct call_data { int sent_status; } call_data; +typedef struct call_data { + int sent_status; + int seen_scheme; + int seen_method; + int seen_te_trailers; +} call_data; typedef struct channel_data { grpc_mdelem *te_trailers; - grpc_mdelem *status_md; + grpc_mdelem *method; + grpc_mdelem *http_scheme; + grpc_mdelem *https_scheme; + /* TODO(klempner): Remove this once we stop using it */ + grpc_mdelem *grpc_scheme; + grpc_mdelem *content_type; + grpc_mdelem *status; } channel_data; /* used to silence 'variable not used' warnings */ @@ -56,20 +69,54 @@ static void call_op(grpc_call_element *elem, grpc_call_element *from_elem, channel_data *channeld = elem->channel_data; GRPC_CALL_LOG_OP(GPR_INFO, elem, op); - ignore_unused(calld); - ignore_unused(channeld); - switch (op->type) { case GRPC_RECV_METADATA: - /* check if it's a te: trailers header */ - if (op->data.metadata == channeld->te_trailers) { + /* Check if it is one of the headers we care about. */ + if (op->data.metadata == channeld->te_trailers || + op->data.metadata == channeld->method || + op->data.metadata == channeld->http_scheme || + op->data.metadata == channeld->https_scheme || + op->data.metadata == channeld->grpc_scheme || + op->data.metadata == channeld->content_type) { /* swallow it */ + if (op->data.metadata == channeld->method) { + calld->seen_method = 1; + } else if (op->data.metadata->key == channeld->http_scheme->key) { + calld->seen_scheme = 1; + } else if (op->data.metadata == channeld->te_trailers) { + calld->seen_te_trailers = 1; + } + /* TODO(klempner): Track that we've seen all the headers we should + require */ grpc_mdelem_unref(op->data.metadata); op->done_cb(op->user_data, GRPC_OP_OK); - } else if (op->data.metadata->key == channeld->te_trailers->key) { - gpr_log(GPR_ERROR, "Invalid te: header: '%s'", + } else if (op->data.metadata->key == channeld->content_type->key) { + if (strncmp(grpc_mdstr_as_c_string(op->data.metadata->value), + "application/grpc+", 17) == 0) { + /* Although the C implementation doesn't (currently) generate them, + any + custom +-suffix is explicitly valid. */ + /* TODO(klempner): We should consider preallocating common values such + as +proto or +json, or at least stashing them if we see them. */ + /* TODO(klempner): Should we be surfacing this to application code? */ + } else { + /* TODO(klempner): We're currently allowing this, but we shouldn't + see it without a proxy so log for now. */ + gpr_log(GPR_INFO, "Unexpected content-type %s", + channeld->content_type->key); + } + grpc_mdelem_unref(op->data.metadata); + op->done_cb(op->user_data, GRPC_OP_OK); + } else if (op->data.metadata->key == channeld->te_trailers->key || + op->data.metadata->key == channeld->method->key || + op->data.metadata->key == channeld->http_scheme->key || + op->data.metadata->key == channeld->content_type->key) { + gpr_log(GPR_ERROR, "Invalid %s: header: '%s'", + grpc_mdstr_as_c_string(op->data.metadata->key), grpc_mdstr_as_c_string(op->data.metadata->value)); - /* swallow it */ + /* swallow it and error everything out. */ + /* TODO(klempner): We ought to generate more descriptive error messages + on the wire here. */ grpc_mdelem_unref(op->data.metadata); op->done_cb(op->user_data, GRPC_OP_OK); grpc_call_element_send_cancel(elem); @@ -78,14 +125,33 @@ static void call_op(grpc_call_element *elem, grpc_call_element *from_elem, grpc_call_next_op(elem, op); } break; + case GRPC_RECV_END_OF_INITIAL_METADATA: + /* Have we seen the required http2 transport headers? + (:method, :scheme, content-type, with :path and :authority covered + at the channel level right now) */ + if (calld->seen_method && calld->seen_scheme && calld->seen_te_trailers) { + grpc_call_next_op(elem, op); + } else { + if (!calld->seen_method) { + gpr_log(GPR_ERROR, "Missing :method header"); + } else if (!calld->seen_scheme) { + gpr_log(GPR_ERROR, "Missing :scheme header"); + } else if (!calld->seen_te_trailers) { + gpr_log(GPR_ERROR, "Missing te trailers header"); + } + /* Error this call out */ + op->done_cb(op->user_data, GRPC_OP_OK); + grpc_call_element_send_cancel(elem); + } + break; case GRPC_SEND_START: case GRPC_SEND_METADATA: /* If we haven't sent status 200 yet, we need to so so because it needs to come before any non : prefixed metadata. */ if (!calld->sent_status) { calld->sent_status = 1; - /* status_md is reffed by grpc_call_element_send_metadata */ - grpc_call_element_send_metadata(elem, channeld->status_md); + /* status is reffed by grpc_call_element_send_metadata */ + grpc_call_element_send_metadata(elem, channeld->status); } grpc_call_next_op(elem, op); break; @@ -124,6 +190,9 @@ static void init_call_elem(grpc_call_element *elem, /* initialize members */ calld->sent_status = 0; + calld->seen_scheme = 0; + calld->seen_method = 0; + calld->seen_te_trailers = 0; } /* Destructor for call_data */ @@ -151,7 +220,13 @@ static void init_channel_elem(grpc_channel_element *elem, /* initialize members */ channeld->te_trailers = grpc_mdelem_from_strings(mdctx, "te", "trailers"); - channeld->status_md = grpc_mdelem_from_strings(mdctx, ":status", "200"); + channeld->status = grpc_mdelem_from_strings(mdctx, ":status", "200"); + channeld->method = grpc_mdelem_from_strings(mdctx, ":method", "POST"); + channeld->http_scheme = grpc_mdelem_from_strings(mdctx, ":scheme", "http"); + channeld->https_scheme = grpc_mdelem_from_strings(mdctx, ":scheme", "https"); + channeld->grpc_scheme = grpc_mdelem_from_strings(mdctx, ":scheme", "grpc"); + channeld->content_type = + grpc_mdelem_from_strings(mdctx, "content-type", "application/grpc"); } /* Destructor for channel data */ @@ -160,7 +235,12 @@ static void destroy_channel_elem(grpc_channel_element *elem) { channel_data *channeld = elem->channel_data; grpc_mdelem_unref(channeld->te_trailers); - grpc_mdelem_unref(channeld->status_md); + grpc_mdelem_unref(channeld->status); + grpc_mdelem_unref(channeld->method); + grpc_mdelem_unref(channeld->http_scheme); + grpc_mdelem_unref(channeld->https_scheme); + grpc_mdelem_unref(channeld->grpc_scheme); + grpc_mdelem_unref(channeld->content_type); } const grpc_channel_filter grpc_http_server_filter = {