From 41dd5f689766c5d90030a308488eb38146a08ba3 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 9 Jun 2015 23:15:02 +0900 Subject: [PATCH] nghttpx: Tokenize request method We share the same method value with http-parser. This commit also returns 501 for unknown request method on HTTP/2 and SPDY frontend. --- genheaderfunc.py | 69 +-------- genmethodfunc.py | 53 +++++++ gentokenlookup.py | 69 +++++++++ integration-tests/nghttpx_http2_test.go | 20 +++ integration-tests/nghttpx_spdy_test.go | 20 +++ src/http2.cc | 186 +++++++++++++++++++++++ src/http2.h | 9 ++ src/shrpx_client_handler.cc | 5 +- src/shrpx_downstream.cc | 18 +-- src/shrpx_downstream.h | 6 +- src/shrpx_http2_downstream_connection.cc | 12 +- src/shrpx_http2_upstream.cc | 19 ++- src/shrpx_http_downstream_connection.cc | 6 +- src/shrpx_https_upstream.cc | 12 +- src/shrpx_spdy_upstream.cc | 25 ++- 15 files changed, 421 insertions(+), 108 deletions(-) create mode 100755 genmethodfunc.py create mode 100644 gentokenlookup.py diff --git a/genheaderfunc.py b/genheaderfunc.py index d87d6e3f..ce2f7ff0 100755 --- a/genheaderfunc.py +++ b/genheaderfunc.py @@ -1,5 +1,7 @@ #!/usr/bin/env python +from gentokenlookup import gentokenlookup + HEADERS = [ ':authority', ':method', @@ -34,70 +36,5 @@ HEADERS = [ 'upgrade' ] -def to_enum_hd(k): - res = 'HD_' - for c in k.upper(): - if c == ':' or c == '-': - res += '_' - continue - res += c - return res - -def build_header(headers): - res = {} - for k in headers: - size = len(k) - if size not in res: - res[size] = {} - ent = res[size] - c = k[-1] - if c not in ent: - ent[c] = [] - ent[c].append(k) - - return res - -def gen_enum(): - print '''\ -enum {''' - for k in sorted(HEADERS): - print '''\ - {},'''.format(to_enum_hd(k)) - print '''\ - HD_MAXIDX, -};''' - -def gen_index_header(): - print '''\ -int lookup_token(const uint8_t *name, size_t namelen) { - switch (namelen) {''' - b = build_header(HEADERS) - for size in sorted(b.keys()): - ents = b[size] - print '''\ - case {}:'''.format(size) - print '''\ - switch (name[{}]) {{'''.format(size - 1) - for c in sorted(ents.keys()): - headers = sorted(ents[c]) - print '''\ - case '{}':'''.format(c) - for k in headers: - print '''\ - if (util::streq_l("{}", name, {})) {{ - return {}; - }}'''.format(k[:-1], size - 1, to_enum_hd(k)) - print '''\ - break;''' - print '''\ - } - break;''' - print '''\ - } - return -1; -}''' - if __name__ == '__main__': - gen_enum() - print '' - gen_index_header() + gentokenlookup(HEADERS, 'HD') diff --git a/genmethodfunc.py b/genmethodfunc.py new file mode 100755 index 00000000..093c7bd0 --- /dev/null +++ b/genmethodfunc.py @@ -0,0 +1,53 @@ +#!/usr/bin/env python +from __future__ import unicode_literals +from io import StringIO + +from gentokenlookup import gentokenlookup + +# copied from http-parser/http_parser.h, and stripped trailing spaces +# and backslashes. +SRC = ''' + XX(0, DELETE, DELETE) + XX(1, GET, GET) + XX(2, HEAD, HEAD) + XX(3, POST, POST) + XX(4, PUT, PUT) + /* pathological */ + XX(5, CONNECT, CONNECT) + XX(6, OPTIONS, OPTIONS) + XX(7, TRACE, TRACE) + /* webdav */ + XX(8, COPY, COPY) + XX(9, LOCK, LOCK) + XX(10, MKCOL, MKCOL) + XX(11, MOVE, MOVE) + XX(12, PROPFIND, PROPFIND) + XX(13, PROPPATCH, PROPPATCH) + XX(14, SEARCH, SEARCH) + XX(15, UNLOCK, UNLOCK) + /* subversion */ + XX(16, REPORT, REPORT) + XX(17, MKACTIVITY, MKACTIVITY) + XX(18, CHECKOUT, CHECKOUT) + XX(19, MERGE, MERGE) + /* upnp */ + XX(20, MSEARCH, M-SEARCH) + XX(21, NOTIFY, NOTIFY) + XX(22, SUBSCRIBE, SUBSCRIBE) + XX(23, UNSUBSCRIBE, UNSUBSCRIBE) + /* RFC-5789 */ + XX(24, PATCH, PATCH) + XX(25, PURGE, PURGE) + /* CalDAV */ + XX(26, MKCALENDAR, MKCALENDAR) +''' + +if __name__ == '__main__': + methods = [] + for line in StringIO(SRC): + line = line.strip() + if not line.startswith('XX'): + continue + _, m, _ = line.split(',', 2) + methods.append(m.strip()) + gentokenlookup(methods, 'HTTP') diff --git a/gentokenlookup.py b/gentokenlookup.py new file mode 100644 index 00000000..254fd5d6 --- /dev/null +++ b/gentokenlookup.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python + +def to_enum_hd(k, prefix): + res = prefix + '_' + for c in k.upper(): + if c == ':' or c == '-': + res += '_' + continue + res += c + return res + +def build_header(headers): + res = {} + for k in headers: + size = len(k) + if size not in res: + res[size] = {} + ent = res[size] + c = k[-1] + if c not in ent: + ent[c] = [] + ent[c].append(k) + + return res + +def gen_enum(tokens, prefix): + print '''\ +enum {''' + for k in sorted(tokens): + print '''\ + {},'''.format(to_enum_hd(k, prefix)) + print '''\ + {}_MAXIDX, +}};'''.format(prefix) + +def gen_index_header(tokens, prefix): + print '''\ +int lookup_token(const uint8_t *name, size_t namelen) { + switch (namelen) {''' + b = build_header(tokens) + for size in sorted(b.keys()): + ents = b[size] + print '''\ + case {}:'''.format(size) + print '''\ + switch (name[{}]) {{'''.format(size - 1) + for c in sorted(ents.keys()): + headers = sorted(ents[c]) + print '''\ + case '{}':'''.format(c) + for k in headers: + print '''\ + if (util::streq_l("{}", name, {})) {{ + return {}; + }}'''.format(k[:-1], size - 1, to_enum_hd(k, prefix)) + print '''\ + break;''' + print '''\ + } + break;''' + print '''\ + } + return -1; +}''' + +def gentokenlookup(tokens, prefix): + gen_enum(tokens, prefix) + print '' + gen_index_header(tokens, prefix) diff --git a/integration-tests/nghttpx_http2_test.go b/integration-tests/nghttpx_http2_test.go index 953e9d42..d860a181 100644 --- a/integration-tests/nghttpx_http2_test.go +++ b/integration-tests/nghttpx_http2_test.go @@ -404,6 +404,26 @@ func TestH2H1ConnectFailure(t *testing.T) { } } +// TestH2H1InvalidMethod tests that server rejects invalid method with +// 501. +func TestH2H1InvalidMethod(t *testing.T) { + st := newServerTester(nil, t, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("server should not forward this request") + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H1InvalidMethod", + method: "get", + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 501; got != want { + t.Errorf("status: %v; want %v", got, want) + } +} + // TestH2H1AssembleCookies tests that crumbled cookies in HTTP/2 // request is assembled into 1 when forwarding to HTTP/1 backend link. func TestH2H1AssembleCookies(t *testing.T) { diff --git a/integration-tests/nghttpx_spdy_test.go b/integration-tests/nghttpx_spdy_test.go index 07a2b2d5..76d349e8 100644 --- a/integration-tests/nghttpx_spdy_test.go +++ b/integration-tests/nghttpx_spdy_test.go @@ -210,6 +210,26 @@ func TestS3H1HeaderFields(t *testing.T) { } } +// TestS3H1InvalidMethod tests that server rejects invalid method with +// 501. +func TestS3H1InvalidMethod(t *testing.T) { + st := newServerTesterTLS([]string{"--npn-list=spdy/3.1"}, t, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("server should not forward this request") + }) + defer st.Close() + + res, err := st.spdy(requestParam{ + name: "TestS3H1InvalidMethod", + method: "get", + }) + if err != nil { + t.Fatalf("Error st.spdy() = %v", err) + } + if got, want := res.status, 501; got != want { + t.Errorf("status: %v; want %v", got, want) + } +} + // TestS3H2ConnectFailure tests that server handles the situation that // connection attempt to HTTP/2 backend failed. func TestS3H2ConnectFailure(t *testing.T) { diff --git a/src/http2.cc b/src/http2.cc index 65b2cee3..b47787da 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -1113,6 +1113,192 @@ bool expect_response_body(const std::string &method, int status_code) { return method != "HEAD" && expect_response_body(status_code); } +bool expect_response_body(int method_token, int status_code) { + return method_token != HTTP_HEAD && expect_response_body(status_code); +} + +int lookup_method_token(const std::string &name) { + return lookup_method_token(reinterpret_cast(name.c_str()), + name.size()); +} + +// This function was generated by genmethodfunc.py. +int lookup_method_token(const uint8_t *name, size_t namelen) { + switch (namelen) { + case 3: + switch (name[2]) { + case 'T': + if (util::streq_l("GE", name, 2)) { + return HTTP_GET; + } + if (util::streq_l("PU", name, 2)) { + return HTTP_PUT; + } + break; + } + break; + case 4: + switch (name[3]) { + case 'D': + if (util::streq_l("HEA", name, 3)) { + return HTTP_HEAD; + } + break; + case 'E': + if (util::streq_l("MOV", name, 3)) { + return HTTP_MOVE; + } + break; + case 'K': + if (util::streq_l("LOC", name, 3)) { + return HTTP_LOCK; + } + break; + case 'T': + if (util::streq_l("POS", name, 3)) { + return HTTP_POST; + } + break; + case 'Y': + if (util::streq_l("COP", name, 3)) { + return HTTP_COPY; + } + break; + } + break; + case 5: + switch (name[4]) { + case 'E': + if (util::streq_l("MERG", name, 4)) { + return HTTP_MERGE; + } + if (util::streq_l("PURG", name, 4)) { + return HTTP_PURGE; + } + if (util::streq_l("TRAC", name, 4)) { + return HTTP_TRACE; + } + break; + case 'H': + if (util::streq_l("PATC", name, 4)) { + return HTTP_PATCH; + } + break; + case 'L': + if (util::streq_l("MKCO", name, 4)) { + return HTTP_MKCOL; + } + break; + } + break; + case 6: + switch (name[5]) { + case 'E': + if (util::streq_l("DELET", name, 5)) { + return HTTP_DELETE; + } + break; + case 'H': + if (util::streq_l("SEARC", name, 5)) { + return HTTP_SEARCH; + } + break; + case 'K': + if (util::streq_l("UNLOC", name, 5)) { + return HTTP_UNLOCK; + } + break; + case 'T': + if (util::streq_l("REPOR", name, 5)) { + return HTTP_REPORT; + } + break; + case 'Y': + if (util::streq_l("NOTIF", name, 5)) { + return HTTP_NOTIFY; + } + break; + } + break; + case 7: + switch (name[6]) { + case 'H': + if (util::streq_l("MSEARC", name, 6)) { + return HTTP_MSEARCH; + } + break; + case 'S': + if (util::streq_l("OPTION", name, 6)) { + return HTTP_OPTIONS; + } + break; + case 'T': + if (util::streq_l("CONNEC", name, 6)) { + return HTTP_CONNECT; + } + break; + } + break; + case 8: + switch (name[7]) { + case 'D': + if (util::streq_l("PROPFIN", name, 7)) { + return HTTP_PROPFIND; + } + break; + case 'T': + if (util::streq_l("CHECKOU", name, 7)) { + return HTTP_CHECKOUT; + } + break; + } + break; + case 9: + switch (name[8]) { + case 'E': + if (util::streq_l("SUBSCRIB", name, 8)) { + return HTTP_SUBSCRIBE; + } + break; + case 'H': + if (util::streq_l("PROPPATC", name, 8)) { + return HTTP_PROPPATCH; + } + break; + } + break; + case 10: + switch (name[9]) { + case 'R': + if (util::streq_l("MKCALENDA", name, 9)) { + return HTTP_MKCALENDAR; + } + break; + case 'Y': + if (util::streq_l("MKACTIVIT", name, 9)) { + return HTTP_MKACTIVITY; + } + break; + } + break; + case 11: + switch (name[10]) { + case 'E': + if (util::streq_l("UNSUBSCRIB", name, 10)) { + return HTTP_UNSUBSCRIBE; + } + break; + } + break; + } + return -1; +} + +const char *to_method_string(int method_token) { + // we happened to use same value for method with http-parser. + return http_method_str(static_cast(method_token)); +} + } // namespace http2 } // namespace nghttp2 diff --git a/src/http2.h b/src/http2.h index e2337247..5749dade 100644 --- a/src/http2.h +++ b/src/http2.h @@ -285,10 +285,19 @@ std::string path_join(const char *base_path, size_t base_pathlen, // true if response has body, taking into account the request method // and status code. bool expect_response_body(const std::string &method, int status_code); +bool expect_response_body(int method_token, int status_code); // true if response has body, taking into account status code only. bool expect_response_body(int status_code); +// Looks up method token for method name |name| of length |namelen|. +// Only methods defined in http-parser/http-parser.h (http_method) are +// tokenized. If method name cannot be tokenized, returns -1. +int lookup_method_token(const uint8_t *name, size_t namelen); +int lookup_method_token(const std::string &name); + +const char *to_method_string(int method_token); + } // namespace http2 } // namespace nghttp2 diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index 89b2590b..b360bd72 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -736,9 +736,10 @@ void ClientHandler::write_accesslog(Downstream *downstream) { upstream_accesslog( get_config()->accesslog_format, LogSpec{ - downstream, ipaddr_.c_str(), downstream->get_request_method().c_str(), + downstream, ipaddr_.c_str(), + http2::to_method_string(downstream->get_request_method()), - (downstream->get_request_method() != "CONNECT" && + (downstream->get_request_method() != HTTP_CONNECT && (get_config()->http2_proxy || get_config()->client_proxy)) ? construct_absolute_request_uri(downstream).c_str() : downstream->get_request_path().empty() diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 2d89adb4..e97e2913 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -117,7 +117,7 @@ Downstream::Downstream(Upstream *upstream, MemchunkPool *mcpool, response_headers_sum_(0), request_datalen_(0), response_datalen_(0), num_retry_(0), stream_id_(stream_id), priority_(priority), downstream_stream_id_(-1), - response_rst_stream_error_code_(NGHTTP2_NO_ERROR), + response_rst_stream_error_code_(NGHTTP2_NO_ERROR), request_method_(-1), request_state_(INITIAL), request_major_(1), request_minor_(1), response_state_(INITIAL), response_http_status_(0), response_major_(1), response_minor_(1), dispatch_state_(DISPATCH_NONE), @@ -470,13 +470,9 @@ void Downstream::append_last_request_trailer_value(const char *data, request_trailers_, data, len); } -void Downstream::set_request_method(std::string method) { - request_method_ = std::move(method); -} +void Downstream::set_request_method(int method) { request_method_ = method; } -const std::string &Downstream::get_request_method() const { - return request_method_; -} +int Downstream::get_request_method() const { return request_method_; } void Downstream::set_request_path(std::string path) { request_path_ = std::move(path); @@ -635,7 +631,7 @@ void Downstream::rewrite_location_response_header( return; } std::string new_uri; - if (get_config()->no_host_rewrite || request_method_ == "CONNECT") { + if (get_config()->no_host_rewrite || request_method_ == HTTP_CONNECT) { if (!request_http2_authority_.empty()) { new_uri = http2::rewrite_location_uri( (*hd).value, u, request_http2_authority_, request_http2_authority_, @@ -900,7 +896,7 @@ void Downstream::set_priority(int32_t pri) { priority_ = pri; } int32_t Downstream::get_priority() const { return priority_; } void Downstream::check_upgrade_fulfilled() { - if (request_method_ == "CONNECT") { + if (request_method_ == HTTP_CONNECT) { upgraded_ = 200 <= response_http_status_ && response_http_status_ < 300; return; @@ -915,13 +911,13 @@ void Downstream::check_upgrade_fulfilled() { } void Downstream::inspect_http2_request() { - if (request_method_ == "CONNECT") { + if (request_method_ == HTTP_CONNECT) { upgrade_request_ = true; } } void Downstream::inspect_http1_request() { - if (request_method_ == "CONNECT") { + if (request_method_ == HTTP_CONNECT) { upgrade_request_ = true; } diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index dc150584..f0edeb2c 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -141,8 +141,8 @@ public: void append_last_request_trailer_key(const char *data, size_t len); void append_last_request_trailer_value(const char *data, size_t len); - void set_request_method(std::string method); - const std::string &get_request_method() const; + void set_request_method(int method); + int get_request_method() const; void set_request_path(std::string path); void add_request_headers_sum(size_t amount); void @@ -360,7 +360,6 @@ private: std::chrono::high_resolution_clock::time_point request_start_time_; - std::string request_method_; std::string request_path_; std::string request_http2_scheme_; std::string request_http2_authority_; @@ -415,6 +414,7 @@ private: // RST_STREAM error_code from downstream HTTP2 connection uint32_t response_rst_stream_error_code_; + int request_method_; int request_state_; int request_major_; int request_minor_; diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index d4a4b090..b0622e50 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -258,7 +258,7 @@ int Http2DownstreamConnection::push_request_headers() { auto no_host_rewrite = get_config()->no_host_rewrite || get_config()->http2_proxy || get_config()->client_proxy || - downstream_->get_request_method() == "CONNECT"; + downstream_->get_request_method() == HTTP_CONNECT; // http2session_ has already in CONNECTED state, so we can get // addr_idx here. @@ -320,10 +320,10 @@ int Http2DownstreamConnection::push_request_headers() { std::string xff_value; std::string scheme, uri_authority, path, query; - nva.push_back( - http2::make_nv_ls(":method", downstream_->get_request_method())); + nva.push_back(http2::make_nv_lc( + ":method", http2::to_method_string(downstream_->get_request_method()))); - if (downstream_->get_request_method() == "CONNECT") { + if (downstream_->get_request_method() == HTTP_CONNECT) { if (authority) { nva.push_back(http2::make_nv_lc(":authority", authority)); } else { @@ -376,7 +376,7 @@ int Http2DownstreamConnection::push_request_headers() { } if (!get_config()->http2_proxy && !get_config()->client_proxy && - downstream_->get_request_method() != "CONNECT") { + downstream_->get_request_method() != HTTP_CONNECT) { // We use same protocol with :scheme header field if (scheme.empty()) { if (client_handler_->get_ssl()) { @@ -428,7 +428,7 @@ int Http2DownstreamConnection::push_request_headers() { downstream_->get_request_header(http2::HD_CONTENT_LENGTH); // TODO check content-length: 0 case - if (downstream_->get_request_method() == "CONNECT" || chunked_encoding || + if (downstream_->get_request_method() == HTTP_CONNECT || chunked_encoding || content_length || downstream_->get_request_http2_expect_body()) { // Request-body is expected. nghttp2_data_provider data_prd; diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 165c6272..d8016ca6 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -283,13 +283,21 @@ int Http2Upstream::on_request_headers(Downstream *downstream, // presence of mandatory header fields are guaranteed by libnghttp2. + auto method_token = http2::lookup_method_token(method->value); + if (method_token == -1) { + if (error_reply(downstream, 501) != 0) { + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } + return 0; + } + // For HTTP/2 proxy, we request :authority. - if (method->value != "CONNECT" && get_config()->http2_proxy && !authority) { + if (method_token != HTTP_CONNECT && get_config()->http2_proxy && !authority) { rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); return 0; } - downstream->set_request_method(http2::value_to_str(method)); + downstream->set_request_method(method_token); downstream->set_request_http2_scheme(http2::value_to_str(scheme)); downstream->set_request_http2_authority(http2::value_to_str(authority)); downstream->set_request_path(http2::value_to_str(path)); @@ -521,7 +529,8 @@ int on_frame_send_callback(nghttp2_session *session, const nghttp2_frame *frame, auto token = http2::lookup_token(nv.name, nv.namelen); switch (token) { case http2::HD__METHOD: - downstream->set_request_method({nv.value, nv.value + nv.valuelen}); + downstream->set_request_method( + http2::lookup_method_token(nv.value, nv.valuelen)); break; case http2::HD__SCHEME: downstream->set_request_http2_scheme( @@ -1307,8 +1316,8 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) { !get_config()->http2_proxy && (downstream->get_stream_id() % 2) && downstream->get_response_header(http2::HD_LINK) && downstream->get_response_http_status() == 200 && - (downstream->get_request_method() == "GET" || - downstream->get_request_method() == "POST")) { + (downstream->get_request_method() == HTTP_GET || + downstream->get_request_method() == HTTP_POST)) { if (prepare_push_promise(downstream) != 0) { return -1; diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 0703e630..bb50a646 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -214,7 +214,7 @@ int HttpDownstreamConnection::push_request_headers() { const char *authority = nullptr, *host = nullptr; auto downstream_hostport = get_config()->downstream_addrs[addr_idx_].hostport.get(); - auto connect_method = downstream_->get_request_method() == "CONNECT"; + auto connect_method = downstream_->get_request_method() == HTTP_CONNECT; if (!get_config()->no_host_rewrite && !get_config()->http2_proxy && !get_config()->client_proxy && !connect_method) { @@ -249,7 +249,7 @@ int HttpDownstreamConnection::push_request_headers() { downstream_->assemble_request_cookie(); // Assume that method and request path do not contain \r\n. - std::string hdrs = downstream_->get_request_method(); + std::string hdrs = http2::to_method_string(downstream_->get_request_method()); hdrs += ' '; if (connect_method) { if (authority) { @@ -583,7 +583,7 @@ int htp_hdrs_completecb(http_parser *htp) { // TODO It seems that the cases other than HEAD are handled by // http-parser. Need test. - return downstream->get_request_method() == "HEAD" || + return downstream->get_request_method() == HTTP_HEAD || (100 <= status && status <= 199) || status == 204 || status == 304 ? 1 diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 6f0cc33c..261d0c38 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -200,7 +200,7 @@ void rewrite_request_host_path_from_uri(Downstream *downstream, const char *uri, std::string path; if (u.field_set & (1 << UF_PATH)) { http2::copy_url_component(path, &u, UF_PATH, uri); - } else if (downstream->get_request_method() == "OPTIONS") { + } else if (downstream->get_request_method() == HTTP_OPTIONS) { // Server-wide OPTIONS takes following form in proxy request: // // OPTIONS http://example.org HTTP/1.1 @@ -235,8 +235,8 @@ int htp_hdrs_completecb(http_parser *htp) { } auto downstream = upstream->get_downstream(); - downstream->set_request_method( - http_method_str((enum http_method)htp->method)); + // We happen to have the same value for method token. + downstream->set_request_method(htp->method); downstream->set_request_major(htp->http_major); downstream->set_request_minor(htp->http_minor); @@ -244,7 +244,7 @@ int htp_hdrs_completecb(http_parser *htp) { if (LOG_ENABLED(INFO)) { std::stringstream ss; - ss << downstream->get_request_method() << " " + ss << http2::to_method_string(downstream->get_request_method()) << " " << downstream->get_request_path() << " " << "HTTP/" << downstream->get_request_major() << "." << downstream->get_request_minor() << "\n"; @@ -268,7 +268,7 @@ int htp_hdrs_completecb(http_parser *htp) { downstream->inspect_http1_request(); - if (downstream->get_request_method() != "CONNECT") { + if (downstream->get_request_method() != HTTP_CONNECT) { http_parser_url u{}; // make a copy of request path, since we may set request path // while we are refering to original request path. @@ -758,7 +758,7 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) { } } - auto connect_method = downstream->get_request_method() == "CONNECT"; + auto connect_method = downstream->get_request_method() == HTTP_CONNECT; std::string hdrs = "HTTP/"; hdrs += util::utos(downstream->get_request_major()); diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index a14b0ed6..57d8462b 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -192,15 +192,28 @@ void on_ctrl_recv_callback(spdylay_session *session, spdylay_frame_type type, auto host = downstream->get_request_header(http2::HD__HOST); auto method = downstream->get_request_header(http2::HD__METHOD); - bool is_connect = method && "CONNECT" == method->value; - if (!path || !host || !method || !http2::non_empty_value(host) || - !http2::non_empty_value(path) || !http2::non_empty_value(method) || - (!is_connect && (!scheme || !http2::non_empty_value(scheme)))) { - upstream->rst_stream(downstream, SPDYLAY_INTERNAL_ERROR); + if (!method) { + upstream->rst_stream(downstream, SPDYLAY_PROTOCOL_ERROR); return; } - downstream->set_request_method(method->value); + auto method_token = http2::lookup_method_token(method->value); + if (method_token == -1) { + if (upstream->error_reply(downstream, 501) != 0) { + ULOG(FATAL, upstream) << "error_reply failed"; + } + return; + } + + auto is_connect = method_token == HTTP_CONNECT; + if (!path || !host || !http2::non_empty_value(host) || + !http2::non_empty_value(path) || + (!is_connect && (!scheme || !http2::non_empty_value(scheme)))) { + upstream->rst_stream(downstream, SPDYLAY_PROTOCOL_ERROR); + return; + } + + downstream->set_request_method(method_token); if (is_connect) { downstream->set_request_http2_authority(path->value); } else {