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

@@ -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()
+    gentokenlookup(HEADERS, 'HD')
-    print ''
-    gen_index_header()

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')

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)

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) {

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) {

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<const uint8_t *>(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<http_method>(method_token));
+}
+
 } // namespace http2
 
 } // namespace nghttp2

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

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()

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) {
+void Downstream::set_request_method(int method) { request_method_ = method; }
-  request_method_ = std::move(method);
-}
 
-const std::string &Downstream::get_request_method() const {
+int Downstream::get_request_method() const { return request_method_; }
-  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;
   }
 

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);
+  void set_request_method(int method);
-  const std::string &get_request_method() const;
+  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_;

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(
+  nva.push_back(http2::make_nv_lc(
-      http2::make_nv_ls(":method", downstream_->get_request_method()));
+      ":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;

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() == HTTP_GET ||
-       downstream->get_request_method() == "POST")) {
+       downstream->get_request_method() == HTTP_POST)) {
 
     if (prepare_push_promise(downstream) != 0) {
       return -1;

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

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(
+  // We happen to have the same value for method token.
-      http_method_str((enum http_method)htp->method));
+  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());

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 (!method) {
-    if (!path || !host || !method || !http2::non_empty_value(host) ||
+      upstream->rst_stream(downstream, SPDYLAY_PROTOCOL_ERROR);
-        !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);
       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 {