From 82f90f903045c85103c757c0710096537a0e3ea3 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 4 Feb 2015 01:15:56 +0900 Subject: [PATCH] nghttpx: Rewrite :authority and host header field We don't rewrite them if -s or -p is used --- integration-tests/nghttpx_http1_test.go | 46 ++++++++++++ integration-tests/nghttpx_http2_test.go | 42 +++++++++++ integration-tests/server_tester.go | 2 + src/http2.cc | 2 + src/shrpx_http2_downstream_connection.cc | 89 +++++++++++++++--------- src/shrpx_http2_session.cc | 3 +- src/shrpx_http_downstream_connection.cc | 50 +++++++++---- src/shrpx_http_downstream_connection.h | 2 + src/shrpx_https_upstream.cc | 6 +- 9 files changed, 196 insertions(+), 46 deletions(-) diff --git a/integration-tests/nghttpx_http1_test.go b/integration-tests/nghttpx_http1_test.go index 48ac9f7c..a150697b 100644 --- a/integration-tests/nghttpx_http1_test.go +++ b/integration-tests/nghttpx_http1_test.go @@ -139,6 +139,27 @@ func TestH1H1GracefulShutdown(t *testing.T) { } } +// TestH1H1HostRewrite tests that server rewrites Host header field +func TestH1H1HostRewrite(t *testing.T) { + st := newServerTester(nil, t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("request-host", r.Host) + }) + defer st.Close() + + res, err := st.http1(requestParam{ + name: "TestH1H1HostRewrite", + }) + if err != nil { + t.Fatalf("Error st.http1() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status: %v; want %v", got, want) + } + if got, want := res.header.Get("request-host"), st.backendHost; got != want { + t.Errorf("request-host: %v; want %v", got, want) + } +} + // TestH1H2ConnectFailure tests that server handles the situation that // connection attempt to HTTP/2 backend failed. func TestH1H2ConnectFailure(t *testing.T) { @@ -184,6 +205,31 @@ func TestH1H2NoHost(t *testing.T) { } } +// TestH1H2HTTP10 tests that server can accept HTTP/1.0 request +// without Host header field +func TestH1H2HTTP10(t *testing.T) { + st := newServerTester([]string{"--http2-bridge"}, t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("request-host", r.Host) + }) + defer st.Close() + + if _, err := io.WriteString(st.conn, "GET / HTTP/1.0\r\nTest-Case: TestH1H2HTTP10\r\n\r\n"); err != nil { + t.Fatalf("Error io.WriteString() = %v", err) + } + + resp, err := http.ReadResponse(bufio.NewReader(st.conn), nil) + if err != nil { + t.Fatalf("Error http.ReadResponse() = %v", err) + } + + if got, want := resp.StatusCode, 200; got != want { + t.Errorf("status: %v; want %v", got, want) + } + if got, want := resp.Header.Get("request-host"), st.backendHost; got != want { + t.Errorf("request-host: %v; want %v", got, want) + } +} + // TestH1H2CrumbleCookie tests that Cookies are crumbled and assembled // when forwarding to HTTP/2 backend link. go-nghttp2 server // concatenates crumbled Cookies automatically, so this test is not diff --git a/integration-tests/nghttpx_http2_test.go b/integration-tests/nghttpx_http2_test.go index 6efe9859..26bf3aa3 100644 --- a/integration-tests/nghttpx_http2_test.go +++ b/integration-tests/nghttpx_http2_test.go @@ -192,6 +192,27 @@ func TestH2H1NoVia(t *testing.T) { } } +// TestH2H1HostRewrite tests that server rewrites host header field +func TestH2H1HostRewrite(t *testing.T) { + st := newServerTester(nil, t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("request-host", r.Host) + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H1HostRewrite", + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status: %v; want %v", got, want) + } + if got, want := res.header.Get("request-host"), st.backendHost; got != want { + t.Errorf("request-host: %v; want %v", got, want) + } +} + // TestH2H1BadRequestCL tests that server rejects request whose // content-length header field value does not match its request body // size. @@ -584,3 +605,24 @@ func TestH2H2ConnectFailure(t *testing.T) { t.Errorf("status: %v; want %v", got, want) } } + +// TestH2H2HostRewrite tests that server rewrites host header field +func TestH2H2HostRewrite(t *testing.T) { + st := newServerTester([]string{"--http2-bridge"}, t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("request-host", r.Host) + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H2HostRewrite", + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status: %v; want %v", got, want) + } + if got, want := res.header.Get("request-host"), st.backendHost; got != want { + t.Errorf("request-host: %v; want %v", got, want) + } +} diff --git a/integration-tests/server_tester.go b/integration-tests/server_tester.go index d323925a..528ae886 100644 --- a/integration-tests/server_tester.go +++ b/integration-tests/server_tester.go @@ -42,6 +42,7 @@ type serverTester struct { url string // test frontend server URL t *testing.T ts *httptest.Server // backend server + backendHost string // backend server host conn net.Conn // connection to frontend server h2PrefaceSent bool // HTTP/2 preface was sent in conn nextStreamID uint32 // next stream ID @@ -124,6 +125,7 @@ func newServerTesterInternal(args []string, t *testing.T, handler http.HandlerFu t: t, ts: ts, url: fmt.Sprintf("%v://%v", scheme, authority), + backendHost: backendURL.Host, nextStreamID: 1, authority: authority, frCh: make(chan http2.Frame), diff --git a/src/http2.cc b/src/http2.cc index 9ddf9bfb..ab9fc772 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -224,6 +224,7 @@ void copy_headers_to_nva(std::vector &nva, const Headers &headers) { switch (lookup_token(kv.name)) { case HD_COOKIE: case HD_CONNECTION: + case HD_HOST: case HD_HTTP2_SETTINGS: case HD_KEEP_ALIVE: case HD_PROXY_CONNECTION: @@ -249,6 +250,7 @@ void build_http1_headers_from_headers(std::string &hdrs, switch (lookup_token(kv.name)) { case HD_CONNECTION: case HD_COOKIE: + case HD_HOST: case HD_HTTP2_SETTINGS: case HD_KEEP_ALIVE: case HD_PROXY_CONNECTION: diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index 72f8645c..39ff1c75 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -232,6 +232,33 @@ int Http2DownstreamConnection::push_request_headers() { if (!downstream_) { return 0; } + + const char *authority = nullptr, *host = nullptr; + if (!get_config()->http2_proxy && !get_config()->client_proxy) { + // HTTP/2 backend does not support multiple address, so we always + // use index = 0. + if (!downstream_->get_request_http2_authority().empty()) { + authority = get_config()->downstream_addrs[0].hostport.get(); + } + if (downstream_->get_request_header(http2::HD_HOST)) { + host = get_config()->downstream_addrs[0].hostport.get(); + } + } else { + if (!downstream_->get_request_http2_authority().empty()) { + authority = downstream_->get_request_http2_authority().c_str(); + } + auto h = downstream_->get_request_header(http2::HD_HOST); + if (h) { + host = h->value.c_str(); + } + } + + if (!authority && !host) { + // upstream is HTTP/1.0. We use backend server's host + // nonetheless. + host = get_config()->downstream_addrs[0].hostport.get(); + } + size_t nheader = downstream_->get_request_headers().size(); Headers cookies; @@ -239,28 +266,28 @@ int Http2DownstreamConnection::push_request_headers() { cookies = downstream_->crumble_request_cookie(); } - // 7 means: + // 8 means: // 1. :method // 2. :scheme // 3. :path - // 4. :authority (optional) - // 5. via (optional) - // 6. x-forwarded-for (optional) - // 7. x-forwarded-proto (optional) + // 4. :authority (at least either :authority or host exists) + // 5. host + // 6. via (optional) + // 7. x-forwarded-for (optional) + // 8. x-forwarded-proto (optional) auto nva = std::vector(); - nva.reserve(nheader + 7 + cookies.size()); + nva.reserve(nheader + 8 + cookies.size()); std::string via_value; std::string xff_value; - std::string scheme, authority, path, query; + std::string scheme, uri_authority, path, query; // To reconstruct HTTP/1 status line and headers, proxy should // preserve host header field. See draft-09 section 8.1.3.1. if (downstream_->get_request_method() == "CONNECT") { // The upstream may be HTTP/2 or HTTP/1 - if (!downstream_->get_request_http2_authority().empty()) { - nva.push_back(http2::make_nv_ls( - ":authority", downstream_->get_request_http2_authority())); + if (authority) { + nva.push_back(http2::make_nv_lc(":authority", authority)); } else { nva.push_back( http2::make_nv_ls(":authority", downstream_->get_request_path())); @@ -270,14 +297,8 @@ int Http2DownstreamConnection::push_request_headers() { nva.push_back( http2::make_nv_ls(":scheme", downstream_->get_request_http2_scheme())); nva.push_back(http2::make_nv_ls(":path", downstream_->get_request_path())); - if (!downstream_->get_request_http2_authority().empty()) { - nva.push_back(http2::make_nv_ls( - ":authority", downstream_->get_request_http2_authority())); - } else if (!downstream_->get_request_header(http2::HD_HOST)) { - if (LOG_ENABLED(INFO)) { - DCLOG(INFO, this) << "host header field missing"; - } - return -1; + if (authority) { + nva.push_back(http2::make_nv_lc(":authority", authority)); } } else { // The upstream is HTTP/1 @@ -288,11 +309,11 @@ int Http2DownstreamConnection::push_request_headers() { &u); if (rv == 0) { http2::copy_url_component(scheme, &u, UF_SCHEMA, url); - http2::copy_url_component(authority, &u, UF_HOST, url); + http2::copy_url_component(uri_authority, &u, UF_HOST, url); http2::copy_url_component(path, &u, UF_PATH, url); http2::copy_url_component(query, &u, UF_QUERY, url); if (path.empty()) { - if (!authority.empty() && + if (!uri_authority.empty() && downstream_->get_request_method() == "OPTIONS") { path = "*"; } else { @@ -319,28 +340,32 @@ int Http2DownstreamConnection::push_request_headers() { } else { nva.push_back(http2::make_nv_ls(":path", path)); } - if (!authority.empty()) { + + if (!get_config()->http2_proxy && !get_config()->client_proxy) { + if (authority) { + nva.push_back(http2::make_nv_lc(":authority", authority)); + } + } else if (!uri_authority.empty()) { // TODO properly check IPv6 numeric address - if (authority.find(":") != std::string::npos) { - authority = "[" + authority; - authority += "]"; + if (uri_authority.find(":") != std::string::npos) { + uri_authority = "[" + uri_authority; + uri_authority += "]"; } if (u.field_set & (1 << UF_PORT)) { - authority += ":"; - authority += util::utos(u.port); + uri_authority += ":"; + uri_authority += util::utos(u.port); } - nva.push_back(http2::make_nv_ls(":authority", authority)); - } else if (!downstream_->get_request_header(http2::HD_HOST)) { - if (LOG_ENABLED(INFO)) { - DCLOG(INFO, this) << "host header field missing"; - } - return -1; + nva.push_back(http2::make_nv_ls(":authority", uri_authority)); } } nva.push_back( http2::make_nv_ls(":method", downstream_->get_request_method())); + if (host) { + nva.push_back(http2::make_nv_lc("host", host)); + } + http2::copy_headers_to_nva(nva, downstream_->get_request_headers()); bool chunked_encoding = false; diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 2a0f19c1..f421fc00 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -634,7 +634,8 @@ int on_stream_close_callback(nghttp2_session *session, int32_t stream_id, auto http2session = static_cast(user_data); if (LOG_ENABLED(INFO)) { SSLOG(INFO, http2session) << "Stream stream_id=" << stream_id - << " is being closed"; + << " is being closed with error code " + << error_code; } auto sd = static_cast( nghttp2_session_get_stream_user_data(session, stream_id)); diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 3c6e87eb..f8c7dbde 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -107,7 +107,7 @@ void connectcb(struct ev_loop *loop, ev_io *w, int revents) { HttpDownstreamConnection::HttpDownstreamConnection( DownstreamConnectionPool *dconn_pool, struct ev_loop *loop) : DownstreamConnection(dconn_pool), rlimit_(loop, &rev_, 0, 0), - ioctrl_(&rlimit_), response_htp_{0}, loop_(loop), fd_(-1) { + ioctrl_(&rlimit_), response_htp_{0}, loop_(loop), addr_idx_(0), fd_(-1) { // We do not know fd yet, so just set dummy fd 0 ev_io_init(&wev_, connectcb, 0, EV_WRITE); ev_io_init(&rev_, readcb, 0, EV_READ); @@ -198,6 +198,8 @@ int HttpDownstreamConnection::attach_downstream(Downstream *downstream) { DCLOG(INFO, this) << "Connecting to downstream server"; } + addr_idx_ = i; + ev_io_set(&wev_, fd_, EV_WRITE); ev_io_set(&rev_, fd_, EV_READ); @@ -223,27 +225,50 @@ int HttpDownstreamConnection::attach_downstream(Downstream *downstream) { } int HttpDownstreamConnection::push_request_headers() { + const char *authority = nullptr, *host = nullptr; + if (!get_config()->http2_proxy && !get_config()->client_proxy) { + if (!downstream_->get_request_http2_authority().empty()) { + authority = get_config()->downstream_addrs[addr_idx_].hostport.get(); + } + if (downstream_->get_request_header(http2::HD_HOST)) { + host = get_config()->downstream_addrs[addr_idx_].hostport.get(); + } + } else { + if (!downstream_->get_request_http2_authority().empty()) { + authority = downstream_->get_request_http2_authority().c_str(); + } + auto h = downstream_->get_request_header(http2::HD_HOST); + if (h) { + host = h->value.c_str(); + } + } + + if (!authority && !host) { + // upstream is HTTP/1.0. We use backend server's host + // nonetheless. + host = get_config()->downstream_addrs[addr_idx_].hostport.get(); + } + downstream_->assemble_request_cookie(); // Assume that method and request path do not contain \r\n. std::string hdrs = downstream_->get_request_method(); hdrs += " "; if (downstream_->get_request_method() == "CONNECT") { - if (!downstream_->get_request_http2_authority().empty()) { - hdrs += downstream_->get_request_http2_authority(); + if (authority) { + hdrs += authority; } else { hdrs += downstream_->get_request_path(); } } else if (get_config()->http2_proxy && - !downstream_->get_request_http2_scheme().empty() && - !downstream_->get_request_http2_authority().empty() && + !downstream_->get_request_http2_scheme().empty() && authority && (downstream_->get_request_path().c_str()[0] == '/' || downstream_->get_request_path() == "*")) { // Construct absolute-form request target because we are going to // send a request to a HTTP/1 proxy. hdrs += downstream_->get_request_http2_scheme(); hdrs += "://"; - hdrs += downstream_->get_request_http2_authority(); + hdrs += authority; // Server-wide OPTIONS takes following form in proxy request: // @@ -259,13 +284,14 @@ int HttpDownstreamConnection::push_request_headers() { // don't care. hdrs += downstream_->get_request_path(); } - hdrs += " HTTP/1.1\r\n"; - if (!downstream_->get_request_header(http2::HD_HOST) && - !downstream_->get_request_http2_authority().empty()) { - hdrs += "Host: "; - hdrs += downstream_->get_request_http2_authority(); - hdrs += "\r\n"; + hdrs += " HTTP/1.1\r\nHost: "; + if (authority) { + hdrs += authority; + } else { + hdrs += host; } + hdrs += "\r\n"; + http2::build_http1_headers_from_headers(hdrs, downstream_->get_request_headers()); diff --git a/src/shrpx_http_downstream_connection.h b/src/shrpx_http_downstream_connection.h index d61de22b..fb95848b 100644 --- a/src/shrpx_http_downstream_connection.h +++ b/src/shrpx_http_downstream_connection.h @@ -70,6 +70,8 @@ private: IOControl ioctrl_; http_parser response_htp_; struct ev_loop *loop_; + // index of get_config()->downstream_addrs this object is using + size_t addr_idx_; int fd_; }; diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index e2d5b2eb..32d64c8b 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -160,7 +160,11 @@ int htp_hdrs_completecb(http_parser *htp) { return -1; } - downstream->inspect_http1_request(); + if (downstream->get_request_major() == 1 && + downstream->get_request_minor() == 1 && + !downstream->get_request_header(http2::HD_HOST)) { + return -1; + } if (get_config()->client_proxy && downstream->get_request_method() != "CONNECT") {