From d247470da2e035d1a6eaea7d2c2796b6051bd909 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 9 Apr 2015 00:25:47 +0900 Subject: [PATCH 01/50] nghttpx: Rewrite ocsp without thread Since libev handles SIGCHLD, using waitpid in separate thread to wait for the completion of fetch-ocsp-response script process is undefined. This commit rewrite ocsp handling code so that it utilizes libev ev_child watcher and perform ocsp update without thread. --- src/shrpx.cc | 7 +- src/shrpx_connection_handler.cc | 266 ++++++++++++++++++++++++-------- src/shrpx_connection_handler.h | 47 ++++-- src/shrpx_ssl.cc | 112 -------------- src/shrpx_ssl.h | 2 - 5 files changed, 237 insertions(+), 197 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 9b4d88b4..20b9456b 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -576,8 +576,6 @@ void refresh_cb(struct ev_loop *loop, ev_timer *w, int revents) { (!worker || worker->get_worker_stat()->num_connections == 0)) { ev_break(loop); } - - conn_handler->handle_ocsp_completion(); } } // namespace @@ -748,7 +746,7 @@ int event_loop() { ev_timer_again(loop, &refresh_timer); if (!get_config()->upstream_no_tls && !get_config()->no_ocsp) { - conn_handler->update_ocsp_async(); + conn_handler->proceed_next_cert_ocsp(); } if (LOG_ENABLED(INFO)) { @@ -758,7 +756,7 @@ int event_loop() { ev_run(loop, 0); conn_handler->join_worker(); - conn_handler->join_ocsp_thread(); + conn_handler->cancel_ocsp_update(); return 0; } @@ -2142,7 +2140,6 @@ int main(int argc, char **argv) { memset(&act, 0, sizeof(struct sigaction)); act.sa_handler = SIG_IGN; sigaction(SIGPIPE, &act, nullptr); - sigaction(SIGCHLD, &act, nullptr); event_loop(); diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index 87bf9336..1021929d 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -25,9 +25,14 @@ #include "shrpx_connection_handler.h" #include +#include +#include #include #include +#ifndef NOTHREADS +#include +#endif // !NOTHREADS #include "shrpx_client_handler.h" #include "shrpx_ssl.h" @@ -67,7 +72,23 @@ void ocsp_cb(struct ev_loop *loop, ev_timer *w, int revent) { return; } - h->update_ocsp_async(); + h->proceed_next_cert_ocsp(); +} +} // namespace + +namespace { +void ocsp_read_cb(struct ev_loop *loop, ev_io *w, int revent) { + auto h = static_cast(w->data); + + h->read_ocsp_chunk(); +} +} // namespace + +namespace { +void ocsp_chld_cb(struct ev_loop *loop, ev_child *w, int revent) { + auto h = static_cast(w->data); + + h->handle_ocsp_complete(); } } // namespace @@ -79,6 +100,17 @@ ConnectionHandler::ConnectionHandler(struct ev_loop *loop) ev_timer_init(&ocsp_timer_, ocsp_cb, 0., 0.); ocsp_timer_.data = this; + + ev_io_init(&ocsp_.rev, ocsp_read_cb, -1, EV_READ); + ocsp_.rev.data = this; + + ev_child_init(&ocsp_.chldev, ocsp_chld_cb, 0, 0); + ocsp_.chldev.data = this; + + ocsp_.next = 0; + ocsp_.fd = -1; + + reset_ocsp(); } ConnectionHandler::~ConnectionHandler() { @@ -315,80 +347,190 @@ bool ConnectionHandler::get_graceful_shutdown() const { return graceful_shutdown_; } -namespace { -void update_ocsp_ssl_ctx(SSL_CTX *ssl_ctx) { +void ConnectionHandler::cancel_ocsp_update() { + if (ocsp_.pid == 0) { + return; + } + + kill(ocsp_.pid, SIGTERM); +} + +// inspired by h2o_read_command function from h2o project: +// https://github.com/h2o/h2o +int ConnectionHandler::start_ocsp_update(const char *cert_file) { +#ifndef NOTHREADS + int rv; + int pfd[2]; + + assert(!ev_is_active(&ocsp_.rev)); + assert(!ev_is_active(&ocsp_.chldev)); + + char *const argv[] = { + const_cast(get_config()->fetch_ocsp_response_file.get()), + const_cast(cert_file), nullptr}; + char *const envp[] = {nullptr}; + +#ifdef O_CLOEXEC + if (pipe2(pfd, O_CLOEXEC) == -1) { + return -1; + } +#else // !O_CLOEXEC + if (pipe(pfd) == -1) { + return -1; + } + util::make_socket_closeonexec(pfd[0]); + util::make_socket_closeonexec(pfd[1]); +#endif // !O_CLOEXEC + + auto closer = defer([&pfd]() { + if (pfd[0] != -1) { + close(pfd[0]); + } + + if (pfd[1] != -1) { + close(pfd[1]); + } + }); + + // posix_spawn family functions are really interesting. They makes + // fork + dup2 + execve pattern easier. + + posix_spawn_file_actions_t file_actions; + + if (posix_spawn_file_actions_init(&file_actions) != 0) { + return -1; + } + + auto file_actions_del = + defer(posix_spawn_file_actions_destroy, &file_actions); + + if (posix_spawn_file_actions_adddup2(&file_actions, pfd[1], 1) != 0) { + return -1; + } + + if (posix_spawn_file_actions_addclose(&file_actions, pfd[0]) != 0) { + return -1; + } + + rv = posix_spawn(&ocsp_.pid, argv[0], &file_actions, nullptr, argv, envp); + if (rv != 0) { + LOG(WARN) << "Cannot execute ocsp query command: " << argv[0] + << ", errno=" << rv; + return -1; + } + + close(pfd[1]); + pfd[1] = -1; + + ocsp_.fd = pfd[0]; + pfd[0] = -1; + + util::make_socket_nonblocking(ocsp_.fd); + ev_io_set(&ocsp_.rev, ocsp_.fd, EV_READ); + ev_io_start(loop_, &ocsp_.rev); + + ev_child_set(&ocsp_.chldev, ocsp_.pid, 0); + ev_child_start(loop_, &ocsp_.chldev); +#endif // !NOTHREADS + return 0; +} + +void ConnectionHandler::read_ocsp_chunk() { + std::array buf; + for (;;) { + ssize_t n; + while ((n = read(ocsp_.fd, buf.data(), buf.size())) == -1 && errno == EINTR) + ; + + if (n == -1) { + if (errno == EAGAIN || errno == EWOULDBLOCK) { + return; + } + auto error = errno; + LOG(WARN) << "Reading from ocsp query command failed: errno=" << error; + ocsp_.error = error; + + break; + } + + if (n == 0) { + break; + } + + std::copy_n(std::begin(buf), n, std::back_inserter(ocsp_.resp)); + } + + ev_io_stop(loop_, &ocsp_.rev); +} + +void ConnectionHandler::handle_ocsp_complete() { + ev_io_stop(loop_, &ocsp_.rev); + ev_child_stop(loop_, &ocsp_.chldev); + + auto rstatus = ocsp_.chldev.rstatus; + auto status = WEXITSTATUS(rstatus); + if (ocsp_.error || !WIFEXITED(rstatus) || status != 0) { + LOG(WARN) << "ocsp query command failed: error=" << ocsp_.error + << ", rstatus=" << rstatus << ", status=" << status; + ++ocsp_.next; + proceed_next_cert_ocsp(); + return; + } + + assert(ocsp_.next < all_ssl_ctx_.size()); + + auto ssl_ctx = all_ssl_ctx_[ocsp_.next]; auto tls_ctx_data = static_cast(SSL_CTX_get_app_data(ssl_ctx)); - auto cert_file = tls_ctx_data->cert_file; - - std::vector out; - if (ssl::get_ocsp_response(out, cert_file) != 0) { - LOG(WARN) << "ocsp update for " << cert_file << " failed"; - return; - } if (LOG_ENABLED(INFO)) { - LOG(INFO) << "ocsp update for " << cert_file << " finished successfully"; + LOG(INFO) << "ocsp update for " << tls_ctx_data->cert_file + << " finished successfully"; } - std::lock_guard g(tls_ctx_data->mu); - tls_ctx_data->ocsp_data = std::move(out); -} -} // namespace - -void ConnectionHandler::update_ocsp() { - for (auto ssl_ctx : all_ssl_ctx_) { - update_ocsp_ssl_ctx(ssl_ctx); + { + std::lock_guard g(tls_ctx_data->mu); + tls_ctx_data->ocsp_data = std::move(ocsp_.resp); } + + ++ocsp_.next; + proceed_next_cert_ocsp(); } -void ConnectionHandler::update_ocsp_async() { -#ifndef NOTHREADS - ocsp_result_ = std::async(std::launch::async, [this]() { - // Log files are managed per thread. We have to open log files - // for this thread. We don't reopen log files in this thread when - // signal is received. This is future TODO. - reopen_log_files(); - auto closer = defer([]() { - auto lgconf = log_config(); - if (lgconf->accesslog_fd != -1) { - close(lgconf->accesslog_fd); - } - if (lgconf->errorlog_fd != -1) { - close(lgconf->errorlog_fd); - } - }); +void ConnectionHandler::reset_ocsp() { + if (ocsp_.fd != -1) { + close(ocsp_.fd); + } - update_ocsp(); - }); -#endif // !NOTHREADS + ocsp_.fd = -1; + ocsp_.pid = 0; + ocsp_.error = 0; + ocsp_.resp = std::vector(); } -void ConnectionHandler::handle_ocsp_completion() { -#ifndef NOTHREADS - if (!ocsp_result_.valid()) { - return; +void ConnectionHandler::proceed_next_cert_ocsp() { + for (;;) { + reset_ocsp(); + if (ocsp_.next == all_ssl_ctx_.size()) { + ocsp_.next = 0; + // We have updated all ocsp response, and schedule next update. + ev_timer_set(&ocsp_timer_, get_config()->ocsp_update_interval, 0.); + ev_timer_start(loop_, &ocsp_timer_); + return; + } + + auto ssl_ctx = all_ssl_ctx_[ocsp_.next]; + auto tls_ctx_data = + static_cast(SSL_CTX_get_app_data(ssl_ctx)); + auto cert_file = tls_ctx_data->cert_file; + + if (start_ocsp_update(cert_file) != 0) { + ++ocsp_.next; + continue; + } + + break; } - - if (ocsp_result_.wait_for(std::chrono::seconds(0)) != - std::future_status::ready) { - return; - } - - ocsp_result_.get(); - - ev_timer_set(&ocsp_timer_, get_config()->ocsp_update_interval, 0.); - ev_timer_start(loop_, &ocsp_timer_); -#endif // !NOTHREADS -} - -void ConnectionHandler::join_ocsp_thread() { -#ifndef NOTHREADS - if (!ocsp_result_.valid()) { - return; - } - ocsp_result_.get(); -#endif // !NOTHREADS } } // namespace shrpx diff --git a/src/shrpx_connection_handler.h b/src/shrpx_connection_handler.h index c5f6e80b..d8c328c5 100644 --- a/src/shrpx_connection_handler.h +++ b/src/shrpx_connection_handler.h @@ -32,9 +32,6 @@ #include #include -#ifndef NOTHREADS -#include -#endif // !NOTHREADS #include @@ -51,6 +48,22 @@ class Worker; struct WorkerStat; struct TicketKeys; +struct OCSPUpdateContext { + // ocsp response buffer + std::vector resp; + // index to ConnectionHandler::all_ssl_ctx_, which points to next + // SSL_CTX to update ocsp response cache. + size_t next; + ev_child chldev; + ev_io rev; + // fd to read response from fetch-ocsp-response script + int fd; + // errno encountered while processing response + int error; + // pid of forked fetch-ocsp-response script process + pid_t pid; +}; + class ConnectionHandler { public: ConnectionHandler(struct ev_loop *loop); @@ -79,23 +92,25 @@ public: void set_graceful_shutdown(bool f); bool get_graceful_shutdown() const; void join_worker(); - // Updates OCSP response cache for all server side SSL_CTX object - void update_ocsp(); - // Just like update_ocsp(), but performed in new thread. Call - // handle_ocsp_completion() to handle its completion and scheduling - // next update. - void update_ocsp_async(); - // Handles asynchronous OCSP update completion and schedules next + + // Cancels ocsp update process + void cancel_ocsp_update(); + // Starts ocsp update for certficate |cert_file|. + int start_ocsp_update(const char *cert_file); + // Reads incoming data from ocsp update process + void read_ocsp_chunk(); + // Handles the completion of one ocsp update + void handle_ocsp_complete(); + // Resets ocsp_; + void reset_ocsp(); + // Proceeds to the next certificate's ocsp update. If all + // certificates' ocsp update has been done, schedule next ocsp // update. - void handle_ocsp_completion(); - // Waits for OCSP thread finishes if it is still running. - void join_ocsp_thread(); + void proceed_next_cert_ocsp(); private: -#ifndef NOTHREADS - std::future ocsp_result_; -#endif // !NOTHREADS std::vector all_ssl_ctx_; + OCSPUpdateContext ocsp_; // Worker instances when multi threaded mode (-nN, N >= 2) is used. std::vector> workers_; // Worker instance used when single threaded mode (-n1) is used. diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index d7a55a97..cb27749a 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -29,10 +29,6 @@ #include #include #include -#include -#ifndef NOTHREADS -#include -#endif // !NOTHREADS #include #include @@ -1025,114 +1021,6 @@ CertLookupTree *create_cert_lookup_tree() { return new ssl::CertLookupTree(); } -namespace { -// inspired by h2o_read_command function from h2o project: -// https://github.com/h2o/h2o -int exec_read_stdout(std::vector &out, char *const *argv, - char *const *envp) { -#ifndef NOTHREADS - int rv; - int pfd[2]; - -#ifdef O_CLOEXEC - if (pipe2(pfd, O_CLOEXEC) == -1) { - return -1; - } -#else // !O_CLOEXEC - if (pipe(pfd) == -1) { - return -1; - } - util::make_socket_closeonexec(pfd[0]); - util::make_socket_closeonexec(pfd[1]); -#endif // !O_CLOEXEC - - auto closer = defer([pfd]() { - close(pfd[0]); - - if (pfd[1] != -1) { - close(pfd[1]); - } - }); - - // posix_spawn family functions are really interesting. They makes - // fork + dup2 + execve pattern easier. - - posix_spawn_file_actions_t file_actions; - - if (posix_spawn_file_actions_init(&file_actions) != 0) { - return -1; - } - - auto file_actions_del = - defer(posix_spawn_file_actions_destroy, &file_actions); - - if (posix_spawn_file_actions_adddup2(&file_actions, pfd[1], 1) != 0) { - return -1; - } - - if (posix_spawn_file_actions_addclose(&file_actions, pfd[0]) != 0) { - return -1; - } - - pid_t pid; - rv = posix_spawn(&pid, argv[0], &file_actions, nullptr, argv, envp); - if (rv != 0) { - LOG(WARN) << "Cannot execute ocsp query command: " << argv[0] - << ", errno=" << rv; - return -1; - } - - close(pfd[1]); - pfd[1] = -1; - - std::array buf; - for (;;) { - ssize_t n; - while ((n = read(pfd[0], buf.data(), buf.size())) == -1 && errno == EINTR) - ; - - if (n == -1) { - auto error = errno; - LOG(WARN) << "Reading from ocsp query command failed: errno=" << error; - return -1; - } - - if (n == 0) { - break; - } - - std::copy_n(std::begin(buf), n, std::back_inserter(out)); - } - - int status; - if (waitpid(pid, &status, 0) == -1) { - auto error = errno; - LOG(WARN) << "waitpid for ocsp query command failed: errno=" << error; - return -1; - } - - if (!WIFEXITED(status)) { - LOG(WARN) << "ocsp query command did not exit normally: " << status; - return -1; - } -#endif // !NOTHREADS - return 0; -} -} // namespace - -int get_ocsp_response(std::vector &out, const char *cert_file) { - char *const argv[] = { - const_cast(get_config()->fetch_ocsp_response_file.get()), - const_cast(cert_file), nullptr}; - char *const envp[] = {nullptr}; - - if (exec_read_stdout(out, argv, envp) != 0 || out.empty()) { - return -1; - } - - return 0; -} - } // namespace ssl } // namespace shrpx diff --git a/src/shrpx_ssl.h b/src/shrpx_ssl.h index 8888723c..5f808cb2 100644 --- a/src/shrpx_ssl.h +++ b/src/shrpx_ssl.h @@ -170,8 +170,6 @@ SSL_CTX *setup_client_ssl_context(); // this function returns nullptr. CertLookupTree *create_cert_lookup_tree(); -int get_ocsp_response(std::vector &out, const char *cert_file); - } // namespace ssl } // namespace shrpx From 09c485e71258721ba6c792289087541d8b3c4ddb Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 9 Apr 2015 00:43:03 +0900 Subject: [PATCH 02/50] nghttpx: Eliminate 1 second refresh timer --- src/shrpx.cc | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 20b9456b..d7bd884b 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -555,7 +555,9 @@ void graceful_shutdown_signal_cb(struct ev_loop *loop, ev_signal *w, conn_handler->graceful_shutdown_worker(); - if (get_config()->num_worker == 1) { + if (get_config()->num_worker == 1 && + conn_handler->get_single_worker()->get_worker_stat()->num_connections > + 0) { return; } @@ -565,20 +567,6 @@ void graceful_shutdown_signal_cb(struct ev_loop *loop, ev_signal *w, } } // namespace -namespace { -void refresh_cb(struct ev_loop *loop, ev_timer *w, int revents) { - auto conn_handler = static_cast(w->data); - auto worker = conn_handler->get_single_worker(); - - // In multi threaded mode (get_config()->num_worker > 1), we have to - // wait for event notification to workers to finish. - if (get_config()->num_worker == 1 && conn_handler->get_graceful_shutdown() && - (!worker || worker->get_worker_stat()->num_connections == 0)) { - ev_break(loop); - } -} -} // namespace - namespace { void renew_ticket_key_cb(struct ev_loop *loop, ev_timer *w, int revents) { auto conn_handler = static_cast(w->data); @@ -740,11 +728,6 @@ int event_loop() { graceful_shutdown_sig.data = conn_handler.get(); ev_signal_start(loop, &graceful_shutdown_sig); - ev_timer refresh_timer; - ev_timer_init(&refresh_timer, refresh_cb, 0., 1.); - refresh_timer.data = conn_handler.get(); - ev_timer_again(loop, &refresh_timer); - if (!get_config()->upstream_no_tls && !get_config()->no_ocsp) { conn_handler->proceed_next_cert_ocsp(); } From bc53c8161603518360bbc73803519de5391823a9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 9 Apr 2015 00:55:22 +0900 Subject: [PATCH 03/50] nghttpx: Replace posix_spawn functions with fork + dup2 + execve Although posx_spawn is very convenient and useful, we have platform which don't have these functions (e.g., Android NDK r10d). --- src/shrpx_connection_handler.cc | 43 +++++++++++++++------------------ 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index 1021929d..c325e1ae 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -30,9 +30,6 @@ #include #include -#ifndef NOTHREADS -#include -#endif // !NOTHREADS #include "shrpx_client_handler.h" #include "shrpx_ssl.h" @@ -392,36 +389,34 @@ int ConnectionHandler::start_ocsp_update(const char *cert_file) { } }); - // posix_spawn family functions are really interesting. They makes - // fork + dup2 + execve pattern easier. - - posix_spawn_file_actions_t file_actions; - - if (posix_spawn_file_actions_init(&file_actions) != 0) { + auto pid = fork(); + if (pid == -1) { + auto error = errno; + LOG(WARN) << "Could not execute ocsp query command: " << argv[0] + << ", fork() failed, errno=" << error; return -1; } - auto file_actions_del = - defer(posix_spawn_file_actions_destroy, &file_actions); + if (pid == 0) { + // child process + dup2(pfd[1], 1); + close(pfd[0]); - if (posix_spawn_file_actions_adddup2(&file_actions, pfd[1], 1) != 0) { - return -1; - } - - if (posix_spawn_file_actions_addclose(&file_actions, pfd[0]) != 0) { - return -1; - } - - rv = posix_spawn(&ocsp_.pid, argv[0], &file_actions, nullptr, argv, envp); - if (rv != 0) { - LOG(WARN) << "Cannot execute ocsp query command: " << argv[0] - << ", errno=" << rv; - return -1; + rv = execve(argv[0], argv, envp); + if (rv == -1) { + auto error = errno; + LOG(WARN) << "Could not execute ocsp query command: " << argv[0] + << ", execve() faild, errno=" << error; + _Exit(EXIT_FAILURE); + } + // unreachable } + // parent process close(pfd[1]); pfd[1] = -1; + ocsp_.pid = pid; ocsp_.fd = pfd[0]; pfd[0] = -1; From b8739308025edded01a96f97a73d822dbafc840f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 9 Apr 2015 00:59:43 +0900 Subject: [PATCH 04/50] nghttpx: Now ocsp works without threads --- src/shrpx.cc | 7 ------- src/shrpx_connection_handler.cc | 3 +-- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index d7bd884b..e225a121 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -2015,12 +2015,6 @@ int main(int argc, char **argv) { } if (!get_config()->upstream_no_tls && !get_config()->no_ocsp) { -#ifdef NOTHREADS - mod_config()->no_ocsp = true; - LOG(WARN) - << "OCSP stapling has been disabled since it requires threading but" - "threading disabled at build time."; -#else // !NOTHREADS struct stat buf; if (stat(get_config()->fetch_ocsp_response_file.get(), &buf) != 0) { mod_config()->no_ocsp = true; @@ -2028,7 +2022,6 @@ int main(int argc, char **argv) { << get_config()->fetch_ocsp_response_file.get() << " not found. OCSP stapling has been disabled."; } -#endif // !NOTHREADS } if (get_config()->downstream_addrs.empty()) { diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index c325e1ae..1fdbc867 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -355,7 +355,6 @@ void ConnectionHandler::cancel_ocsp_update() { // inspired by h2o_read_command function from h2o project: // https://github.com/h2o/h2o int ConnectionHandler::start_ocsp_update(const char *cert_file) { -#ifndef NOTHREADS int rv; int pfd[2]; @@ -426,7 +425,7 @@ int ConnectionHandler::start_ocsp_update(const char *cert_file) { ev_child_set(&ocsp_.chldev, ocsp_.pid, 0); ev_child_start(loop_, &ocsp_.chldev); -#endif // !NOTHREADS + return 0; } From 5c426877595c7b2e365594cc3f529935c80a0186 Mon Sep 17 00:00:00 2001 From: Alexis La Goutte Date: Thu, 9 Apr 2015 15:44:36 +0200 Subject: [PATCH 05/50] fix comma at end of enumerator list [-Wpedantic] --- lib/nghttp2_int.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/nghttp2_int.h b/lib/nghttp2_int.h index 3d77eaab..60070176 100644 --- a/lib/nghttp2_int.h +++ b/lib/nghttp2_int.h @@ -51,7 +51,7 @@ typedef enum { * Invalid HTTP header field was received but it can be treated as * if it was not received because of compatibility reasons. */ - NGHTTP2_ERR_IGN_HTTP_HEADER = -105, + NGHTTP2_ERR_IGN_HTTP_HEADER = -105 } nghttp2_internal_error; #endif /* NGHTTP2_INT_H */ From 6ca24264e4139619c2a9a8407a2e082ceb14a618 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 9 Apr 2015 23:30:12 +0900 Subject: [PATCH 06/50] Enable PIE for Android build --- android-config | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/android-config b/android-config index 21d269d8..9828822e 100755 --- a/android-config +++ b/android-config @@ -42,6 +42,6 @@ PATH=$TOOLCHAIN/bin:$PATH --enable-werror \ CC=clang \ CXX=clang++ \ - CPPFLAGS="-I$PREFIX/include" \ + CPPFLAGS="-fPIE -I$PREFIX/include" \ PKG_CONFIG_LIBDIR="$PREFIX/lib/pkgconfig" \ - LDFLAGS="-L$PREFIX/lib" + LDFLAGS="-fPIE -pie -L$PREFIX/lib" From 5334cb5acc668a630d83b99b429931fbb3cf7b56 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 9 Apr 2015 23:34:42 +0900 Subject: [PATCH 07/50] Enable PIE in Dockerfile.android too --- Dockerfile.android | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile.android b/Dockerfile.android index 6404651d..87990f0a 100644 --- a/Dockerfile.android +++ b/Dockerfile.android @@ -101,9 +101,9 @@ RUN autoreconf -i && \ --disable-threads \ LIBSPDYLAY_CFLAGS=-I$PREFIX/usr/local/include \ LIBSPDYLAY_LIBS="-L$PREFIX/usr/local/lib -lspdylay" \ - CPPFLAGS="-I$PREFIX/include" \ + CPPFLAGS="-fPIE -I$PREFIX/include" \ CXXFLAGS="-fno-strict-aliasing" \ PKG_CONFIG_LIBDIR="$PREFIX/lib/pkgconfig" \ - LDFLAGS="-L$PREFIX/lib" && \ + LDFLAGS="-fPIE -pie -L$PREFIX/lib" && \ make && \ arm-linux-androideabi-strip src/nghttpx src/nghttpd src/nghttp From 69a4f3bf4206b10cc26a1a1315b61097e420f8e9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 00:15:01 +0900 Subject: [PATCH 08/50] nghttp: Consider :authority custom header field for SNI --- src/nghttp.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/nghttp.cc b/src/nghttp.cc index fa57a99e..9329d436 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -531,12 +531,13 @@ int HttpClient::initiate_connection() { SSL_set_fd(ssl, fd); SSL_set_connect_state(ssl); - // If the user overrode the host header, use that value for - // the SNI extension + // If the user overrode the :authority or host header, use that + // value for the SNI extension const char *host_string = nullptr; - auto i = - std::find_if(std::begin(config.headers), std::end(config.headers), - [](const Header &nv) { return "host" == nv.name; }); + auto i = std::find_if(std::begin(config.headers), + std::end(config.headers), [](const Header &nv) { + return ":authority" == nv.name || "host" == nv.name; + }); if (i != std::end(config.headers)) { host_string = (*i).value.c_str(); } else { From 44b4cda2007ab01e4cc8e90577de04ffff25fc1c Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 00:21:31 +0900 Subject: [PATCH 09/50] src: Check return value from nghttp2_session_get_stream_user_data --- src/h2load_http2_session.cc | 2 +- src/shrpx_http2_upstream.cc | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/h2load_http2_session.cc b/src/h2load_http2_session.cc index bc658380..2cdba9e1 100644 --- a/src/h2load_http2_session.cc +++ b/src/h2load_http2_session.cc @@ -119,7 +119,7 @@ ssize_t file_read_callback(nghttp2_session *session, int32_t stream_id, auto config = client->worker->config; auto req_stat = static_cast( nghttp2_session_get_stream_user_data(session, stream_id)); - + assert(req_stat); ssize_t nread; while ((nread = pread(config->data_fd, buf, length, req_stat->data_offset)) == -1 && diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 10791f27..1e9bbda0 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -500,6 +500,10 @@ int on_frame_send_callback(nghttp2_session *session, const nghttp2_frame *frame, auto downstream = static_cast( nghttp2_session_get_stream_user_data(session, stream_id)); + if (!downstream) { + return 0; + } + // For tunneling, issue RST_STREAM to finish the stream. if (downstream->get_upgraded() || nghttp2_session_get_stream_remote_close(session, stream_id) == 0) { From 6ed710adbd240e6ae75dc2f559d0547d3b34f019 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 00:31:46 +0900 Subject: [PATCH 10/50] Bump up version number to 0.7.11 --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 31317479..61bae087 100644 --- a/configure.ac +++ b/configure.ac @@ -25,7 +25,7 @@ dnl Do not change user variables! dnl http://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html AC_PREREQ(2.61) -AC_INIT([nghttp2], [0.7.11-DEV], [t-tujikawa@users.sourceforge.net]) +AC_INIT([nghttp2], [0.7.11], [t-tujikawa@users.sourceforge.net]) LT_PREREQ([2.2.6]) LT_INIT() dnl See versioning rule: From 03746884a4492e8a219e008c7b3c4b88f9e50f80 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 00:35:00 +0900 Subject: [PATCH 11/50] Update man pages --- doc/h2load.1 | 2 +- doc/nghttp.1 | 2 +- doc/nghttpd.1 | 2 +- doc/nghttpx.1 | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/h2load.1 b/doc/h2load.1 index 3fed07cd..399cc7b0 100644 --- a/doc/h2load.1 +++ b/doc/h2load.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "H2LOAD" "1" "April 08, 2015" "0.7.10" "nghttp2" +.TH "H2LOAD" "1" "April 10, 2015" "0.7.11" "nghttp2" .SH NAME h2load \- HTTP/2 benchmarking tool . diff --git a/doc/nghttp.1 b/doc/nghttp.1 index ba533a30..000b3fde 100644 --- a/doc/nghttp.1 +++ b/doc/nghttp.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTP" "1" "April 08, 2015" "0.7.10" "nghttp2" +.TH "NGHTTP" "1" "April 10, 2015" "0.7.11" "nghttp2" .SH NAME nghttp \- HTTP/2 experimental client . diff --git a/doc/nghttpd.1 b/doc/nghttpd.1 index 67337d8d..7684de21 100644 --- a/doc/nghttpd.1 +++ b/doc/nghttpd.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPD" "1" "April 08, 2015" "0.7.10" "nghttp2" +.TH "NGHTTPD" "1" "April 10, 2015" "0.7.11" "nghttp2" .SH NAME nghttpd \- HTTP/2 experimental server . diff --git a/doc/nghttpx.1 b/doc/nghttpx.1 index 5ceda445..e309995f 100644 --- a/doc/nghttpx.1 +++ b/doc/nghttpx.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPX" "1" "April 08, 2015" "0.7.10" "nghttp2" +.TH "NGHTTPX" "1" "April 10, 2015" "0.7.11" "nghttp2" .SH NAME nghttpx \- HTTP/2 experimental proxy . From cbfa0210956f46d9b57709a63f94e8f1f5d63bfc Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 00:38:17 +0900 Subject: [PATCH 12/50] Bump up version number to 0.7.12-DEV --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 61bae087..ca2eddb7 100644 --- a/configure.ac +++ b/configure.ac @@ -25,7 +25,7 @@ dnl Do not change user variables! dnl http://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html AC_PREREQ(2.61) -AC_INIT([nghttp2], [0.7.11], [t-tujikawa@users.sourceforge.net]) +AC_INIT([nghttp2], [0.7.12-DEV], [t-tujikawa@users.sourceforge.net]) LT_PREREQ([2.2.6]) LT_INIT() dnl See versioning rule: From 9803f92e9c8a5649efdd82c621921d5db1ef4f19 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 02:40:09 +0900 Subject: [PATCH 13/50] nghttpx: Set Downstream to stream user data on HTTP Upgrade to h2 --- src/shrpx_http2_upstream.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 1e9bbda0..a3179b9f 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -163,6 +163,8 @@ int Http2Upstream::upgrade_upstream(HttpsUpstream *http) { downstream->set_request_http2_scheme(scheme); auto ptr = downstream.get(); + + nghttp2_session_set_stream_user_data(session_, 1, ptr); downstream_queue_.add_pending(std::move(downstream)); downstream_queue_.mark_active(ptr); From 87cadca3d8aec93afa1e8c1177abd92d64204d47 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 21:28:12 +0900 Subject: [PATCH 14/50] integration: Add HTTP Upgrade test --- integration-tests/nghttpx_http2_test.go | 33 +++++++ integration-tests/server_tester.go | 120 ++++++++++++------------ 2 files changed, 94 insertions(+), 59 deletions(-) diff --git a/integration-tests/nghttpx_http2_test.go b/integration-tests/nghttpx_http2_test.go index 909816c3..89cd353a 100644 --- a/integration-tests/nghttpx_http2_test.go +++ b/integration-tests/nghttpx_http2_test.go @@ -558,6 +558,39 @@ func TestH2H1RequestTrailer(t *testing.T) { } } +// TestH2H1Upgrade tests HTTP Upgrade to HTTP/2 +func TestH2H1Upgrade(t *testing.T) { + st := newServerTester(nil, t, func(w http.ResponseWriter, r *http.Request) {}) + defer st.Close() + + res, err := st.http1(requestParam{ + name: "TestH2H1Upgrade", + header: []hpack.HeaderField{ + pair("Connection", "Upgrade, HTTP2-Settings"), + pair("Upgrade", "h2c-14"), + pair("HTTP2-Settings", "AAMAAABkAAQAAP__"), + }, + }) + + if err != nil { + t.Fatalf("Error st.http1() = %v", err) + } + + if got, want := res.status, 101; got != want { + t.Errorf("res.status: %v; want %v", got, want) + } + + res, err = st.http2(requestParam{ + httpUpgrade: true, + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("res.status: %v; want %v", got, want) + } +} + // TestH2H1GracefulShutdown tests graceful shutdown. func TestH2H1GracefulShutdown(t *testing.T) { st := newServerTester(nil, t, noopHandler) diff --git a/integration-tests/server_tester.go b/integration-tests/server_tester.go index 58a3051a..3a4b5568 100644 --- a/integration-tests/server_tester.go +++ b/integration-tests/server_tester.go @@ -247,15 +247,16 @@ func (st *serverTester) readSpdyFrame() (spdy.Frame, error) { } type requestParam struct { - name string // name for this request to identify the request in log easily - streamID uint32 // stream ID, automatically assigned if 0 - method string // method, defaults to GET - scheme string // scheme, defaults to http - authority string // authority, defaults to backend server address - path string // path, defaults to / - header []hpack.HeaderField // additional request header fields - body []byte // request body - trailer []hpack.HeaderField // trailer part + name string // name for this request to identify the request in log easily + streamID uint32 // stream ID, automatically assigned if 0 + method string // method, defaults to GET + scheme string // scheme, defaults to http + authority string // authority, defaults to backend server address + path string // path, defaults to / + header []hpack.HeaderField // additional request header fields + body []byte // request body + trailer []hpack.HeaderField // trailer part + httpUpgrade bool // true if upgraded to HTTP/2 through HTTP Upgrade } // wrapper for request body to set trailer part @@ -478,69 +479,70 @@ func (st *serverTester) http2(rp requestParam) (*serverResponse, error) { streams := make(map[uint32]*serverResponse) streams[id] = res - method := "GET" - if rp.method != "" { - method = rp.method - } - _ = st.enc.WriteField(pair(":method", method)) - - scheme := "http" - if rp.scheme != "" { - scheme = rp.scheme - } - _ = st.enc.WriteField(pair(":scheme", scheme)) - - authority := st.authority - if rp.authority != "" { - authority = rp.authority - } - _ = st.enc.WriteField(pair(":authority", authority)) - - path := "/" - if rp.path != "" { - path = rp.path - } - _ = st.enc.WriteField(pair(":path", path)) - - _ = st.enc.WriteField(pair("test-case", rp.name)) - - for _, h := range rp.header { - _ = st.enc.WriteField(h) - } - - err := st.fr.WriteHeaders(http2.HeadersFrameParam{ - StreamID: id, - EndStream: len(rp.body) == 0 && len(rp.trailer) == 0, - EndHeaders: true, - BlockFragment: st.headerBlkBuf.Bytes(), - }) - if err != nil { - return nil, err - } - - if len(rp.body) != 0 { - // TODO we assume rp.body fits in 1 frame - if err := st.fr.WriteData(id, len(rp.trailer) == 0, rp.body); err != nil { - return nil, err + if !rp.httpUpgrade { + method := "GET" + if rp.method != "" { + method = rp.method } - } + _ = st.enc.WriteField(pair(":method", method)) - if len(rp.trailer) != 0 { - st.headerBlkBuf.Reset() - for _, h := range rp.trailer { + scheme := "http" + if rp.scheme != "" { + scheme = rp.scheme + } + _ = st.enc.WriteField(pair(":scheme", scheme)) + + authority := st.authority + if rp.authority != "" { + authority = rp.authority + } + _ = st.enc.WriteField(pair(":authority", authority)) + + path := "/" + if rp.path != "" { + path = rp.path + } + _ = st.enc.WriteField(pair(":path", path)) + + _ = st.enc.WriteField(pair("test-case", rp.name)) + + for _, h := range rp.header { _ = st.enc.WriteField(h) } + err := st.fr.WriteHeaders(http2.HeadersFrameParam{ StreamID: id, - EndStream: true, + EndStream: len(rp.body) == 0 && len(rp.trailer) == 0, EndHeaders: true, BlockFragment: st.headerBlkBuf.Bytes(), }) if err != nil { return nil, err } - } + if len(rp.body) != 0 { + // TODO we assume rp.body fits in 1 frame + if err := st.fr.WriteData(id, len(rp.trailer) == 0, rp.body); err != nil { + return nil, err + } + } + + if len(rp.trailer) != 0 { + st.headerBlkBuf.Reset() + for _, h := range rp.trailer { + _ = st.enc.WriteField(h) + } + err := st.fr.WriteHeaders(http2.HeadersFrameParam{ + StreamID: id, + EndStream: true, + EndHeaders: true, + BlockFragment: st.headerBlkBuf.Bytes(), + }) + if err != nil { + return nil, err + } + } + } loop: for { fr, err := st.readFrame() From 97366bf55c91e568b555c39c029bca82920a36ab Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 22:10:51 +0900 Subject: [PATCH 15/50] nghttpx: Set content-length after complete request/response headers --- src/shrpx_http2_session.cc | 14 ++++++++------ src/shrpx_http2_upstream.cc | 14 ++++++++------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index a73c91c5..0e31a0f9 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -757,12 +757,6 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, auto token = http2::lookup_token(name, namelen); - if (token == http2::HD_CONTENT_LENGTH) { - // libnghttp2 guarantees this can be parsed - auto len = util::parse_uint(value, valuelen); - downstream->set_response_content_length(len); - } - downstream->add_response_header(name, namelen, value, valuelen, flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0; @@ -844,6 +838,14 @@ int on_response_headers(Http2Session *http2session, Downstream *downstream, return 0; } + auto content_length = + downstream->get_response_header(http2::HD_CONTENT_LENGTH); + if (content_length) { + // libnghttp2 guarantees this can be parsed + auto len = util::parse_uint(content_length->value); + downstream->set_response_content_length(len); + } + if (downstream->get_response_content_length() == -1 && downstream->expect_response_body()) { // Here we have response body but Content-Length is not known in diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index a3179b9f..fcb91829 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -233,12 +233,6 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, auto token = http2::lookup_token(name, namelen); - if (token == http2::HD_CONTENT_LENGTH) { - // libnghttp2 guarantees this can be parsed - auto len = util::parse_uint(value, valuelen); - downstream->set_request_content_length(len); - } - downstream->add_request_header(name, namelen, value, valuelen, flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0; @@ -300,6 +294,14 @@ int Http2Upstream::on_request_headers(Downstream *downstream, http2::dump_nv(get_config()->http2_upstream_dump_request_header, nva); } + auto content_length = + downstream->get_request_header(http2::HD_CONTENT_LENGTH); + if (content_length) { + // libnghttp2 guarantees this can be parsed + auto len = util::parse_uint(content_length->value); + downstream->set_request_content_length(len); + } + auto authority = downstream->get_request_header(http2::HD__AUTHORITY); auto path = downstream->get_request_header(http2::HD__PATH); auto method = downstream->get_request_header(http2::HD__METHOD); From 308738025c78e1ebc4bdf9f09e13fa9e0e1de919 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 22:23:46 +0900 Subject: [PATCH 16/50] nghttpx: Don't set response content-length if HTTP/2 response upgraded --- src/shrpx_http2_session.cc | 60 ++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 0e31a0f9..89e12dd6 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -838,35 +838,9 @@ int on_response_headers(Http2Session *http2session, Downstream *downstream, return 0; } - auto content_length = - downstream->get_response_header(http2::HD_CONTENT_LENGTH); - if (content_length) { - // libnghttp2 guarantees this can be parsed - auto len = util::parse_uint(content_length->value); - downstream->set_response_content_length(len); - } - - if (downstream->get_response_content_length() == -1 && - downstream->expect_response_body()) { - // Here we have response body but Content-Length is not known in - // advance. - if (downstream->get_request_major() <= 0 || - (downstream->get_request_major() <= 1 && - downstream->get_request_minor() <= 0)) { - // We simply close connection for pre-HTTP/1.1 in this case. - downstream->set_response_connection_close(true); - } else if (downstream->get_request_method() != "CONNECT") { - // Otherwise, use chunked encoding to keep upstream connection - // open. In HTTP2, we are supporsed not to receive - // transfer-encoding. - downstream->add_response_header("transfer-encoding", "chunked", - http2::HD_TRANSFER_ENCODING); - downstream->set_chunked_response(true); - } - } - downstream->set_response_state(Downstream::HEADER_COMPLETE); downstream->check_upgrade_fulfilled(); + if (downstream->get_upgraded()) { downstream->set_response_connection_close(true); // On upgrade sucess, both ends can send data @@ -880,11 +854,35 @@ int on_response_headers(Http2Session *http2session, Downstream *downstream, SSLOG(INFO, http2session) << "HTTP upgrade success. stream_id=" << frame->hd.stream_id; } - } else if (downstream->get_request_method() == "CONNECT") { - // If request is CONNECT, terminate request body to avoid for - // stream to stall. - downstream->end_upload_data(); + } else { + auto content_length = + downstream->get_response_header(http2::HD_CONTENT_LENGTH); + if (content_length) { + // libnghttp2 guarantees this can be parsed + auto len = util::parse_uint(content_length->value); + downstream->set_response_content_length(len); + } + + if (downstream->get_response_content_length() == -1 && + downstream->expect_response_body()) { + // Here we have response body but Content-Length is not known in + // advance. + if (downstream->get_request_major() <= 0 || + (downstream->get_request_major() == 1 && + downstream->get_request_minor() == 0)) { + // We simply close connection for pre-HTTP/1.1 in this case. + downstream->set_response_connection_close(true); + } else { + // Otherwise, use chunked encoding to keep upstream connection + // open. In HTTP2, we are supporsed not to receive + // transfer-encoding. + downstream->add_response_header("transfer-encoding", "chunked", + http2::HD_TRANSFER_ENCODING); + downstream->set_chunked_response(true); + } + } } + rv = upstream->on_downstream_header_complete(downstream); if (rv != 0) { http2session->submit_rst_stream(frame->hd.stream_id, From 095bc178f3072288b8e8c58a06714ac2edb3fc58 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 22:30:20 +0900 Subject: [PATCH 17/50] nghttpx: Robust HTTP/1 backend CL and TE handling We should ignore Content-Length and Transfer-Encoding for upgraded response, and reset content-length if this is a non-final response. --- src/shrpx_http_downstream_connection.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index e1b90383..329fdce5 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -520,6 +520,9 @@ int htp_hdrs_completecb(http_parser *htp) { } if (downstream->get_non_final_response()) { + // Reset content-length because we reuse same Downstream for the + // next response. + downstream->set_response_content_length(-1); // For non-final response code, we just call // on_downstream_header_complete() without changing response // state. @@ -537,7 +540,11 @@ int htp_hdrs_completecb(http_parser *htp) { downstream->inspect_http1_response(); downstream->check_upgrade_fulfilled(); if (downstream->get_upgraded()) { + // content-length must be ignored for upgraded connection. + downstream->set_response_content_length(-1); downstream->set_response_connection_close(true); + // transfer-encoding not applied to upgraded connection + downstream->set_chunked_response(false); } if (upstream->on_downstream_header_complete(downstream) != 0) { return -1; From 14d4979c547dd9e5a383a8f4453c575f142ddfb9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Apr 2015 23:11:40 +0900 Subject: [PATCH 18/50] Don't install libnghttp2_asio headers if they are disabled --- src/includes/Makefile.am | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/includes/Makefile.am b/src/includes/Makefile.am index d711dddb..abc93c1e 100644 --- a/src/includes/Makefile.am +++ b/src/includes/Makefile.am @@ -20,5 +20,8 @@ # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +if ENABLE_ASIO_LIB nobase_include_HEADERS = nghttp2/asio_http2.h nghttp2/asio_http2_client.h \ nghttp2/asio_http2_server.h +endif # ENABLE_ASIO_LIB From 889e705f35b3f382509ee302fc57d55aead91ba2 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 11 Apr 2015 00:08:28 +0900 Subject: [PATCH 19/50] nghttpx: Add logging for somewhat important events (logs, tickets, and ocsp) --- src/shrpx.cc | 9 +++------ src/shrpx_connection_handler.cc | 31 +++++++++++++++++++------------ src/shrpx_worker.cc | 8 ++------ 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index e225a121..6d9e1a7a 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -417,9 +417,7 @@ namespace { void reopen_log_signal_cb(struct ev_loop *loop, ev_signal *w, int revents) { auto conn_handler = static_cast(w->data); - if (LOG_ENABLED(INFO)) { - LOG(INFO) << "Reopening log files: main"; - } + LOG(NOTICE) << "Reopening log files: main"; (void)reopen_log_files(); redirect_stderr_to_errorlog(); @@ -573,9 +571,8 @@ void renew_ticket_key_cb(struct ev_loop *loop, ev_timer *w, int revents) { const auto &old_ticket_keys = conn_handler->get_ticket_keys(); auto ticket_keys = std::make_shared(); - if (LOG_ENABLED(INFO)) { - LOG(INFO) << "renew ticket key"; - } + LOG(NOTICE) << "Renew ticket keys: main"; + // We store at most 2 ticket keys if (old_ticket_keys) { auto &old_keys = old_ticket_keys->keys; diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index 1fdbc867..8779779d 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -69,6 +69,8 @@ void ocsp_cb(struct ev_loop *loop, ev_timer *w, int revent) { return; } + LOG(NOTICE) << "Start ocsp update"; + h->proceed_next_cert_ocsp(); } } // namespace @@ -358,6 +360,10 @@ int ConnectionHandler::start_ocsp_update(const char *cert_file) { int rv; int pfd[2]; + if (LOG_ENABLED(INFO)) { + LOG(INFO) << "Start ocsp update for " << cert_file; + } + assert(!ev_is_active(&ocsp_.rev)); assert(!ev_is_active(&ocsp_.chldev)); @@ -391,8 +397,8 @@ int ConnectionHandler::start_ocsp_update(const char *cert_file) { auto pid = fork(); if (pid == -1) { auto error = errno; - LOG(WARN) << "Could not execute ocsp query command: " << argv[0] - << ", fork() failed, errno=" << error; + LOG(WARN) << "Could not execute ocsp query command for " << cert_file + << ": " << argv[0] << ", fork() failed, errno=" << error; return -1; } @@ -461,22 +467,23 @@ void ConnectionHandler::handle_ocsp_complete() { ev_io_stop(loop_, &ocsp_.rev); ev_child_stop(loop_, &ocsp_.chldev); - auto rstatus = ocsp_.chldev.rstatus; - auto status = WEXITSTATUS(rstatus); - if (ocsp_.error || !WIFEXITED(rstatus) || status != 0) { - LOG(WARN) << "ocsp query command failed: error=" << ocsp_.error - << ", rstatus=" << rstatus << ", status=" << status; - ++ocsp_.next; - proceed_next_cert_ocsp(); - return; - } - assert(ocsp_.next < all_ssl_ctx_.size()); auto ssl_ctx = all_ssl_ctx_[ocsp_.next]; auto tls_ctx_data = static_cast(SSL_CTX_get_app_data(ssl_ctx)); + auto rstatus = ocsp_.chldev.rstatus; + auto status = WEXITSTATUS(rstatus); + if (ocsp_.error || !WIFEXITED(rstatus) || status != 0) { + LOG(WARN) << "ocsp query command for " << tls_ctx_data->cert_file + << " failed: error=" << ocsp_.error << ", rstatus=" << rstatus + << ", status=" << status; + ++ocsp_.next; + proceed_next_cert_ocsp(); + return; + } + if (LOG_ENABLED(INFO)) { LOG(INFO) << "ocsp update for " << tls_ctx_data->cert_file << " finished successfully"; diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc index bbae73b1..6b2b658b 100644 --- a/src/shrpx_worker.cc +++ b/src/shrpx_worker.cc @@ -160,17 +160,13 @@ void Worker::process_events() { break; } case RENEW_TICKET_KEYS: - if (LOG_ENABLED(INFO)) { - WLOG(INFO, this) << "Renew ticket keys: worker(" << this << ")"; - } + WLOG(NOTICE, this) << "Renew ticket keys: worker(" << this << ")"; ticket_keys_ = wev.ticket_keys; break; case REOPEN_LOG: - if (LOG_ENABLED(INFO)) { - WLOG(INFO, this) << "Reopening log files: worker(" << this << ")"; - } + WLOG(NOTICE, this) << "Reopening log files: worker(" << this << ")"; reopen_log_files(); From 7451a73defc84376807e59420f89eed9686d1512 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 12 Apr 2015 17:42:25 +0900 Subject: [PATCH 20/50] nghttpx: Don't push resource if link header has non empty loadpolicy --- src/http2.cc | 47 +++++++++++++++++++++++++++++++++++------------ src/http2_test.cc | 13 +++++++++++++ 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/http2.cc b/src/http2.cc index 5fc50683..26e61cbf 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -718,6 +718,30 @@ InputIt skip_to_right_dquote(InputIt first, InputIt last) { } } // namespace +namespace { +// Returns true if link-param does not match pattern |pat| of length +// |patlen| or it has empty value (""). |pat| should be parmname +// followed by "=". +bool check_link_param_empty(const char *first, const char *last, + const char *pat, size_t patlen) { + if (first + patlen <= last) { + if (std::equal(pat, pat + patlen, first, util::CaseCmp())) { + // we only accept URI if pat is followd by "" (e.g., + // loadpolicy="") here. + if (first + patlen + 2 <= last) { + if (*(first + patlen) != '"' || *(first + patlen + 1) != '"') { + return false; + } + } else { + // here we got invalid production (anchor=") or anchor=? + return false; + } + } + } + return true; +} +} // namespace + namespace { std::pair parse_next_link_header_once(const char *first, const char *last) { @@ -836,18 +860,17 @@ parse_next_link_header_once(const char *first, const char *last) { // we have to reject URI if we have nonempty anchor parameter. static const char ANCHOR[] = "anchor="; static const size_t ANCHORLEN = sizeof(ANCHOR) - 1; - if (!ign && first + ANCHORLEN <= last) { - if (std::equal(ANCHOR, ANCHOR + ANCHORLEN, first, util::CaseCmp())) { - // we only accept URI if anchor="" here. - if (first + ANCHORLEN + 2 <= last) { - if (*(first + ANCHORLEN) != '"' || *(first + ANCHORLEN + 1) != '"') { - ign = true; - } - } else { - // here we got invalid production (anchor=") or anchor=? - ign = true; - } - } + if (!ign && !check_link_param_empty(first, last, ANCHOR, ANCHORLEN)) { + ign = true; + } + + // reject URI if we have non-empty loadpolicy. This could be + // tightened up to just pick up "next" or "insert". + static const char LOADPOLICY[] = "loadpolicy="; + static const size_t LOADPOLICYLEN = sizeof(LOADPOLICY) - 1; + if (!ign && + !check_link_param_empty(first, last, LOADPOLICY, LOADPOLICYLEN)) { + ign = true; } auto param_first = first; diff --git a/src/http2_test.cc b/src/http2_test.cc index 01a2bc50..e2975d1f 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -627,6 +627,19 @@ void test_http2_parse_link_header(void) { CU_ASSERT(1 == res.size()); CU_ASSERT(std::make_pair(&s[36], &s[39]) == res[0].uri); } + { + // With loadpolicy="next", url should be ignored + const char s[] = R"(; rel=preload; loadpolicy="next")"; + auto res = http2::parse_link_header(s, sizeof(s) - 1); + CU_ASSERT(0 == res.size()); + } + { + // url should be picked up if empty loadpolicy is specified + const char s[] = R"(; rel=preload; loadpolicy="")"; + auto res = http2::parse_link_header(s, sizeof(s) - 1); + CU_ASSERT(1 == res.size()); + CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + } { // case-insensitive match const char s[] = R"(; rel=preload; ANCHOR="#foo", ; )" From a8ea86cfe5c64182c8845fa0c266d6382a9a0b88 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 12 Apr 2015 17:51:23 +0900 Subject: [PATCH 21/50] src: constexpr --- src/http2.cc | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/http2.cc b/src/http2.cc index 26e61cbf..65b2cee3 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -779,11 +779,11 @@ parse_next_link_header_once(const char *first, const char *last) { // we expect link-param // rel can take several relations using quoted form. - static const char PLP[] = "rel=\""; - static const size_t PLPLEN = sizeof(PLP) - 1; + static constexpr char PLP[] = "rel=\""; + static constexpr size_t PLPLEN = sizeof(PLP) - 1; - static const char PLT[] = "preload"; - static const size_t PLTLEN = sizeof(PLT) - 1; + static constexpr char PLT[] = "preload"; + static constexpr size_t PLTLEN = sizeof(PLT) - 1; if (first + PLPLEN < last && *(first + PLPLEN - 1) == '"' && std::equal(PLP, PLP + PLPLEN, first, util::CaseCmp())) { // we have to search preload in whitespace separated list: @@ -828,8 +828,8 @@ parse_next_link_header_once(const char *first, const char *last) { } // we are only interested in rel=preload parameter. Others are // simply skipped. - static const char PL[] = "rel=preload"; - static const size_t PLLEN = sizeof(PL) - 1; + static constexpr char PL[] = "rel=preload"; + static constexpr size_t PLLEN = sizeof(PL) - 1; if (first + PLLEN == last) { if (std::equal(PL, PL + PLLEN, first, util::CaseCmp())) { ok = true; @@ -858,16 +858,16 @@ parse_next_link_header_once(const char *first, const char *last) { } } // we have to reject URI if we have nonempty anchor parameter. - static const char ANCHOR[] = "anchor="; - static const size_t ANCHORLEN = sizeof(ANCHOR) - 1; + static constexpr char ANCHOR[] = "anchor="; + static constexpr size_t ANCHORLEN = sizeof(ANCHOR) - 1; if (!ign && !check_link_param_empty(first, last, ANCHOR, ANCHORLEN)) { ign = true; } // reject URI if we have non-empty loadpolicy. This could be // tightened up to just pick up "next" or "insert". - static const char LOADPOLICY[] = "loadpolicy="; - static const size_t LOADPOLICYLEN = sizeof(LOADPOLICY) - 1; + static constexpr char LOADPOLICY[] = "loadpolicy="; + static constexpr size_t LOADPOLICYLEN = sizeof(LOADPOLICY) - 1; if (!ign && !check_link_param_empty(first, last, LOADPOLICY, LOADPOLICYLEN)) { ign = true; From 5c2ca28706304600625a832d74d7b1043455d322 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 13 Apr 2015 21:33:43 +0900 Subject: [PATCH 22/50] asio: client: Call error_cb on error occurred in do_read and do_write Fixes GH-207 --- src/asio_client_session_impl.cc | 24 ++++++++++++++++-------- src/asio_client_session_impl.h | 1 + 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/asio_client_session_impl.cc b/src/asio_client_session_impl.cc index 844b1957..6334c7ac 100644 --- a/src/asio_client_session_impl.cc +++ b/src/asio_client_session_impl.cc @@ -89,10 +89,7 @@ void session_impl::connected(tcp::resolver::iterator endpoint_it) { } void session_impl::not_connected(const boost::system::error_code &ec) { - auto &error_cb = on_error(); - if (error_cb) { - error_cb(ec); - } + call_error_cb(ec); } void session_impl::on_connect(connect_cb cb) { connect_cb_ = std::move(cb); } @@ -103,6 +100,14 @@ const connect_cb &session_impl::on_connect() const { return connect_cb_; } const error_cb &session_impl::on_error() const { return error_cb_; } +void session_impl::call_error_cb(const boost::system::error_code &ec) { + auto &error_cb = on_error(); + if (!error_cb) { + return; + } + error_cb(ec); +} + namespace { int on_begin_headers_callback(nghttp2_session *session, const nghttp2_frame *frame, void *user_data) { @@ -308,10 +313,7 @@ bool session_impl::setup_session() { auto rv = nghttp2_session_client_new(&session_, callbacks, this); if (rv != 0) { - auto &error_cb = on_error(); - if (error_cb) { - error_cb(make_error_code(static_cast(rv))); - } + call_error_cb(make_error_code(static_cast(rv))); return false; } @@ -528,6 +530,7 @@ void session_impl::do_read() { std::size_t bytes_transferred) { if (ec) { if (ec.value() == boost::asio::error::operation_aborted) { + call_error_cb(ec); shutdown_socket(); } return; @@ -540,6 +543,8 @@ void session_impl::do_read() { nghttp2_session_mem_recv(session_, rb_.data(), bytes_transferred); if (rv != static_cast(bytes_transferred)) { + call_error_cb(make_error_code( + static_cast(rv < 0 ? rv : NGHTTP2_ERR_PROTO))); shutdown_socket(); return; } @@ -577,6 +582,7 @@ void session_impl::do_write() { const uint8_t *data; auto n = nghttp2_session_mem_send(session_, &data); if (n < 0) { + call_error_cb(make_error_code(static_cast(n))); shutdown_socket(); return; } @@ -606,6 +612,8 @@ void session_impl::do_write() { write_socket([this](const boost::system::error_code &ec, std::size_t n) { if (ec) { + call_error_cb(ec); + shutdown_socket(); return; } diff --git a/src/asio_client_session_impl.h b/src/asio_client_session_impl.h index be77365b..88bbad83 100644 --- a/src/asio_client_session_impl.h +++ b/src/asio_client_session_impl.h @@ -97,6 +97,7 @@ protected: private: bool should_stop() const; bool setup_session(); + void call_error_cb(const boost::system::error_code &ec); boost::asio::io_service &io_service_; tcp::resolver resolver_; From 061732adf0261ed644d55d8e50c484850846cb92 Mon Sep 17 00:00:00 2001 From: Tatsuhiko Kubo Date: Wed, 15 Apr 2015 09:20:45 +0900 Subject: [PATCH 23/50] improved malloc error handlings. --- src/comp_helper.c | 3 +++ src/util.cc | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/src/comp_helper.c b/src/comp_helper.c index fe8bc2b5..d697c6a0 100644 --- a/src/comp_helper.c +++ b/src/comp_helper.c @@ -57,6 +57,9 @@ json_t *dump_header(const uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen) { json_t *nv_pair = json_object(); char *cname = malloc(namelen + 1); + if (cname == NULL) { + return NULL; + } memcpy(cname, name, namelen); cname[namelen] = '\0'; json_object_set_new(nv_pair, cname, json_pack("s#", value, valuelen)); diff --git a/src/util.cc b/src/util.cc index d08d8b5b..65d06487 100644 --- a/src/util.cc +++ b/src/util.cc @@ -737,10 +737,16 @@ char *get_exec_path(int argc, char **const argv, const char *cwd) { if (argv0[0] == '/') { path = static_cast(malloc(len + 1)); + if (path == NULL) { + return NULL; + } memcpy(path, argv0, len + 1); } else { auto cwdlen = strlen(cwd); path = static_cast(malloc(len + 1 + cwdlen + 1)); + if (path == NULL) { + return NULL; + } memcpy(path, cwd, cwdlen); path[cwdlen] = '/'; memcpy(path + cwdlen + 1, argv0, len + 1); From 59f8397659133de12993aecea810831a3b1acd39 Mon Sep 17 00:00:00 2001 From: Tatsuhiko Kubo Date: Wed, 15 Apr 2015 21:18:39 +0900 Subject: [PATCH 24/50] Use nullptr instead of NULL in C++. --- src/util.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util.cc b/src/util.cc index 65d06487..acaf1f9f 100644 --- a/src/util.cc +++ b/src/util.cc @@ -737,15 +737,15 @@ char *get_exec_path(int argc, char **const argv, const char *cwd) { if (argv0[0] == '/') { path = static_cast(malloc(len + 1)); - if (path == NULL) { - return NULL; + if (path == nullptr) { + return nullptr; } memcpy(path, argv0, len + 1); } else { auto cwdlen = strlen(cwd); path = static_cast(malloc(len + 1 + cwdlen + 1)); - if (path == NULL) { - return NULL; + if (path == nullptr) { + return nullptr; } memcpy(path, cwd, cwdlen); path[cwdlen] = '/'; From 82e2c5bd2255a7f698cc1c9deaf3a039cf924a2f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 15 Apr 2015 01:21:27 +0900 Subject: [PATCH 25/50] Never index authorization and small cookie header field nghttp2 library now use Literal Header Field never Indexed for "authorization" header field and small "cookie" header field, regardless of nghttp2_nv.flags. --- lib/nghttp2_hd.c | 101 ++++++++++++++++++++-------------------- lib/nghttp2_hd.h | 10 +++- src/nghttp.cc | 5 +- tests/nghttp2_hd_test.c | 36 ++++++++------ 4 files changed, 82 insertions(+), 70 deletions(-) diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index 7acc1c71..fbebe28c 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -639,38 +639,36 @@ static int emit_string(nghttp2_bufs *bufs, const uint8_t *str, size_t len) { return rv; } -static uint8_t pack_first_byte(int inc_indexing, int no_index) { - if (inc_indexing) { +static uint8_t pack_first_byte(int indexing_mode) { + switch (indexing_mode) { + case NGHTTP2_HD_WITH_INDEXING: return 0x40u; - } - - if (no_index) { + case NGHTTP2_HD_WITHOUT_INDEXING: + return 0; + case NGHTTP2_HD_NEVER_INDEXING: return 0x10u; + default: + assert(0); } - - return 0; } static int emit_indname_block(nghttp2_bufs *bufs, size_t idx, - const nghttp2_nv *nv, int inc_indexing) { + const nghttp2_nv *nv, int indexing_mode) { int rv; uint8_t *bufp; size_t blocklen; uint8_t sb[16]; size_t prefixlen; - int no_index; - no_index = (nv->flags & NGHTTP2_NV_FLAG_NO_INDEX) != 0; - - if (inc_indexing) { + if (indexing_mode == NGHTTP2_HD_WITH_INDEXING) { prefixlen = 6; } else { prefixlen = 4; } DEBUGF(fprintf(stderr, "deflatehd: emit indname index=%zu, valuelen=%zu, " - "indexing=%d, no_index=%d\n", - idx, nv->valuelen, inc_indexing, no_index)); + "indexing_mode=%d\n", + idx, nv->valuelen, indexing_mode)); blocklen = count_encoded_length(idx + 1, prefixlen); @@ -680,7 +678,7 @@ static int emit_indname_block(nghttp2_bufs *bufs, size_t idx, bufp = sb; - *bufp = pack_first_byte(inc_indexing, no_index); + *bufp = pack_first_byte(indexing_mode); encode_length(bufp, idx + 1, prefixlen); @@ -698,17 +696,14 @@ static int emit_indname_block(nghttp2_bufs *bufs, size_t idx, } static int emit_newname_block(nghttp2_bufs *bufs, const nghttp2_nv *nv, - int inc_indexing) { + int indexing_mode) { int rv; - int no_index; - - no_index = (nv->flags & NGHTTP2_NV_FLAG_NO_INDEX) != 0; DEBUGF(fprintf(stderr, "deflatehd: emit newname namelen=%zu, valuelen=%zu, " - "indexing=%d, no_index=%d\n", - nv->namelen, nv->valuelen, inc_indexing, no_index)); + "indexing_mode=%d\n", + nv->namelen, nv->valuelen, indexing_mode)); - rv = nghttp2_bufs_addb(bufs, pack_first_byte(inc_indexing, no_index)); + rv = nghttp2_bufs_addb(bufs, pack_first_byte(indexing_mode)); if (rv != 0) { return rv; } @@ -820,15 +815,14 @@ typedef struct { static search_result search_hd_table(nghttp2_hd_context *context, const nghttp2_nv *nv, uint32_t name_hash, - uint32_t value_hash) { + uint32_t value_hash, int indexing_mode) { ssize_t left = -1, right = (ssize_t)STATIC_TABLE_LENGTH; search_result res = {-1, 0}; size_t i; - int use_index = (nv->flags & NGHTTP2_NV_FLAG_NO_INDEX) == 0; /* Search dynamic table first, so that we can find recently used entry first */ - if (use_index) { + if (indexing_mode != NGHTTP2_HD_NEVER_INDEXING) { for (i = 0; i < context->hd_table.len; ++i) { nghttp2_hd_entry *ent = hd_ringbuf_get(&context->hd_table, i); if (ent->name_hash != name_hash || !name_eq(&ent->nv, nv)) { @@ -867,8 +861,8 @@ static search_result search_hd_table(nghttp2_hd_context *context, if (res.index == -1) { res.index = (ssize_t)(static_table[i].index); } - if (use_index && ent->value_hash == value_hash && - value_eq(&ent->nv, nv)) { + if (indexing_mode != NGHTTP2_HD_NEVER_INDEXING && + ent->value_hash == value_hash && value_eq(&ent->nv, nv)) { res.index = (ssize_t)(static_table[i].index); res.name_value_match = 1; return res; @@ -942,22 +936,17 @@ nghttp2_hd_entry *nghttp2_hd_table_get(nghttp2_hd_context *context, #define name_match(NV, NAME) \ (nv->namelen == sizeof(NAME) - 1 && memeq(nv->name, NAME, sizeof(NAME) - 1)) -static int hd_deflate_should_indexing(nghttp2_hd_deflater *deflater, +static int hd_deflate_decide_indexing(nghttp2_hd_deflater *deflater, const nghttp2_nv *nv) { - if ((nv->flags & NGHTTP2_NV_FLAG_NO_INDEX) || - entry_room(nv->namelen, nv->valuelen) > - deflater->ctx.hd_table_bufsize_max * 3 / 4) { - return 0; + if (entry_room(nv->namelen, nv->valuelen) > + deflater->ctx.hd_table_bufsize_max * 3 / 4 || + name_match(nv, ":path") || name_match(nv, "content-length") || + name_match(nv, "set-cookie") || name_match(nv, "etag") || + name_match(nv, "if-modified-since") || name_match(nv, "if-none-match") || + name_match(nv, "location") || name_match(nv, "age")) { + return NGHTTP2_HD_WITHOUT_INDEXING; } -#ifdef NGHTTP2_XHD - return !name_match(nv, NGHTTP2_XHD); -#else /* !NGHTTP2_XHD */ - return !name_match(nv, ":path") && !name_match(nv, "content-length") && - !name_match(nv, "set-cookie") && !name_match(nv, "etag") && - !name_match(nv, "if-modified-since") && - !name_match(nv, "if-none-match") && !name_match(nv, "location") && - !name_match(nv, "age"); -#endif /* !NGHTTP2_XHD */ + return NGHTTP2_HD_WITH_INDEXING; } static int deflate_nv(nghttp2_hd_deflater *deflater, nghttp2_bufs *bufs, @@ -965,7 +954,7 @@ static int deflate_nv(nghttp2_hd_deflater *deflater, nghttp2_bufs *bufs, int rv; search_result res; ssize_t idx; - int incidx = 0; + int indexing_mode; uint32_t name_hash = hash(nv->name, nv->namelen); uint32_t value_hash = hash(nv->value, nv->valuelen); nghttp2_mem *mem; @@ -974,7 +963,18 @@ static int deflate_nv(nghttp2_hd_deflater *deflater, nghttp2_bufs *bufs, mem = deflater->ctx.mem; - res = search_hd_table(&deflater->ctx, nv, name_hash, value_hash); + /* Don't index authorization header field since it may contain low + entropy secret data (e.g., id/password). Also cookie header + field with less than 20 bytes value is also never indexed. This + is the same criteria used in Firefox codebase. */ + indexing_mode = name_match(nv, "authorization") || + (name_match(nv, "cookie") && nv->valuelen < 20) || + (nv->flags & NGHTTP2_NV_FLAG_NO_INDEX) + ? NGHTTP2_HD_NEVER_INDEXING + : hd_deflate_decide_indexing(deflater, nv); + + res = + search_hd_table(&deflater->ctx, nv, name_hash, value_hash, indexing_mode); idx = res.index; @@ -994,7 +994,7 @@ static int deflate_nv(nghttp2_hd_deflater *deflater, nghttp2_bufs *bufs, DEBUGF(fprintf(stderr, "deflatehd: name match index=%zd\n", res.index)); } - if (hd_deflate_should_indexing(deflater, nv)) { + if (indexing_mode == NGHTTP2_HD_WITH_INDEXING) { nghttp2_hd_entry *new_ent; if (idx != -1 && idx < (ssize_t)NGHTTP2_STATIC_TABLE_LENGTH) { nghttp2_nv nv_indname; @@ -1015,12 +1015,11 @@ static int deflate_nv(nghttp2_hd_deflater *deflater, nghttp2_bufs *bufs, nghttp2_hd_entry_free(new_ent, mem); nghttp2_mem_free(mem, new_ent); } - incidx = 1; } if (idx == -1) { - rv = emit_newname_block(bufs, nv, incidx); + rv = emit_newname_block(bufs, nv, indexing_mode); } else { - rv = emit_indname_block(bufs, idx, nv, incidx); + rv = emit_indname_block(bufs, idx, nv, indexing_mode); } if (rv != 0) { return rv; @@ -1917,14 +1916,14 @@ void nghttp2_hd_inflate_del(nghttp2_hd_inflater *inflater) { } int nghttp2_hd_emit_indname_block(nghttp2_bufs *bufs, size_t idx, - nghttp2_nv *nv, int inc_indexing) { + nghttp2_nv *nv, int indexing_mode) { - return emit_indname_block(bufs, idx, nv, inc_indexing); + return emit_indname_block(bufs, idx, nv, indexing_mode); } int nghttp2_hd_emit_newname_block(nghttp2_bufs *bufs, nghttp2_nv *nv, - int inc_indexing) { - return emit_newname_block(bufs, nv, inc_indexing); + int indexing_mode) { + return emit_newname_block(bufs, nv, indexing_mode); } int nghttp2_hd_emit_table_size(nghttp2_bufs *bufs, size_t table_size) { diff --git a/lib/nghttp2_hd.h b/lib/nghttp2_hd.h index d4616795..ef9fa3ac 100644 --- a/lib/nghttp2_hd.h +++ b/lib/nghttp2_hd.h @@ -107,6 +107,12 @@ typedef enum { NGHTTP2_HD_STATE_READ_VALUE } nghttp2_hd_inflate_state; +typedef enum { + NGHTTP2_HD_WITH_INDEXING, + NGHTTP2_HD_WITHOUT_INDEXING, + NGHTTP2_HD_NEVER_INDEXING +} nghttp2_hd_indexing_mode; + typedef struct { /* dynamic header table */ nghttp2_hd_ringbuf hd_table; @@ -273,11 +279,11 @@ void nghttp2_hd_inflate_free(nghttp2_hd_inflater *inflater); /* For unittesting purpose */ int nghttp2_hd_emit_indname_block(nghttp2_bufs *bufs, size_t index, - nghttp2_nv *nv, int inc_indexing); + nghttp2_nv *nv, int indexing_mode); /* For unittesting purpose */ int nghttp2_hd_emit_newname_block(nghttp2_bufs *bufs, nghttp2_nv *nv, - int inc_indexing); + int indexing_mode); /* For unittesting purpose */ int nghttp2_hd_emit_table_size(nghttp2_bufs *bufs, size_t table_size); diff --git a/src/nghttp.cc b/src/nghttp.cc index 9329d436..05bd22ac 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -2565,10 +2565,7 @@ int main(int argc, char **argv) { << std::endl; exit(EXIT_FAILURE); } - // To test "never index" repr, don't index authorization header - // field unconditionally. - auto no_index = util::strieq_l("authorization", header); - config.headers.emplace_back(header, value, no_index); + config.headers.emplace_back(header, value, false); util::inp_strlower(config.headers.back().name); break; } diff --git a/tests/nghttp2_hd_test.c b/tests/nghttp2_hd_test.c index 027ac639..ecd7bc11 100644 --- a/tests/nghttp2_hd_test.c +++ b/tests/nghttp2_hd_test.c @@ -140,9 +140,9 @@ void test_nghttp2_hd_deflate(void) { void test_nghttp2_hd_deflate_same_indexed_repr(void) { nghttp2_hd_deflater deflater; nghttp2_hd_inflater inflater; - nghttp2_nv nva1[] = {MAKE_NV("cookie", "alpha"), MAKE_NV("cookie", "alpha")}; - nghttp2_nv nva2[] = {MAKE_NV("cookie", "alpha"), MAKE_NV("cookie", "alpha"), - MAKE_NV("cookie", "alpha")}; + nghttp2_nv nva1[] = {MAKE_NV("host", "alpha"), MAKE_NV("host", "alpha")}; + nghttp2_nv nva2[] = {MAKE_NV("host", "alpha"), MAKE_NV("host", "alpha"), + MAKE_NV("host", "alpha")}; nghttp2_bufs bufs; ssize_t blocklen; nva_out out; @@ -250,7 +250,8 @@ void test_nghttp2_hd_inflate_indname_noinc(void) { nghttp2_hd_inflate_init(&inflater, mem); for (i = 0; i < ARRLEN(nv); ++i) { - CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 57, &nv[i], 0)); + CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 57, &nv[i], + NGHTTP2_HD_WITHOUT_INDEXING)); blocklen = nghttp2_bufs_len(&bufs); @@ -283,7 +284,8 @@ void test_nghttp2_hd_inflate_indname_inc(void) { nva_out_init(&out); nghttp2_hd_inflate_init(&inflater, mem); - CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 57, &nv, 1)); + CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 57, &nv, + NGHTTP2_HD_WITH_INDEXING)); blocklen = nghttp2_bufs_len(&bufs); @@ -325,10 +327,14 @@ void test_nghttp2_hd_inflate_indname_inc_eviction(void) { nv.flags = NGHTTP2_NV_FLAG_NONE; - CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 14, &nv, 1)); - CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 15, &nv, 1)); - CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 16, &nv, 1)); - CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 17, &nv, 1)); + CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 14, &nv, + NGHTTP2_HD_WITH_INDEXING)); + CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 15, &nv, + NGHTTP2_HD_WITH_INDEXING)); + CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 16, &nv, + NGHTTP2_HD_WITH_INDEXING)); + CU_ASSERT(0 == nghttp2_hd_emit_indname_block(&bufs, 17, &nv, + NGHTTP2_HD_WITH_INDEXING)); blocklen = nghttp2_bufs_len(&bufs); @@ -372,7 +378,8 @@ void test_nghttp2_hd_inflate_newname_noinc(void) { nva_out_init(&out); nghttp2_hd_inflate_init(&inflater, mem); for (i = 0; i < ARRLEN(nv); ++i) { - CU_ASSERT(0 == nghttp2_hd_emit_newname_block(&bufs, &nv[i], 0)); + CU_ASSERT(0 == nghttp2_hd_emit_newname_block(&bufs, &nv[i], + NGHTTP2_HD_WITHOUT_INDEXING)); blocklen = nghttp2_bufs_len(&bufs); @@ -405,7 +412,8 @@ void test_nghttp2_hd_inflate_newname_inc(void) { nva_out_init(&out); nghttp2_hd_inflate_init(&inflater, mem); - CU_ASSERT(0 == nghttp2_hd_emit_newname_block(&bufs, &nv, 1)); + CU_ASSERT( + 0 == nghttp2_hd_emit_newname_block(&bufs, &nv, NGHTTP2_HD_WITH_INDEXING)); blocklen = nghttp2_bufs_len(&bufs); @@ -450,7 +458,8 @@ void test_nghttp2_hd_inflate_clearall_inc(void) { nghttp2_hd_inflate_init(&inflater, mem); - CU_ASSERT(0 == nghttp2_hd_emit_newname_block(&bufs, &nv, 1)); + CU_ASSERT( + 0 == nghttp2_hd_emit_newname_block(&bufs, &nv, NGHTTP2_HD_WITH_INDEXING)); blocklen = nghttp2_bufs_len(&bufs); @@ -477,7 +486,8 @@ void test_nghttp2_hd_inflate_clearall_inc(void) { header table */ nv.valuelen = sizeof(value) - 2; - CU_ASSERT(0 == nghttp2_hd_emit_newname_block(&bufs, &nv, 1)); + CU_ASSERT( + 0 == nghttp2_hd_emit_newname_block(&bufs, &nv, NGHTTP2_HD_WITH_INDEXING)); blocklen = nghttp2_bufs_len(&bufs); From 93afbc7d2fb5947791f37f1549ebe01042d2be68 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 15 Apr 2015 22:32:11 +0900 Subject: [PATCH 26/50] Rewrite static header table handling We rewrite static header table handling in nghttp2_hd.c. We expand nghttp2_token to include all static header table entries, and fully use them in header compression and decompression. The lookup function is now located in nghttp2_hd.c. We add new nghttp2_hd_inflate_hd2() function to export token value for header name, then we pass it to nghttp2_http_on_header function, so that we don't have to look up token there. We carefully set enum value of token to static table index, so looking up static table is now O(1), assuming we have token. --- genlibtokenlookup.py | 103 ++++-- lib/nghttp2_hd.c | 739 +++++++++++++++++++++++++++++++----------- lib/nghttp2_hd.h | 92 +++++- lib/nghttp2_helper.h | 4 + lib/nghttp2_http.c | 167 +--------- lib/nghttp2_http.h | 6 +- lib/nghttp2_session.c | 7 +- mkstatichdtbl.py | 38 +-- 8 files changed, 748 insertions(+), 408 deletions(-) diff --git a/genlibtokenlookup.py b/genlibtokenlookup.py index 25591aa9..d7ac7f47 100755 --- a/genlibtokenlookup.py +++ b/genlibtokenlookup.py @@ -1,19 +1,72 @@ #!/usr/bin/env python HEADERS = [ - ':authority', - ':method', - ':path', - ':scheme', - ':status', - "content-length", - "host", - "te", - 'connection', - 'keep-alive', - 'proxy-connection', - 'transfer-encoding', - 'upgrade' + (':authority', 0), + (':method', 1), + (':method', 2), + (':path', 3), + (':path', 4), + (':scheme', 5), + (':scheme', 6), + (':status', 7), + (':status', 8), + (':status', 9), + (':status', 10), + (':status', 11), + (':status', 12), + (':status', 13), + ('accept-charset', 14), + ('accept-encoding', 15), + ('accept-language', 16), + ('accept-ranges', 17), + ('accept', 18), + ('access-control-allow-origin', 19), + ('age', 20), + ('allow', 21), + ('authorization', 22), + ('cache-control', 23), + ('content-disposition', 24), + ('content-encoding', 25), + ('content-language', 26), + ('content-length', 27), + ('content-location', 28), + ('content-range', 29), + ('content-type', 30), + ('cookie', 31), + ('date', 32), + ('etag', 33), + ('expect', 34), + ('expires', 35), + ('from', 36), + ('host', 37), + ('if-match', 38), + ('if-modified-since', 39), + ('if-none-match', 40), + ('if-range', 41), + ('if-unmodified-since', 42), + ('last-modified', 43), + ('link', 44), + ('location', 45), + ('max-forwards', 46), + ('proxy-authenticate', 47), + ('proxy-authorization', 48), + ('range', 49), + ('referer', 50), + ('refresh', 51), + ('retry-after', 52), + ('server', 53), + ('set-cookie', 54), + ('strict-transport-security', 55), + ('transfer-encoding', 56), + ('user-agent', 57), + ('vary', 58), + ('via', 59), + ('www-authenticate', 60), + ('te', None), + ('connection', None), + ('keep-alive',None), + ('proxy-connection', None), + ('upgrade', None), ] def to_enum_hd(k): @@ -27,7 +80,7 @@ def to_enum_hd(k): def build_header(headers): res = {} - for k in headers: + for k, _ in headers: size = len(k) if size not in res: res[size] = {} @@ -40,18 +93,20 @@ def build_header(headers): return res def gen_enum(): - print '''\ -typedef enum {''' - for k in sorted(HEADERS): - print '''\ - {},'''.format(to_enum_hd(k)) - print '''\ - NGHTTP2_TOKEN_MAXIDX, -} nghttp2_token;''' + name = '' + print 'typedef enum {' + for k, token in HEADERS: + if token is None: + print ' {},'.format(to_enum_hd(k)) + else: + if name != k: + name = k + print ' {} = {},'.format(to_enum_hd(k), token) + print '} nghttp2_token;' def gen_index_header(): print '''\ -static int lookup_token(const uint8_t *name, size_t namelen) { +static inline int lookup_token(const uint8_t *name, size_t namelen) { switch (namelen) {''' b = build_header(HEADERS) for size in sorted(b.keys()): @@ -66,7 +121,7 @@ static int lookup_token(const uint8_t *name, size_t namelen) { case '{}':'''.format(c) for k in headers: print '''\ - if (streq("{}", name, {})) {{ + if (lstreq("{}", name, {})) {{ return {}; }}'''.format(k[:-1], size - 1, to_enum_hd(k)) print '''\ diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index fbebe28c..e023229e 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -31,118 +31,472 @@ #include "nghttp2_helper.h" #include "nghttp2_int.h" -#define STATIC_TABLE_LENGTH 61 - -/* Make scalar initialization form of nghttp2_nv */ -#define MAKE_STATIC_ENT(I, N, V, NH, VH) \ +/* Make scalar initialization form of nghttp2_hd_entry */ +#define MAKE_STATIC_ENT(N, V, T) \ { \ - { \ - { (uint8_t *)(N), (uint8_t *)(V), sizeof((N)) - 1, sizeof((V)) - 1, 0 } \ - , (NH), (VH), 1, NGHTTP2_HD_FLAG_NONE \ - } \ - , I \ + { (uint8_t *)(N), (uint8_t *)(V), sizeof((N)) - 1, sizeof((V)) - 1, 0 } \ + , (T), 1, NGHTTP2_HD_FLAG_NONE \ } /* Generated by mkstatictbl.py */ -/* Sorted by hash(name) and its table index */ -static nghttp2_hd_static_entry static_table[] = { - MAKE_STATIC_ENT(20, "age", "", 96511u, 0u), - MAKE_STATIC_ENT(59, "via", "", 116750u, 0u), - MAKE_STATIC_ENT(32, "date", "", 3076014u, 0u), - MAKE_STATIC_ENT(33, "etag", "", 3123477u, 0u), - MAKE_STATIC_ENT(36, "from", "", 3151786u, 0u), - MAKE_STATIC_ENT(37, "host", "", 3208616u, 0u), - MAKE_STATIC_ENT(44, "link", "", 3321850u, 0u), - MAKE_STATIC_ENT(58, "vary", "", 3612210u, 0u), - MAKE_STATIC_ENT(38, "if-match", "", 34533653u, 0u), - MAKE_STATIC_ENT(41, "if-range", "", 39145613u, 0u), - MAKE_STATIC_ENT(3, ":path", "/", 56997727u, 47u), - MAKE_STATIC_ENT(4, ":path", "/index.html", 56997727u, 2144181430u), - MAKE_STATIC_ENT(21, "allow", "", 92906313u, 0u), - MAKE_STATIC_ENT(49, "range", "", 108280125u, 0u), - MAKE_STATIC_ENT(14, "accept-charset", "", 124285319u, 0u), - MAKE_STATIC_ENT(43, "last-modified", "", 150043680u, 0u), - MAKE_STATIC_ENT(48, "proxy-authorization", "", 329532250u, 0u), - MAKE_STATIC_ENT(57, "user-agent", "", 486342275u, 0u), - MAKE_STATIC_ENT(40, "if-none-match", "", 646073760u, 0u), - MAKE_STATIC_ENT(30, "content-type", "", 785670158u, 0u), - MAKE_STATIC_ENT(16, "accept-language", "", 802785917u, 0u), - MAKE_STATIC_ENT(50, "referer", "", 1085069613u, 0u), - MAKE_STATIC_ENT(51, "refresh", "", 1085444827u, 0u), - MAKE_STATIC_ENT(55, "strict-transport-security", "", 1153852136u, 0u), - MAKE_STATIC_ENT(54, "set-cookie", "", 1237214767u, 0u), - MAKE_STATIC_ENT(56, "transfer-encoding", "", 1274458357u, 0u), - MAKE_STATIC_ENT(17, "accept-ranges", "", 1397189435u, 0u), - MAKE_STATIC_ENT(42, "if-unmodified-since", "", 1454068927u, 0u), - MAKE_STATIC_ENT(46, "max-forwards", "", 1619948695u, 0u), - MAKE_STATIC_ENT(45, "location", "", 1901043637u, 0u), - MAKE_STATIC_ENT(52, "retry-after", "", 1933352567u, 0u), - MAKE_STATIC_ENT(25, "content-encoding", "", 2095084583u, 0u), - MAKE_STATIC_ENT(28, "content-location", "", 2284906121u, 0u), - MAKE_STATIC_ENT(39, "if-modified-since", "", 2302095846u, 0u), - MAKE_STATIC_ENT(18, "accept", "", 2871506184u, 0u), - MAKE_STATIC_ENT(29, "content-range", "", 2878374633u, 0u), - MAKE_STATIC_ENT(22, "authorization", "", 2909397113u, 0u), - MAKE_STATIC_ENT(31, "cookie", "", 2940209764u, 0u), - MAKE_STATIC_ENT(0, ":authority", "", 2962729033u, 0u), - MAKE_STATIC_ENT(35, "expires", "", 2985731892u, 0u), - MAKE_STATIC_ENT(34, "expect", "", 3005803609u, 0u), - MAKE_STATIC_ENT(24, "content-disposition", "", 3027699811u, 0u), - MAKE_STATIC_ENT(26, "content-language", "", 3065240108u, 0u), - MAKE_STATIC_ENT(1, ":method", "GET", 3153018267u, 70454u), - MAKE_STATIC_ENT(2, ":method", "POST", 3153018267u, 2461856u), - MAKE_STATIC_ENT(27, "content-length", "", 3162187450u, 0u), - MAKE_STATIC_ENT(19, "access-control-allow-origin", "", 3297999203u, 0u), - MAKE_STATIC_ENT(5, ":scheme", "http", 3322585695u, 3213448u), - MAKE_STATIC_ENT(6, ":scheme", "https", 3322585695u, 99617003u), - MAKE_STATIC_ENT(7, ":status", "200", 3338091692u, 49586u), - MAKE_STATIC_ENT(8, ":status", "204", 3338091692u, 49590u), - MAKE_STATIC_ENT(9, ":status", "206", 3338091692u, 49592u), - MAKE_STATIC_ENT(10, ":status", "304", 3338091692u, 50551u), - MAKE_STATIC_ENT(11, ":status", "400", 3338091692u, 51508u), - MAKE_STATIC_ENT(12, ":status", "404", 3338091692u, 51512u), - MAKE_STATIC_ENT(13, ":status", "500", 3338091692u, 52469u), - MAKE_STATIC_ENT(53, "server", "", 3389140803u, 0u), - MAKE_STATIC_ENT(47, "proxy-authenticate", "", 3993199572u, 0u), - MAKE_STATIC_ENT(60, "www-authenticate", "", 4051929931u, 0u), - MAKE_STATIC_ENT(23, "cache-control", "", 4086191634u, 0u), - MAKE_STATIC_ENT(15, "accept-encoding", "gzip, deflate", 4127597688u, - 1733326877u), +/* 3rd parameter is nghttp2_token value for header field name. We use + first enum value if same header names are repeated (e.g., + :status). */ +static nghttp2_hd_entry static_table[] = { + MAKE_STATIC_ENT(":authority", "", 0), + MAKE_STATIC_ENT(":method", "GET", 1), + MAKE_STATIC_ENT(":method", "POST", 1), + MAKE_STATIC_ENT(":path", "/", 3), + MAKE_STATIC_ENT(":path", "/index.html", 3), + MAKE_STATIC_ENT(":scheme", "http", 5), + MAKE_STATIC_ENT(":scheme", "https", 5), + MAKE_STATIC_ENT(":status", "200", 7), + MAKE_STATIC_ENT(":status", "204", 7), + MAKE_STATIC_ENT(":status", "206", 7), + MAKE_STATIC_ENT(":status", "304", 7), + MAKE_STATIC_ENT(":status", "400", 7), + MAKE_STATIC_ENT(":status", "404", 7), + MAKE_STATIC_ENT(":status", "500", 7), + MAKE_STATIC_ENT("accept-charset", "", 14), + MAKE_STATIC_ENT("accept-encoding", "gzip, deflate", 15), + MAKE_STATIC_ENT("accept-language", "", 16), + MAKE_STATIC_ENT("accept-ranges", "", 17), + MAKE_STATIC_ENT("accept", "", 18), + MAKE_STATIC_ENT("access-control-allow-origin", "", 19), + MAKE_STATIC_ENT("age", "", 20), + MAKE_STATIC_ENT("allow", "", 21), + MAKE_STATIC_ENT("authorization", "", 22), + MAKE_STATIC_ENT("cache-control", "", 23), + MAKE_STATIC_ENT("content-disposition", "", 24), + MAKE_STATIC_ENT("content-encoding", "", 25), + MAKE_STATIC_ENT("content-language", "", 26), + MAKE_STATIC_ENT("content-length", "", 27), + MAKE_STATIC_ENT("content-location", "", 28), + MAKE_STATIC_ENT("content-range", "", 29), + MAKE_STATIC_ENT("content-type", "", 30), + MAKE_STATIC_ENT("cookie", "", 31), + MAKE_STATIC_ENT("date", "", 32), + MAKE_STATIC_ENT("etag", "", 33), + MAKE_STATIC_ENT("expect", "", 34), + MAKE_STATIC_ENT("expires", "", 35), + MAKE_STATIC_ENT("from", "", 36), + MAKE_STATIC_ENT("host", "", 37), + MAKE_STATIC_ENT("if-match", "", 38), + MAKE_STATIC_ENT("if-modified-since", "", 39), + MAKE_STATIC_ENT("if-none-match", "", 40), + MAKE_STATIC_ENT("if-range", "", 41), + MAKE_STATIC_ENT("if-unmodified-since", "", 42), + MAKE_STATIC_ENT("last-modified", "", 43), + MAKE_STATIC_ENT("link", "", 44), + MAKE_STATIC_ENT("location", "", 45), + MAKE_STATIC_ENT("max-forwards", "", 46), + MAKE_STATIC_ENT("proxy-authenticate", "", 47), + MAKE_STATIC_ENT("proxy-authorization", "", 48), + MAKE_STATIC_ENT("range", "", 49), + MAKE_STATIC_ENT("referer", "", 50), + MAKE_STATIC_ENT("refresh", "", 51), + MAKE_STATIC_ENT("retry-after", "", 52), + MAKE_STATIC_ENT("server", "", 53), + MAKE_STATIC_ENT("set-cookie", "", 54), + MAKE_STATIC_ENT("strict-transport-security", "", 55), + MAKE_STATIC_ENT("transfer-encoding", "", 56), + MAKE_STATIC_ENT("user-agent", "", 57), + MAKE_STATIC_ENT("vary", "", 58), + MAKE_STATIC_ENT("via", "", 59), + MAKE_STATIC_ENT("www-authenticate", "", 60), }; -/* Index to the position in static_table */ -const size_t static_table_index[] = { - 38, 43, 44, 10, 11, 47, 48, 49, 50, 51, 52, 53, 54, 55, 14, 60, - 20, 26, 34, 46, 0, 12, 36, 59, 41, 31, 42, 45, 32, 35, 19, 37, - 2, 3, 40, 39, 4, 5, 8, 33, 18, 9, 27, 15, 6, 29, 28, 57, - 16, 13, 21, 22, 30, 56, 24, 23, 25, 17, 7, 1, 58}; - -const size_t NGHTTP2_STATIC_TABLE_LENGTH = - sizeof(static_table) / sizeof(static_table[0]); - static int memeq(const void *s1, const void *s2, size_t n) { - const uint8_t *a = (const uint8_t *)s1, *b = (const uint8_t *)s2; - uint8_t c = 0; - while (n > 0) { - c |= (*a++) ^ (*b++); - --n; - } - return c == 0; + return memcmp(s1, s2, n) == 0; } -static uint32_t hash(const uint8_t *s, size_t n) { - uint32_t h = 0; - while (n > 0) { - h = h * 31 + *s++; - --n; +/* + * This function was generated by genlibtokenlookup.py. Inspired by + * h2o header lookup. https://github.com/h2o/h2o + */ +static inline int lookup_token(const uint8_t *name, size_t namelen) { + switch (namelen) { + case 2: + switch (name[1]) { + case 'e': + if (lstreq("t", name, 1)) { + return NGHTTP2_TOKEN_TE; + } + break; + } + break; + case 3: + switch (name[2]) { + case 'a': + if (lstreq("vi", name, 2)) { + return NGHTTP2_TOKEN_VIA; + } + break; + case 'e': + if (lstreq("ag", name, 2)) { + return NGHTTP2_TOKEN_AGE; + } + break; + } + break; + case 4: + switch (name[3]) { + case 'e': + if (lstreq("dat", name, 3)) { + return NGHTTP2_TOKEN_DATE; + } + break; + case 'g': + if (lstreq("eta", name, 3)) { + return NGHTTP2_TOKEN_ETAG; + } + break; + case 'k': + if (lstreq("lin", name, 3)) { + return NGHTTP2_TOKEN_LINK; + } + break; + case 'm': + if (lstreq("fro", name, 3)) { + return NGHTTP2_TOKEN_FROM; + } + break; + case 't': + if (lstreq("hos", name, 3)) { + return NGHTTP2_TOKEN_HOST; + } + break; + case 'y': + if (lstreq("var", name, 3)) { + return NGHTTP2_TOKEN_VARY; + } + break; + } + break; + case 5: + switch (name[4]) { + case 'e': + if (lstreq("rang", name, 4)) { + return NGHTTP2_TOKEN_RANGE; + } + break; + case 'h': + if (lstreq(":pat", name, 4)) { + return NGHTTP2_TOKEN__PATH; + } + if (lstreq(":pat", name, 4)) { + return NGHTTP2_TOKEN__PATH; + } + break; + case 'w': + if (lstreq("allo", name, 4)) { + return NGHTTP2_TOKEN_ALLOW; + } + break; + } + break; + case 6: + switch (name[5]) { + case 'e': + if (lstreq("cooki", name, 5)) { + return NGHTTP2_TOKEN_COOKIE; + } + break; + case 'r': + if (lstreq("serve", name, 5)) { + return NGHTTP2_TOKEN_SERVER; + } + break; + case 't': + if (lstreq("accep", name, 5)) { + return NGHTTP2_TOKEN_ACCEPT; + } + if (lstreq("expec", name, 5)) { + return NGHTTP2_TOKEN_EXPECT; + } + break; + } + break; + case 7: + switch (name[6]) { + case 'd': + if (lstreq(":metho", name, 6)) { + return NGHTTP2_TOKEN__METHOD; + } + if (lstreq(":metho", name, 6)) { + return NGHTTP2_TOKEN__METHOD; + } + break; + case 'e': + if (lstreq(":schem", name, 6)) { + return NGHTTP2_TOKEN__SCHEME; + } + if (lstreq(":schem", name, 6)) { + return NGHTTP2_TOKEN__SCHEME; + } + if (lstreq("upgrad", name, 6)) { + return NGHTTP2_TOKEN_UPGRADE; + } + break; + case 'h': + if (lstreq("refres", name, 6)) { + return NGHTTP2_TOKEN_REFRESH; + } + break; + case 'r': + if (lstreq("refere", name, 6)) { + return NGHTTP2_TOKEN_REFERER; + } + break; + case 's': + if (lstreq(":statu", name, 6)) { + return NGHTTP2_TOKEN__STATUS; + } + if (lstreq(":statu", name, 6)) { + return NGHTTP2_TOKEN__STATUS; + } + if (lstreq(":statu", name, 6)) { + return NGHTTP2_TOKEN__STATUS; + } + if (lstreq(":statu", name, 6)) { + return NGHTTP2_TOKEN__STATUS; + } + if (lstreq(":statu", name, 6)) { + return NGHTTP2_TOKEN__STATUS; + } + if (lstreq(":statu", name, 6)) { + return NGHTTP2_TOKEN__STATUS; + } + if (lstreq(":statu", name, 6)) { + return NGHTTP2_TOKEN__STATUS; + } + if (lstreq("expire", name, 6)) { + return NGHTTP2_TOKEN_EXPIRES; + } + break; + } + break; + case 8: + switch (name[7]) { + case 'e': + if (lstreq("if-rang", name, 7)) { + return NGHTTP2_TOKEN_IF_RANGE; + } + break; + case 'h': + if (lstreq("if-matc", name, 7)) { + return NGHTTP2_TOKEN_IF_MATCH; + } + break; + case 'n': + if (lstreq("locatio", name, 7)) { + return NGHTTP2_TOKEN_LOCATION; + } + break; + } + break; + case 10: + switch (name[9]) { + case 'e': + if (lstreq("keep-aliv", name, 9)) { + return NGHTTP2_TOKEN_KEEP_ALIVE; + } + if (lstreq("set-cooki", name, 9)) { + return NGHTTP2_TOKEN_SET_COOKIE; + } + break; + case 'n': + if (lstreq("connectio", name, 9)) { + return NGHTTP2_TOKEN_CONNECTION; + } + break; + case 't': + if (lstreq("user-agen", name, 9)) { + return NGHTTP2_TOKEN_USER_AGENT; + } + break; + case 'y': + if (lstreq(":authorit", name, 9)) { + return NGHTTP2_TOKEN__AUTHORITY; + } + break; + } + break; + case 11: + switch (name[10]) { + case 'r': + if (lstreq("retry-afte", name, 10)) { + return NGHTTP2_TOKEN_RETRY_AFTER; + } + break; + } + break; + case 12: + switch (name[11]) { + case 'e': + if (lstreq("content-typ", name, 11)) { + return NGHTTP2_TOKEN_CONTENT_TYPE; + } + break; + case 's': + if (lstreq("max-forward", name, 11)) { + return NGHTTP2_TOKEN_MAX_FORWARDS; + } + break; + } + break; + case 13: + switch (name[12]) { + case 'd': + if (lstreq("last-modifie", name, 12)) { + return NGHTTP2_TOKEN_LAST_MODIFIED; + } + break; + case 'e': + if (lstreq("content-rang", name, 12)) { + return NGHTTP2_TOKEN_CONTENT_RANGE; + } + break; + case 'h': + if (lstreq("if-none-matc", name, 12)) { + return NGHTTP2_TOKEN_IF_NONE_MATCH; + } + break; + case 'l': + if (lstreq("cache-contro", name, 12)) { + return NGHTTP2_TOKEN_CACHE_CONTROL; + } + break; + case 'n': + if (lstreq("authorizatio", name, 12)) { + return NGHTTP2_TOKEN_AUTHORIZATION; + } + break; + case 's': + if (lstreq("accept-range", name, 12)) { + return NGHTTP2_TOKEN_ACCEPT_RANGES; + } + break; + } + break; + case 14: + switch (name[13]) { + case 'h': + if (lstreq("content-lengt", name, 13)) { + return NGHTTP2_TOKEN_CONTENT_LENGTH; + } + break; + case 't': + if (lstreq("accept-charse", name, 13)) { + return NGHTTP2_TOKEN_ACCEPT_CHARSET; + } + break; + } + break; + case 15: + switch (name[14]) { + case 'e': + if (lstreq("accept-languag", name, 14)) { + return NGHTTP2_TOKEN_ACCEPT_LANGUAGE; + } + break; + case 'g': + if (lstreq("accept-encodin", name, 14)) { + return NGHTTP2_TOKEN_ACCEPT_ENCODING; + } + break; + } + break; + case 16: + switch (name[15]) { + case 'e': + if (lstreq("content-languag", name, 15)) { + return NGHTTP2_TOKEN_CONTENT_LANGUAGE; + } + if (lstreq("www-authenticat", name, 15)) { + return NGHTTP2_TOKEN_WWW_AUTHENTICATE; + } + break; + case 'g': + if (lstreq("content-encodin", name, 15)) { + return NGHTTP2_TOKEN_CONTENT_ENCODING; + } + break; + case 'n': + if (lstreq("content-locatio", name, 15)) { + return NGHTTP2_TOKEN_CONTENT_LOCATION; + } + if (lstreq("proxy-connectio", name, 15)) { + return NGHTTP2_TOKEN_PROXY_CONNECTION; + } + break; + } + break; + case 17: + switch (name[16]) { + case 'e': + if (lstreq("if-modified-sinc", name, 16)) { + return NGHTTP2_TOKEN_IF_MODIFIED_SINCE; + } + break; + case 'g': + if (lstreq("transfer-encodin", name, 16)) { + return NGHTTP2_TOKEN_TRANSFER_ENCODING; + } + break; + } + break; + case 18: + switch (name[17]) { + case 'e': + if (lstreq("proxy-authenticat", name, 17)) { + return NGHTTP2_TOKEN_PROXY_AUTHENTICATE; + } + break; + } + break; + case 19: + switch (name[18]) { + case 'e': + if (lstreq("if-unmodified-sinc", name, 18)) { + return NGHTTP2_TOKEN_IF_UNMODIFIED_SINCE; + } + break; + case 'n': + if (lstreq("content-dispositio", name, 18)) { + return NGHTTP2_TOKEN_CONTENT_DISPOSITION; + } + if (lstreq("proxy-authorizatio", name, 18)) { + return NGHTTP2_TOKEN_PROXY_AUTHORIZATION; + } + break; + } + break; + case 25: + switch (name[24]) { + case 'y': + if (lstreq("strict-transport-securit", name, 24)) { + return NGHTTP2_TOKEN_STRICT_TRANSPORT_SECURITY; + } + break; + } + break; + case 27: + switch (name[26]) { + case 'n': + if (lstreq("access-control-allow-origi", name, 26)) { + return NGHTTP2_TOKEN_ACCESS_CONTROL_ALLOW_ORIGIN; + } + break; + } + break; } - return h; + return -1; } int nghttp2_hd_entry_init(nghttp2_hd_entry *ent, uint8_t flags, uint8_t *name, size_t namelen, uint8_t *value, size_t valuelen, - uint32_t name_hash, uint32_t value_hash, - nghttp2_mem *mem) { + int token, nghttp2_mem *mem) { int rv = 0; /* Since nghttp2_hd_entry is used for indexing, ent->nv.flags always @@ -183,12 +537,10 @@ int nghttp2_hd_entry_init(nghttp2_hd_entry *ent, uint8_t flags, uint8_t *name, } ent->nv.namelen = namelen; ent->nv.valuelen = valuelen; + ent->token = token; ent->ref = 1; ent->flags = flags; - ent->name_hash = name_hash; - ent->value_hash = value_hash; - return 0; fail2: @@ -406,19 +758,23 @@ static size_t entry_room(size_t namelen, size_t valuelen) { return NGHTTP2_HD_ENTRY_OVERHEAD + namelen + valuelen; } -static int emit_indexed_header(nghttp2_nv *nv_out, nghttp2_hd_entry *ent) { +static int emit_indexed_header(nghttp2_nv *nv_out, int *token_out, + nghttp2_hd_entry *ent) { DEBUGF(fprintf(stderr, "inflatehd: header emission: %s: %s\n", ent->nv.name, ent->nv.value)); /* ent->ref may be 0. This happens if the encoder emits literal block larger than header table capacity with indexing. */ *nv_out = ent->nv; + *token_out = ent->token; return 0; } -static int emit_literal_header(nghttp2_nv *nv_out, nghttp2_nv *nv) { +static int emit_literal_header(nghttp2_nv *nv_out, int *token_out, + nghttp2_nv *nv) { DEBUGF(fprintf(stderr, "inflatehd: header emission: %s: %s\n", nv->name, nv->value)); *nv_out = *nv; + *token_out = lookup_token(nv->name, nv->namelen); return 0; } @@ -723,8 +1079,7 @@ static int emit_newname_block(nghttp2_bufs *bufs, const nghttp2_nv *nv, static nghttp2_hd_entry *add_hd_table_incremental(nghttp2_hd_context *context, const nghttp2_nv *nv, - uint32_t name_hash, - uint32_t value_hash, + int token, uint8_t entry_flags) { int rv; nghttp2_hd_entry *new_ent; @@ -758,8 +1113,7 @@ static nghttp2_hd_entry *add_hd_table_incremental(nghttp2_hd_context *context, } rv = nghttp2_hd_entry_init(new_ent, entry_flags, nv->name, nv->namelen, - nv->value, nv->valuelen, name_hash, value_hash, - mem); + nv->value, nv->valuelen, token, mem); if (rv != 0) { nghttp2_mem_free(mem, new_ent); return NULL; @@ -800,11 +1154,15 @@ static nghttp2_hd_entry *add_hd_table_incremental(nghttp2_hd_context *context, } static int name_eq(const nghttp2_nv *a, const nghttp2_nv *b) { - return a->namelen == b->namelen && memeq(a->name, b->name, a->namelen); + return a->namelen == b->namelen && + a->name[a->namelen - 1] == b->name[a->namelen - 1] && + memeq(a->name, b->name, a->namelen); } static int value_eq(const nghttp2_nv *a, const nghttp2_nv *b) { - return a->valuelen == b->valuelen && memeq(a->value, b->value, a->valuelen); + return a->valuelen == b->valuelen && + a->value[a->valuelen - 1] == b->value[a->valuelen - 1] && + memeq(a->value, b->value, a->valuelen); } typedef struct { @@ -813,19 +1171,46 @@ typedef struct { uint8_t name_value_match; } search_result; +static search_result search_static_table(const nghttp2_nv *nv, int token, + int indexing_mode) { + search_result res = {token, 0}; + int i; + + if (indexing_mode == NGHTTP2_HD_NEVER_INDEXING) { + return res; + } + + for (i = token; + i <= NGHTTP2_TOKEN_WWW_AUTHENTICATE && static_table[i].token == token; + ++i) { + if (value_eq(&static_table[i].nv, nv)) { + res.index = i; + res.name_value_match = 1; + return res; + } + } + return res; +} + static search_result search_hd_table(nghttp2_hd_context *context, - const nghttp2_nv *nv, uint32_t name_hash, - uint32_t value_hash, int indexing_mode) { - ssize_t left = -1, right = (ssize_t)STATIC_TABLE_LENGTH; + const nghttp2_nv *nv, int token, + int indexing_mode) { search_result res = {-1, 0}; size_t i; + if (token >= 0 && token <= NGHTTP2_TOKEN_WWW_AUTHENTICATE) { + res = search_static_table(nv, token, indexing_mode); + if (res.name_value_match) { + return res; + } + } + /* Search dynamic table first, so that we can find recently used entry first */ if (indexing_mode != NGHTTP2_HD_NEVER_INDEXING) { for (i = 0; i < context->hd_table.len; ++i) { nghttp2_hd_entry *ent = hd_ringbuf_get(&context->hd_table, i); - if (ent->name_hash != name_hash || !name_eq(&ent->nv, nv)) { + if (ent->token != token || (token == -1 && !name_eq(&ent->nv, nv))) { continue; } @@ -833,7 +1218,7 @@ static search_result search_hd_table(nghttp2_hd_context *context, res.index = (ssize_t)(i + NGHTTP2_STATIC_TABLE_LENGTH); } - if (ent->value_hash == value_hash && value_eq(&ent->nv, nv)) { + if (value_eq(&ent->nv, nv)) { res.index = (ssize_t)(i + NGHTTP2_STATIC_TABLE_LENGTH); res.name_value_match = 1; return res; @@ -841,35 +1226,6 @@ static search_result search_hd_table(nghttp2_hd_context *context, } } - while (right - left > 1) { - ssize_t mid = (left + right) / 2; - nghttp2_hd_entry *ent = &static_table[mid].ent; - if (ent->name_hash < name_hash) { - left = mid; - } else { - right = mid; - } - } - - for (i = right; i < STATIC_TABLE_LENGTH; ++i) { - nghttp2_hd_entry *ent = &static_table[i].ent; - if (ent->name_hash != name_hash) { - break; - } - - if (name_eq(&ent->nv, nv)) { - if (res.index == -1) { - res.index = (ssize_t)(static_table[i].index); - } - if (indexing_mode != NGHTTP2_HD_NEVER_INDEXING && - ent->value_hash == value_hash && value_eq(&ent->nv, nv)) { - res.index = (ssize_t)(static_table[i].index); - res.name_value_match = 1; - return res; - } - } - } - return res; } @@ -929,23 +1285,22 @@ nghttp2_hd_entry *nghttp2_hd_table_get(nghttp2_hd_context *context, return hd_ringbuf_get(&context->hd_table, idx - NGHTTP2_STATIC_TABLE_LENGTH); } else { - return &static_table[static_table_index[idx]].ent; + return &static_table[idx]; } } -#define name_match(NV, NAME) \ - (nv->namelen == sizeof(NAME) - 1 && memeq(nv->name, NAME, sizeof(NAME) - 1)) - static int hd_deflate_decide_indexing(nghttp2_hd_deflater *deflater, - const nghttp2_nv *nv) { - if (entry_room(nv->namelen, nv->valuelen) > - deflater->ctx.hd_table_bufsize_max * 3 / 4 || - name_match(nv, ":path") || name_match(nv, "content-length") || - name_match(nv, "set-cookie") || name_match(nv, "etag") || - name_match(nv, "if-modified-since") || name_match(nv, "if-none-match") || - name_match(nv, "location") || name_match(nv, "age")) { + const nghttp2_nv *nv, int token) { + if (token == NGHTTP2_TOKEN__PATH || token == NGHTTP2_TOKEN_AGE || + token == NGHTTP2_TOKEN_CONTENT_LENGTH || token == NGHTTP2_TOKEN_ETAG || + token == NGHTTP2_TOKEN_IF_MODIFIED_SINCE || + token == NGHTTP2_TOKEN_IF_NONE_MATCH || token == NGHTTP2_TOKEN_LOCATION || + token == NGHTTP2_TOKEN_SET_COOKIE || + entry_room(nv->namelen, nv->valuelen) > + deflater->ctx.hd_table_bufsize_max * 3 / 4) { return NGHTTP2_HD_WITHOUT_INDEXING; } + return NGHTTP2_HD_WITH_INDEXING; } @@ -955,26 +1310,27 @@ static int deflate_nv(nghttp2_hd_deflater *deflater, nghttp2_bufs *bufs, search_result res; ssize_t idx; int indexing_mode; - uint32_t name_hash = hash(nv->name, nv->namelen); - uint32_t value_hash = hash(nv->value, nv->valuelen); + int token; nghttp2_mem *mem; DEBUGF(fprintf(stderr, "deflatehd: deflating %s: %s\n", nv->name, nv->value)); mem = deflater->ctx.mem; + token = lookup_token(nv->name, nv->namelen); + /* Don't index authorization header field since it may contain low entropy secret data (e.g., id/password). Also cookie header field with less than 20 bytes value is also never indexed. This is the same criteria used in Firefox codebase. */ - indexing_mode = name_match(nv, "authorization") || - (name_match(nv, "cookie") && nv->valuelen < 20) || - (nv->flags & NGHTTP2_NV_FLAG_NO_INDEX) - ? NGHTTP2_HD_NEVER_INDEXING - : hd_deflate_decide_indexing(deflater, nv); + indexing_mode = + token == NGHTTP2_TOKEN_AUTHORIZATION || + (token == NGHTTP2_TOKEN_COOKIE && nv->valuelen < 20) || + (nv->flags & NGHTTP2_NV_FLAG_NO_INDEX) + ? NGHTTP2_HD_NEVER_INDEXING + : hd_deflate_decide_indexing(deflater, nv, token); - res = - search_hd_table(&deflater->ctx, nv, name_hash, value_hash, indexing_mode); + res = search_hd_table(&deflater->ctx, nv, token, indexing_mode); idx = res.index; @@ -1000,13 +1356,12 @@ static int deflate_nv(nghttp2_hd_deflater *deflater, nghttp2_bufs *bufs, nghttp2_nv nv_indname; nv_indname = *nv; nv_indname.name = nghttp2_hd_table_get(&deflater->ctx, idx)->nv.name; - new_ent = - add_hd_table_incremental(&deflater->ctx, &nv_indname, name_hash, - value_hash, NGHTTP2_HD_FLAG_VALUE_ALLOC); + new_ent = add_hd_table_incremental(&deflater->ctx, &nv_indname, token, + NGHTTP2_HD_FLAG_VALUE_ALLOC); } else { - new_ent = add_hd_table_incremental( - &deflater->ctx, nv, name_hash, value_hash, - NGHTTP2_HD_FLAG_NAME_ALLOC | NGHTTP2_HD_FLAG_VALUE_ALLOC); + new_ent = add_hd_table_incremental(&deflater->ctx, nv, token, + NGHTTP2_HD_FLAG_NAME_ALLOC | + NGHTTP2_HD_FLAG_VALUE_ALLOC); } if (!new_ent) { return NGHTTP2_ERR_HEADER_COMP; @@ -1303,10 +1658,10 @@ static ssize_t hd_inflate_read(nghttp2_hd_inflater *inflater, * Out of memory */ static int hd_inflate_commit_indexed(nghttp2_hd_inflater *inflater, - nghttp2_nv *nv_out) { + nghttp2_nv *nv_out, int *token_out) { nghttp2_hd_entry *ent = nghttp2_hd_table_get(&inflater->ctx, inflater->index); - emit_indexed_header(nv_out, ent); + emit_indexed_header(nv_out, token_out, ent); return 0; } @@ -1388,7 +1743,7 @@ static int hd_inflate_remove_bufs(nghttp2_hd_inflater *inflater, nghttp2_nv *nv, * Out of memory */ static int hd_inflate_commit_newname(nghttp2_hd_inflater *inflater, - nghttp2_nv *nv_out) { + nghttp2_nv *nv_out, int *token_out) { int rv; nghttp2_nv nv; nghttp2_mem *mem; @@ -1415,12 +1770,11 @@ static int hd_inflate_commit_newname(nghttp2_hd_inflater *inflater, management. */ ent_flags = NGHTTP2_HD_FLAG_NAME_ALLOC | NGHTTP2_HD_FLAG_NAME_GIFT; - new_ent = - add_hd_table_incremental(&inflater->ctx, &nv, hash(nv.name, nv.namelen), - hash(nv.value, nv.valuelen), ent_flags); + new_ent = add_hd_table_incremental( + &inflater->ctx, &nv, lookup_token(nv.name, nv.namelen), ent_flags); if (new_ent) { - emit_indexed_header(nv_out, new_ent); + emit_indexed_header(nv_out, token_out, new_ent); inflater->ent_keep = new_ent; return 0; @@ -1431,7 +1785,7 @@ static int hd_inflate_commit_newname(nghttp2_hd_inflater *inflater, return NGHTTP2_ERR_NOMEM; } - emit_literal_header(nv_out, &nv); + emit_literal_header(nv_out, token_out, &nv); if (nv.name != inflater->nvbufs.head->buf.pos) { inflater->nv_keep = nv.name; @@ -1452,7 +1806,7 @@ static int hd_inflate_commit_newname(nghttp2_hd_inflater *inflater, * Out of memory */ static int hd_inflate_commit_indname(nghttp2_hd_inflater *inflater, - nghttp2_nv *nv_out) { + nghttp2_nv *nv_out, int *token_out) { int rv; nghttp2_nv nv; nghttp2_hd_entry *ent_name; @@ -1491,8 +1845,8 @@ static int hd_inflate_commit_indname(nghttp2_hd_inflater *inflater, ++ent_name->ref; } - new_ent = add_hd_table_incremental(&inflater->ctx, &nv, ent_name->name_hash, - hash(nv.value, nv.valuelen), ent_flags); + new_ent = add_hd_table_incremental(&inflater->ctx, &nv, ent_name->token, + ent_flags); if (!static_name && --ent_name->ref == 0) { nghttp2_hd_entry_free(ent_name, mem); @@ -1500,7 +1854,7 @@ static int hd_inflate_commit_indname(nghttp2_hd_inflater *inflater, } if (new_ent) { - emit_indexed_header(nv_out, new_ent); + emit_indexed_header(nv_out, token_out, new_ent); inflater->ent_keep = new_ent; @@ -1512,7 +1866,7 @@ static int hd_inflate_commit_indname(nghttp2_hd_inflater *inflater, return NGHTTP2_ERR_NOMEM; } - emit_literal_header(nv_out, &nv); + emit_literal_header(nv_out, token_out, &nv); if (nv.value != inflater->nvbufs.head->buf.pos) { inflater->nv_keep = nv.value; @@ -1524,6 +1878,16 @@ static int hd_inflate_commit_indname(nghttp2_hd_inflater *inflater, ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, nghttp2_nv *nv_out, int *inflate_flags, uint8_t *in, size_t inlen, int in_final) { + int token; + + return nghttp2_hd_inflate_hd2(inflater, nv_out, inflate_flags, &token, in, + inlen, in_final); +} + +ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater, + nghttp2_nv *nv_out, int *inflate_flags, + int *token_out, uint8_t *in, size_t inlen, + int in_final) { ssize_t rv = 0; uint8_t *first = in; uint8_t *last = in + inlen; @@ -1536,6 +1900,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, nghttp2_nv *nv_out, DEBUGF(fprintf(stderr, "inflatehd: start state=%d\n", inflater->state)); hd_inflate_keep_free(inflater); + *token_out = -1; *inflate_flags = NGHTTP2_HD_INFLATE_NONE; for (; in != last || busy;) { busy = 0; @@ -1622,7 +1987,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, nghttp2_nv *nv_out, inflater->index = inflater->left; --inflater->index; - rv = hd_inflate_commit_indexed(inflater, nv_out); + rv = hd_inflate_commit_indexed(inflater, nv_out, token_out); if (rv < 0) { goto fail; } @@ -1781,9 +2146,9 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, nghttp2_nv *nv_out, } if (inflater->opcode == NGHTTP2_HD_OPCODE_NEWNAME) { - rv = hd_inflate_commit_newname(inflater, nv_out); + rv = hd_inflate_commit_newname(inflater, nv_out, token_out); } else { - rv = hd_inflate_commit_indname(inflater, nv_out); + rv = hd_inflate_commit_indname(inflater, nv_out, token_out); } if (rv != 0) { @@ -1818,9 +2183,9 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, nghttp2_nv *nv_out, } if (inflater->opcode == NGHTTP2_HD_OPCODE_NEWNAME) { - rv = hd_inflate_commit_newname(inflater, nv_out); + rv = hd_inflate_commit_newname(inflater, nv_out, token_out); } else { - rv = hd_inflate_commit_indname(inflater, nv_out); + rv = hd_inflate_commit_indname(inflater, nv_out, token_out); } if (rv != 0) { diff --git a/lib/nghttp2_hd.h b/lib/nghttp2_hd.h index ef9fa3ac..2d53d57a 100644 --- a/lib/nghttp2_hd.h +++ b/lib/nghttp2_hd.h @@ -49,7 +49,67 @@ #define NGHTTP2_HD_DEFAULT_MAX_DEFLATE_BUFFER_SIZE (1 << 12) /* Exported for unit test */ -extern const size_t NGHTTP2_STATIC_TABLE_LENGTH; +#define NGHTTP2_STATIC_TABLE_LENGTH 61 + +typedef enum { + NGHTTP2_TOKEN__AUTHORITY = 0, + NGHTTP2_TOKEN__METHOD = 1, + NGHTTP2_TOKEN__PATH = 3, + NGHTTP2_TOKEN__SCHEME = 5, + NGHTTP2_TOKEN__STATUS = 7, + NGHTTP2_TOKEN_ACCEPT_CHARSET = 14, + NGHTTP2_TOKEN_ACCEPT_ENCODING = 15, + NGHTTP2_TOKEN_ACCEPT_LANGUAGE = 16, + NGHTTP2_TOKEN_ACCEPT_RANGES = 17, + NGHTTP2_TOKEN_ACCEPT = 18, + NGHTTP2_TOKEN_ACCESS_CONTROL_ALLOW_ORIGIN = 19, + NGHTTP2_TOKEN_AGE = 20, + NGHTTP2_TOKEN_ALLOW = 21, + NGHTTP2_TOKEN_AUTHORIZATION = 22, + NGHTTP2_TOKEN_CACHE_CONTROL = 23, + NGHTTP2_TOKEN_CONTENT_DISPOSITION = 24, + NGHTTP2_TOKEN_CONTENT_ENCODING = 25, + NGHTTP2_TOKEN_CONTENT_LANGUAGE = 26, + NGHTTP2_TOKEN_CONTENT_LENGTH = 27, + NGHTTP2_TOKEN_CONTENT_LOCATION = 28, + NGHTTP2_TOKEN_CONTENT_RANGE = 29, + NGHTTP2_TOKEN_CONTENT_TYPE = 30, + NGHTTP2_TOKEN_COOKIE = 31, + NGHTTP2_TOKEN_DATE = 32, + NGHTTP2_TOKEN_ETAG = 33, + NGHTTP2_TOKEN_EXPECT = 34, + NGHTTP2_TOKEN_EXPIRES = 35, + NGHTTP2_TOKEN_FROM = 36, + NGHTTP2_TOKEN_HOST = 37, + NGHTTP2_TOKEN_IF_MATCH = 38, + NGHTTP2_TOKEN_IF_MODIFIED_SINCE = 39, + NGHTTP2_TOKEN_IF_NONE_MATCH = 40, + NGHTTP2_TOKEN_IF_RANGE = 41, + NGHTTP2_TOKEN_IF_UNMODIFIED_SINCE = 42, + NGHTTP2_TOKEN_LAST_MODIFIED = 43, + NGHTTP2_TOKEN_LINK = 44, + NGHTTP2_TOKEN_LOCATION = 45, + NGHTTP2_TOKEN_MAX_FORWARDS = 46, + NGHTTP2_TOKEN_PROXY_AUTHENTICATE = 47, + NGHTTP2_TOKEN_PROXY_AUTHORIZATION = 48, + NGHTTP2_TOKEN_RANGE = 49, + NGHTTP2_TOKEN_REFERER = 50, + NGHTTP2_TOKEN_REFRESH = 51, + NGHTTP2_TOKEN_RETRY_AFTER = 52, + NGHTTP2_TOKEN_SERVER = 53, + NGHTTP2_TOKEN_SET_COOKIE = 54, + NGHTTP2_TOKEN_STRICT_TRANSPORT_SECURITY = 55, + NGHTTP2_TOKEN_TRANSFER_ENCODING = 56, + NGHTTP2_TOKEN_USER_AGENT = 57, + NGHTTP2_TOKEN_VARY = 58, + NGHTTP2_TOKEN_VIA = 59, + NGHTTP2_TOKEN_WWW_AUTHENTICATE = 60, + NGHTTP2_TOKEN_TE, + NGHTTP2_TOKEN_CONNECTION, + NGHTTP2_TOKEN_KEEP_ALIVE, + NGHTTP2_TOKEN_PROXY_CONNECTION, + NGHTTP2_TOKEN_UPGRADE, +} nghttp2_token; typedef enum { NGHTTP2_HD_FLAG_NONE = 0, @@ -67,18 +127,14 @@ typedef enum { typedef struct { nghttp2_nv nv; - uint32_t name_hash; - uint32_t value_hash; + /* nghttp2_token value for nv.name. It could be -1 if we have no + token for that header field name. */ + int token; /* Reference count */ uint8_t ref; uint8_t flags; } nghttp2_hd_entry; -typedef struct { - nghttp2_hd_entry ent; - size_t index; -} nghttp2_hd_static_entry; - typedef struct { nghttp2_hd_entry **buffer; size_t mask; @@ -182,9 +238,8 @@ struct nghttp2_hd_inflater { * set in the |flags|, the content pointed by the |name| with length * |namelen| is copied. Likewise, if NGHTTP2_HD_FLAG_VALUE_ALLOC bit * set in the |flags|, the content pointed by the |value| with length - * |valuelen| is copied. The |name_hash| and |value_hash| are hash - * value for |name| and |value| respectively. The hash function is - * defined in nghttp2_hd.c. + * |valuelen| is copied. The |token| is enum number looked up by + * |name|. It could be -1 if we don't have that enum value. * * This function returns 0 if it succeeds, or one of the following * negative error codes: @@ -194,8 +249,7 @@ struct nghttp2_hd_inflater { */ int nghttp2_hd_entry_init(nghttp2_hd_entry *ent, uint8_t flags, uint8_t *name, size_t namelen, uint8_t *value, size_t valuelen, - uint32_t name_hash, uint32_t value_hash, - nghttp2_mem *mem); + int token, nghttp2_mem *mem); void nghttp2_hd_entry_free(nghttp2_hd_entry *ent, nghttp2_mem *mem); @@ -277,6 +331,18 @@ int nghttp2_hd_inflate_init(nghttp2_hd_inflater *inflater, nghttp2_mem *mem); */ void nghttp2_hd_inflate_free(nghttp2_hd_inflater *inflater); +/* + * Similar to nghttp2_hd_inflate_hd(), but this takes additional + * output parameter |token|. On successful header emission, it + * contains nghttp2_token value for nv_out->name. It could be -1 if + * we don't have enum value for the name. Other than that return + * values and semantics are the same as nghttp2_hd_inflate_hd(). + */ +ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater, + nghttp2_nv *nv_out, int *inflate_flags, + int *token, uint8_t *in, size_t inlen, + int in_final); + /* For unittesting purpose */ int nghttp2_hd_emit_indname_block(nghttp2_bufs *bufs, size_t index, nghttp2_nv *nv, int indexing_mode); diff --git a/lib/nghttp2_helper.h b/lib/nghttp2_helper.h index 54422a8f..0a7f8c81 100644 --- a/lib/nghttp2_helper.h +++ b/lib/nghttp2_helper.h @@ -29,12 +29,16 @@ #include #endif /* HAVE_CONFIG_H */ +#include + #include #include "nghttp2_mem.h" #define nghttp2_min(A, B) ((A) < (B) ? (A) : (B)) #define nghttp2_max(A, B) ((A) > (B) ? (A) : (B)) +#define lstreq(A, B, N) ((sizeof((A)) - 1) == (N) && memcmp((A), (B), (N)) == 0) + /* * Copies 2 byte unsigned integer |n| in host byte order to |buf| in * network byte order. diff --git a/lib/nghttp2_http.c b/lib/nghttp2_http.c index ae49fd63..695307e1 100644 --- a/lib/nghttp2_http.c +++ b/lib/nghttp2_http.c @@ -28,11 +28,8 @@ #include #include -static int memeq(const void *a, const void *b, size_t n) { - return memcmp(a, b, n) == 0; -} - -#define streq(A, B, N) ((sizeof((A)) - 1) == (N) && memeq((A), (B), (N))) +#include "nghttp2_hd.h" +#include "nghttp2_helper.h" static char downcase(char c) { return 'A' <= c && c <= 'Z' ? (c - 'A' + 'a') : c; @@ -50,129 +47,7 @@ static int memieq(const void *a, const void *b, size_t n) { return 1; } -#define strieq(A, B, N) ((sizeof((A)) - 1) == (N) && memieq((A), (B), (N))) - -typedef enum { - NGHTTP2_TOKEN__AUTHORITY, - NGHTTP2_TOKEN__METHOD, - NGHTTP2_TOKEN__PATH, - NGHTTP2_TOKEN__SCHEME, - NGHTTP2_TOKEN__STATUS, - NGHTTP2_TOKEN_CONNECTION, - NGHTTP2_TOKEN_CONTENT_LENGTH, - NGHTTP2_TOKEN_HOST, - NGHTTP2_TOKEN_KEEP_ALIVE, - NGHTTP2_TOKEN_PROXY_CONNECTION, - NGHTTP2_TOKEN_TE, - NGHTTP2_TOKEN_TRANSFER_ENCODING, - NGHTTP2_TOKEN_UPGRADE, - NGHTTP2_TOKEN_MAXIDX, -} nghttp2_token; - -/* - * This function was generated by genlibtokenlookup.py. Inspired by - * h2o header lookup. https://github.com/h2o/h2o - */ -static int lookup_token(const uint8_t *name, size_t namelen) { - switch (namelen) { - case 2: - switch (name[1]) { - case 'e': - if (streq("t", name, 1)) { - return NGHTTP2_TOKEN_TE; - } - break; - } - break; - case 4: - switch (name[3]) { - case 't': - if (streq("hos", name, 3)) { - return NGHTTP2_TOKEN_HOST; - } - break; - } - break; - case 5: - switch (name[4]) { - case 'h': - if (streq(":pat", name, 4)) { - return NGHTTP2_TOKEN__PATH; - } - break; - } - break; - case 7: - switch (name[6]) { - case 'd': - if (streq(":metho", name, 6)) { - return NGHTTP2_TOKEN__METHOD; - } - break; - case 'e': - if (streq(":schem", name, 6)) { - return NGHTTP2_TOKEN__SCHEME; - } - if (streq("upgrad", name, 6)) { - return NGHTTP2_TOKEN_UPGRADE; - } - break; - case 's': - if (streq(":statu", name, 6)) { - return NGHTTP2_TOKEN__STATUS; - } - break; - } - break; - case 10: - switch (name[9]) { - case 'e': - if (streq("keep-aliv", name, 9)) { - return NGHTTP2_TOKEN_KEEP_ALIVE; - } - break; - case 'n': - if (streq("connectio", name, 9)) { - return NGHTTP2_TOKEN_CONNECTION; - } - break; - case 'y': - if (streq(":authorit", name, 9)) { - return NGHTTP2_TOKEN__AUTHORITY; - } - break; - } - break; - case 14: - switch (name[13]) { - case 'h': - if (streq("content-lengt", name, 13)) { - return NGHTTP2_TOKEN_CONTENT_LENGTH; - } - break; - } - break; - case 16: - switch (name[15]) { - case 'n': - if (streq("proxy-connectio", name, 15)) { - return NGHTTP2_TOKEN_PROXY_CONNECTION; - } - break; - } - break; - case 17: - switch (name[16]) { - case 'g': - if (streq("transfer-encodin", name, 16)) { - return NGHTTP2_TOKEN_TRANSFER_ENCODING; - } - break; - } - break; - } - return -1; -} +#define lstrieq(A, B, N) ((sizeof((A)) - 1) == (N) && memieq((A), (B), (N))) static int64_t parse_uint(const uint8_t *s, size_t len) { int64_t n = 0; @@ -238,9 +113,7 @@ static int check_path(nghttp2_stream *stream) { } static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv, - int trailer) { - int token; - + int token, int trailer) { if (nv->name[0] == ':') { if (trailer || (stream->http_flags & NGHTTP2_HTTP_FLAG_PSEUDO_HEADER_DISALLOWED)) { @@ -248,8 +121,6 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv, } } - token = lookup_token(nv->name, nv->namelen); - switch (token) { case NGHTTP2_TOKEN__AUTHORITY: if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__AUTHORITY)) { @@ -262,14 +133,14 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv, } switch (nv->valuelen) { case 4: - if (streq("HEAD", nv->value, nv->valuelen)) { + if (lstreq("HEAD", nv->value, nv->valuelen)) { stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_HEAD; } break; case 7: switch (nv->value[6]) { case 'T': - if (streq("CONNECT", nv->value, nv->valuelen)) { + if (lstreq("CONNECT", nv->value, nv->valuelen)) { if (stream->stream_id % 2 == 0) { /* we won't allow CONNECT for push */ return NGHTTP2_ERR_HTTP_HEADER; @@ -282,7 +153,7 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv, } break; case 'S': - if (streq("OPTIONS", nv->value, nv->valuelen)) { + if (lstreq("OPTIONS", nv->value, nv->valuelen)) { stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_OPTIONS; } break; @@ -338,7 +209,7 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv, case NGHTTP2_TOKEN_UPGRADE: return NGHTTP2_ERR_HTTP_HEADER; case NGHTTP2_TOKEN_TE: - if (!strieq("trailers", nv->value, nv->valuelen)) { + if (!lstrieq("trailers", nv->value, nv->valuelen)) { return NGHTTP2_ERR_HTTP_HEADER; } break; @@ -356,9 +227,7 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv, } static int http_response_on_header(nghttp2_stream *stream, nghttp2_nv *nv, - int trailer) { - int token; - + int token, int trailer) { if (nv->name[0] == ':') { if (trailer || (stream->http_flags & NGHTTP2_HTTP_FLAG_PSEUDO_HEADER_DISALLOWED)) { @@ -366,8 +235,6 @@ static int http_response_on_header(nghttp2_stream *stream, nghttp2_nv *nv, } } - token = lookup_token(nv->name, nv->namelen); - switch (token) { case NGHTTP2_TOKEN__STATUS: { if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__STATUS)) { @@ -400,7 +267,7 @@ static int http_response_on_header(nghttp2_stream *stream, nghttp2_nv *nv, case NGHTTP2_TOKEN_UPGRADE: return NGHTTP2_ERR_HTTP_HEADER; case NGHTTP2_TOKEN_TE: - if (!strieq("trailers", nv->value, nv->valuelen)) { + if (!lstrieq("trailers", nv->value, nv->valuelen)) { return NGHTTP2_ERR_HTTP_HEADER; } break; @@ -418,7 +285,8 @@ static int http_response_on_header(nghttp2_stream *stream, nghttp2_nv *nv, } int nghttp2_http_on_header(nghttp2_session *session, nghttp2_stream *stream, - nghttp2_frame *frame, nghttp2_nv *nv, int trailer) { + nghttp2_frame *frame, nghttp2_nv *nv, int token, + int trailer) { /* We are strict for pseudo header field. One bad character should lead to fail. OTOH, we should be a bit forgiving for regular headers, since existing public internet has so much illegal @@ -458,10 +326,10 @@ int nghttp2_http_on_header(nghttp2_session *session, nghttp2_stream *stream, } if (session->server || frame->hd.type == NGHTTP2_PUSH_PROMISE) { - return http_request_on_header(stream, nv, trailer); + return http_request_on_header(stream, nv, token, trailer); } - return http_response_on_header(stream, nv, trailer); + return http_response_on_header(stream, nv, token, trailer); } int nghttp2_http_on_request_headers(nghttp2_stream *stream, @@ -574,14 +442,15 @@ void nghttp2_http_record_request_method(nghttp2_stream *stream, /* TODO we should do this strictly. */ for (i = 0; i < nvlen; ++i) { const nghttp2_nv *nv = &nva[i]; - if (lookup_token(nv->name, nv->namelen) != NGHTTP2_TOKEN__METHOD) { + if (!(nv->namelen == 7 && nv->name[6] == 'd' && + memcmp(":metho", nv->name, nv->namelen - 1) == 0)) { continue; } - if (streq("CONNECT", nv->value, nv->valuelen)) { + if (lstreq("CONNECT", nv->value, nv->valuelen)) { stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_CONNECT; return; } - if (streq("HEAD", nv->value, nv->valuelen)) { + if (lstreq("HEAD", nv->value, nv->valuelen)) { stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_HEAD; return; } diff --git a/lib/nghttp2_http.h b/lib/nghttp2_http.h index f7966c67..f782058f 100644 --- a/lib/nghttp2_http.h +++ b/lib/nghttp2_http.h @@ -36,7 +36,8 @@ /* * This function is called when HTTP header field |nv| in |frame| is * received for |stream|. This function will validate |nv| against - * the current state of stream. + * the current state of stream. The |token| is nghttp2_token value + * for nv->name, or -1 if we don't have enum value for the name. * * This function returns 0 if it succeeds, or one of the following * negative error codes: @@ -48,7 +49,8 @@ * if it was not received because of compatibility reasons. */ int nghttp2_http_on_header(nghttp2_session *session, nghttp2_stream *stream, - nghttp2_frame *frame, nghttp2_nv *nv, int trailer); + nghttp2_frame *frame, nghttp2_nv *nv, int token, + int trailer); /* * This function is called when request header is received. This diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 238b9d00..a8c4d857 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3230,6 +3230,7 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame, nghttp2_stream *stream; nghttp2_stream *subject_stream; int trailer = 0; + int token; *readlen_ptr = 0; stream = nghttp2_session_get_stream(session, frame->hd.stream_id); @@ -3245,8 +3246,8 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame, DEBUGF(fprintf(stderr, "recv: decoding header block %zu bytes\n", inlen)); for (;;) { inflate_flags = 0; - proclen = nghttp2_hd_inflate_hd(&session->hd_inflater, &nv, &inflate_flags, - in, inlen, final); + proclen = nghttp2_hd_inflate_hd2(&session->hd_inflater, &nv, &inflate_flags, + &token, in, inlen, final); if (nghttp2_is_fatal((int)proclen)) { return (int)proclen; } @@ -3281,7 +3282,7 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame, if (call_header_cb && (inflate_flags & NGHTTP2_HD_INFLATE_EMIT)) { rv = 0; if (subject_stream && session_enforce_http_messaging(session)) { - rv = nghttp2_http_on_header(session, subject_stream, frame, &nv, + rv = nghttp2_http_on_header(session, subject_stream, frame, &nv, token, trailer); if (rv == NGHTTP2_ERR_HTTP_HEADER) { DEBUGF(fprintf( diff --git a/mkstatichdtbl.py b/mkstatichdtbl.py index b6b6e38b..fbe97a61 100755 --- a/mkstatichdtbl.py +++ b/mkstatichdtbl.py @@ -10,39 +10,17 @@ from __future__ import unicode_literals import re, sys -def hash(s): - h = 0 - for c in s: - h = h * 31 + ord(c) - return h & ((1 << 32) - 1) - entries = [] for line in sys.stdin: m = re.match(r'(\d+)\s+(\S+)\s+(\S.*)?', line) val = m.group(3).strip() if m.group(3) else '' - entries.append((hash(m.group(2)), int(m.group(1)), m.group(2), val)) - -entries.sort() - -print '/* Sorted by hash(name) and its table index */' -print 'static nghttp2_hd_static_entry static_table[] = {' -for ent in entries: - print 'MAKE_STATIC_ENT({}, "{}", "{}", {}u, {}u),'\ - .format(ent[1] - 1, ent[2], ent[3], ent[0], hash(ent[3])) -print '};' - -print '' - -print '/* Index to the position in static_table */' -print 'const size_t static_table_index[] = {' -for i in range(len(entries)): - for j, ent in enumerate(entries): - if ent[1] - 1 == i: - sys.stdout.write('{: <2d},'.format(j)) - break - if (i + 1) % 16 == 0: - sys.stdout.write('\n') - else: - sys.stdout.write(' ') + entries.append((int(m.group(1)), m.group(2), val)) +print 'static nghttp2_hd_entry static_table[] = {' +idx = 0 +for i, ent in enumerate(entries): + if entries[idx][1] != ent[1]: + idx = i + print 'MAKE_STATIC_ENT("{}", "{}", {}),'\ + .format(ent[1], ent[2], entries[idx][0] - 1) print '};' From dc335b9025a5e5d220a42aecaec27030902bf1b2 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 16 Apr 2015 21:38:13 +0900 Subject: [PATCH 27/50] Improve weight handling a bit --- lib/nghttp2_outbound_item.h | 24 ++++++------ lib/nghttp2_session.c | 73 ++++++++++++------------------------- lib/nghttp2_session.h | 3 +- lib/nghttp2_stream.c | 10 ++--- src/nghttp.cc | 5 ++- 5 files changed, 45 insertions(+), 70 deletions(-) diff --git a/lib/nghttp2_outbound_item.h b/lib/nghttp2_outbound_item.h index b400babe..9d225ced 100644 --- a/lib/nghttp2_outbound_item.h +++ b/lib/nghttp2_outbound_item.h @@ -33,12 +33,12 @@ #include "nghttp2_frame.h" #include "nghttp2_mem.h" -/* A bit higher weight for non-DATA frames */ -#define NGHTTP2_OB_EX_WEIGHT 300 -/* Higher weight for SETTINGS */ -#define NGHTTP2_OB_SETTINGS_WEIGHT 301 -/* Highest weight for PING */ -#define NGHTTP2_OB_PING_WEIGHT 302 +/* A bit higher priority for non-DATA frames */ +#define NGHTTP2_OB_EX_CYCLE 2 +/* Even more higher priority for SETTINGS frame */ +#define NGHTTP2_OB_SETTINGS_CYCLE 1 +/* Highest priority for PING frame */ +#define NGHTTP2_OB_PING_CYCLE 0 /* struct used for HEADERS and PUSH_PROMISE frame */ typedef struct { @@ -108,12 +108,14 @@ typedef struct { nghttp2_frame frame; nghttp2_aux_data aux_data; int64_t seq; - /* Reset count of weight. See comment for last_cycle in - nghttp2_session.h */ + /* The priority used in priority comparion. Smaller is served + ealier. For PING, SETTINGS and non-DATA frames (excluding + response HEADERS frame) have dedicated cycle value defined above. + For DATA frame, cycle is computed by taking into account of + effective weight and frame payload length previously sent, so + that the amount of transmission is distributed across streams + proportional to effective weight (inside a tree). */ uint64_t cycle; - /* The priority used in priority comparion. Larger is served - ealier. */ - int32_t weight; /* nonzero if this object is queued. */ uint8_t queued; } nghttp2_outbound_item; diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index a8c4d857..02fe062c 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -228,12 +228,7 @@ static int outbound_item_compar(const void *lhsx, const void *rhsx) { rhs = (const nghttp2_outbound_item *)rhsx; if (lhs->cycle == rhs->cycle) { - if (lhs->weight == rhs->weight) { - return (lhs->seq < rhs->seq) ? -1 : ((lhs->seq > rhs->seq) ? 1 : 0); - } - - /* Larger weight has higher precedence */ - return rhs->weight - lhs->weight; + return (lhs->seq < rhs->seq) ? -1 : ((lhs->seq > rhs->seq) ? 1 : 0); } return (lhs->cycle < rhs->cycle) ? -1 : 1; @@ -367,7 +362,9 @@ static int session_new(nghttp2_session **session_ptr, nghttp2_stream_roots_init(&(*session_ptr)->roots); (*session_ptr)->next_seq = 0; - (*session_ptr)->last_cycle = 1; + /* Do +1 so that any HEADERS/DATA frames are scheduled after urgent + frames. */ + (*session_ptr)->last_cycle = NGHTTP2_OB_EX_CYCLE + 1; (*session_ptr)->remote_window_size = NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE; (*session_ptr)->recv_window_size = 0; @@ -694,9 +691,7 @@ nghttp2_session_reprioritize_stream(nghttp2_session *session, void nghttp2_session_outbound_item_init(nghttp2_session *session, nghttp2_outbound_item *item) { item->seq = session->next_seq++; - /* We use cycle for DATA only */ - item->cycle = 0; - item->weight = NGHTTP2_OB_EX_WEIGHT; + item->cycle = NGHTTP2_OB_EX_CYCLE; item->queued = 0; memset(&item->aux_data, 0, sizeof(nghttp2_aux_data)); @@ -723,12 +718,12 @@ int nghttp2_session_add_item(nghttp2_session *session, break; case NGHTTP2_SETTINGS: - item->weight = NGHTTP2_OB_SETTINGS_WEIGHT; + item->cycle = NGHTTP2_OB_SETTINGS_CYCLE; break; case NGHTTP2_PING: /* Ping has highest priority. */ - item->weight = NGHTTP2_OB_PING_WEIGHT; + item->cycle = NGHTTP2_OB_PING_CYCLE; break; default: @@ -752,7 +747,6 @@ int nghttp2_session_add_item(nghttp2_session *session, item->queued = 1; } else if (stream && (stream->state == NGHTTP2_STREAM_RESERVED || item->aux_data.headers.attach_stream)) { - item->weight = stream->effective_weight; item->cycle = session->last_cycle; rv = nghttp2_stream_attach_item(stream, item, session); @@ -790,7 +784,6 @@ int nghttp2_session_add_item(nghttp2_session *session, return NGHTTP2_ERR_DATA_EXIST; } - item->weight = stream->effective_weight; item->cycle = session->last_cycle; rv = nghttp2_stream_attach_item(stream, item, session); @@ -1986,7 +1979,8 @@ static int session_prep_frame(nghttp2_session *session, } rv = nghttp2_session_pack_data(session, &session->aob.framebufs, - next_readmax, frame, &item->aux_data.data); + next_readmax, frame, &item->aux_data.data, + stream); if (rv == NGHTTP2_ERR_DEFERRED) { rv = nghttp2_stream_defer_item(stream, NGHTTP2_STREAM_FLAG_DEFERRED_USER, session); @@ -2069,8 +2063,7 @@ nghttp2_session_get_next_ob_item(nghttp2_session *session) { headers_item = nghttp2_pq_top(&session->ob_ss_pq); if (session_is_outgoing_concurrent_streams_max(session) || - item->weight > headers_item->weight || - (item->weight == headers_item->weight && item->seq < headers_item->seq)) { + outbound_item_compar(item, headers_item) < 0) { return item; } @@ -2133,8 +2126,7 @@ nghttp2_session_pop_next_ob_item(nghttp2_session *session) { headers_item = nghttp2_pq_top(&session->ob_ss_pq); if (session_is_outgoing_concurrent_streams_max(session) || - item->weight > headers_item->weight || - (item->weight == headers_item->weight && item->seq < headers_item->seq)) { + outbound_item_compar(item, headers_item) < 0) { nghttp2_pq_pop(&session->ob_pq); item->queued = 0; @@ -2249,21 +2241,13 @@ static int session_close_stream_on_goaway(nghttp2_session *session, return 0; } -static void session_outbound_item_cycle_weight(nghttp2_session *session, - nghttp2_outbound_item *item, - int32_t ini_weight) { - if (item->weight == NGHTTP2_MIN_WEIGHT || item->weight > ini_weight) { +static void session_outbound_item_schedule(nghttp2_session *session, + nghttp2_outbound_item *item, + int32_t weight) { + size_t delta = item->frame.hd.length * NGHTTP2_MAX_WEIGHT / weight; - item->weight = ini_weight; - - if (item->cycle == session->last_cycle) { - item->cycle = ++session->last_cycle; - } else { - item->cycle = session->last_cycle; - } - } else { - --item->weight; - } + session->last_cycle = nghttp2_max(session->last_cycle, item->cycle); + item->cycle = session->last_cycle + delta; } /* @@ -2583,15 +2567,6 @@ static int session_after_frame_sent2(nghttp2_session *session) { assert(stream); next_item = nghttp2_session_get_next_ob_item(session); - /* Imagine we hit connection window size limit while sending DATA - frame. If we decrement weight here, its stream might get - inferior share because the other streams' weight is not - decremented because of flow control. */ - if (session->remote_window_size > 0 || stream->remote_window_size <= 0) { - session_outbound_item_cycle_weight(session, aob->item, - stream->effective_weight); - } - /* If priority of this stream is higher or equal to other stream waiting at the top of the queue, we continue to send this data. */ @@ -2634,7 +2609,7 @@ static int session_after_frame_sent2(nghttp2_session *session) { nghttp2_bufs_reset(framebufs); rv = nghttp2_session_pack_data(session, framebufs, next_readmax, frame, - aux_data); + aux_data, stream); if (nghttp2_is_fatal(rv)) { return rv; } @@ -6229,7 +6204,8 @@ int nghttp2_session_add_settings(nghttp2_session *session, uint8_t flags, int nghttp2_session_pack_data(nghttp2_session *session, nghttp2_bufs *bufs, size_t datamax, nghttp2_frame *frame, - nghttp2_data_aux_data *aux_data) { + nghttp2_data_aux_data *aux_data, + nghttp2_stream *stream) { int rv; uint32_t data_flags; ssize_t payloadlen; @@ -6242,12 +6218,6 @@ int nghttp2_session_pack_data(nghttp2_session *session, nghttp2_bufs *bufs, buf = &bufs->cur->buf; if (session->callbacks.read_length_callback) { - nghttp2_stream *stream; - - stream = nghttp2_session_get_stream(session, frame->hd.stream_id); - if (!stream) { - return NGHTTP2_ERR_INVALID_ARGUMENT; - } payloadlen = session->callbacks.read_length_callback( session, frame->hd.type, stream->stream_id, session->remote_window_size, @@ -6361,6 +6331,9 @@ int nghttp2_session_pack_data(nghttp2_session *session, nghttp2_bufs *bufs, return rv; } + session_outbound_item_schedule(session, stream->item, + stream->effective_weight); + return 0; } diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 94f990cc..56709ec6 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -704,7 +704,8 @@ nghttp2_stream *nghttp2_session_get_stream_raw(nghttp2_session *session, */ int nghttp2_session_pack_data(nghttp2_session *session, nghttp2_bufs *bufs, size_t datamax, nghttp2_frame *frame, - nghttp2_data_aux_data *aux_data); + nghttp2_data_aux_data *aux_data, + nghttp2_stream *stream); /* * Returns top of outbound frame queue. This function returns NULL if diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 09b70d79..16981efc 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -102,17 +102,15 @@ static int stream_push_item(nghttp2_stream *stream, nghttp2_session *session) { return 0; } - if (item->weight > stream->effective_weight) { - item->weight = stream->effective_weight; - } - - item->cycle = session->last_cycle; - switch (item->frame.hd.type) { case NGHTTP2_DATA: + /* Penalize low weight stream */ + item->cycle = + session->last_cycle + NGHTTP2_MAX_WEIGHT / stream->effective_weight; rv = nghttp2_pq_push(&session->ob_da_pq, item); break; case NGHTTP2_HEADERS: + item->cycle = session->last_cycle; if (stream->state == NGHTTP2_STREAM_RESERVED) { rv = nghttp2_pq_push(&session->ob_ss_pq, item); } else { diff --git a/src/nghttp.cc b/src/nghttp.cc index 05bd22ac..3d513f4f 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -2062,11 +2062,12 @@ int communicate( dep_stream_id = ANCHOR_ID_HIGH; } - nghttp2_priority_spec_init(&pri_spec, dep_stream_id, config.weight, 0); - for (auto req : requests) { for (int i = 0; i < config.multiply; ++i) { auto dep = std::make_shared(); + nghttp2_priority_spec_init(&pri_spec, dep_stream_id, + config.weight * (i + 1), 0); + client.add_request(std::get<0>(req), std::get<1>(req), std::get<2>(req), pri_spec, std::move(dep)); } From 1a8da6caec20e2e3491fd611597a1b061f6861ff Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 16 Apr 2015 21:40:39 +0900 Subject: [PATCH 28/50] Update doc --- lib/nghttp2_stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 16981efc..d0e875c1 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -354,7 +354,7 @@ static int stream_update_dep_queue_top(nghttp2_stream *stream, * work, this function returns 1 if any of its descendants has dpri * value of NGHTTP2_STREAM_DPRI_TOP, otherwise 0. * - * Calculating stream->sum_top-weight is very simple compared to + * Calculating stream->sum_top_weight is very simple compared to * stream->sum_norest_weight. It just adds up the weight of direct * descendants whose dpri is NGHTTP2_STREAM_DPRI_TOP. */ From 8f4e2d941f9d740ec193b7427f4f52d384997f96 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 16 Apr 2015 22:36:31 +0900 Subject: [PATCH 29/50] Revert accidental change in nghttp.cc --- src/nghttp.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/nghttp.cc b/src/nghttp.cc index 3d513f4f..05bd22ac 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -2062,12 +2062,11 @@ int communicate( dep_stream_id = ANCHOR_ID_HIGH; } + nghttp2_priority_spec_init(&pri_spec, dep_stream_id, config.weight, 0); + for (auto req : requests) { for (int i = 0; i < config.multiply; ++i) { auto dep = std::make_shared(); - nghttp2_priority_spec_init(&pri_spec, dep_stream_id, - config.weight * (i + 1), 0); - client.add_request(std::get<0>(req), std::get<1>(req), std::get<2>(req), pri_spec, std::move(dep)); } From d4a22edeb34aa5772980ae32b41b8bf6af9cede6 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 16 Apr 2015 22:37:39 +0900 Subject: [PATCH 30/50] Don't do fancy stuff yet --- lib/nghttp2_stream.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index d0e875c1..3e9d63d1 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -102,15 +102,13 @@ static int stream_push_item(nghttp2_stream *stream, nghttp2_session *session) { return 0; } + item->cycle = session->last_cycle; + switch (item->frame.hd.type) { case NGHTTP2_DATA: - /* Penalize low weight stream */ - item->cycle = - session->last_cycle + NGHTTP2_MAX_WEIGHT / stream->effective_weight; rv = nghttp2_pq_push(&session->ob_da_pq, item); break; case NGHTTP2_HEADERS: - item->cycle = session->last_cycle; if (stream->state == NGHTTP2_STREAM_RESERVED) { rv = nghttp2_pq_push(&session->ob_ss_pq, item); } else { From e6ad2eb14fba0b5120f77e98c891eae7d35254bd Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 16 Apr 2015 22:41:00 +0900 Subject: [PATCH 31/50] We can assert this --- lib/nghttp2_session.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 02fe062c..9992c45a 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -2246,7 +2246,9 @@ static void session_outbound_item_schedule(nghttp2_session *session, int32_t weight) { size_t delta = item->frame.hd.length * NGHTTP2_MAX_WEIGHT / weight; - session->last_cycle = nghttp2_max(session->last_cycle, item->cycle); + assert(session->last_cycle <= item->cycle); + + session->last_cycle = item->cycle; item->cycle = session->last_cycle + delta; } From e23225689f87ae0644521304d99307817630674d Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 16 Apr 2015 23:42:36 +0900 Subject: [PATCH 32/50] nghttp: Use same priority anchor nodes as Firefox does --- src/HtmlParser.cc | 9 ++--- src/HtmlParser.h | 9 ++--- src/nghttp.cc | 90 ++++++++++++++++++++++++++--------------------- 3 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/HtmlParser.cc b/src/HtmlParser.cc index 8b5f4096..26559cce 100644 --- a/src/HtmlParser.cc +++ b/src/HtmlParser.cc @@ -75,22 +75,23 @@ void start_element_func(void *user_data, const xmlChar *name, return; } if (util::strieq(rel_attr, "shortcut icon")) { - add_link(parser_data, href_attr, REQ_PRI_LOWEST); + add_link(parser_data, href_attr, REQ_OTHERS); } else if (util::strieq(rel_attr, "stylesheet")) { - add_link(parser_data, href_attr, REQ_PRI_MEDIUM); + add_link(parser_data, href_attr, REQ_CSS); } } else if (util::strieq(reinterpret_cast(name), "img")) { auto src_attr = get_attr(attrs, "src"); if (!src_attr) { return; } - add_link(parser_data, src_attr, REQ_PRI_LOWEST); + add_link(parser_data, src_attr, REQ_IMG); } else if (util::strieq(reinterpret_cast(name), "script")) { auto src_attr = get_attr(attrs, "src"); if (!src_attr) { return; } - add_link(parser_data, src_attr, REQ_PRI_LOW); + // TODO if script is inside in head, this should be REQ_LEADERS. + add_link(parser_data, src_attr, REQ_UNBLOCK_JS); } } } // namespace diff --git a/src/HtmlParser.h b/src/HtmlParser.h index bee87d39..6d438212 100644 --- a/src/HtmlParser.h +++ b/src/HtmlParser.h @@ -38,11 +38,12 @@ namespace nghttp2 { +// Lower value has higher priority enum RequestPriority { - REQ_PRI_HIGH = 0, - REQ_PRI_MEDIUM = 1, - REQ_PRI_LOW = 2, - REQ_PRI_LOWEST = 3 + REQ_CSS = 1, + REQ_UNBLOCK_JS, + REQ_IMG, + REQ_OTHERS, }; struct ParserData { diff --git a/src/nghttp.cc b/src/nghttp.cc index 05bd22ac..8f27e9a8 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -61,17 +61,33 @@ namespace nghttp2 { -// stream ID of anchor stream node when --dep-idle is enabled. These -// * portion of ANCHOR_ID_* matches RequestPriority in HtmlParser.h. -// The stream ID = 1 is excluded since it is used as first stream in -// upgrade case. -enum { - ANCHOR_ID_HIGH = 3, - ANCHOR_ID_MEDIUM = 5, - ANCHOR_ID_LOW = 7, - ANCHOR_ID_LOWEST = 9, +// The anchor stream nodes when --dep-idle is enabled. The stream ID +// = 1 is excluded since it is used as first stream in upgrade case. +// We follows the same dependency anchor nodes as Firefox does. +struct Anchor { + int32_t stream_id; + // stream ID this anchor depends on + int32_t dep_stream_id; + // .. with this weight. + int32_t weight; }; +// This is index into anchors. Firefox uses ANCHOR_FOLLOWERS for html +// file. +enum { + ANCHOR_LEADERS, + ANCHOR_UNBLOCKED, + ANCHOR_BACKGROUND, + ANCHOR_SPECULATIVE, + ANCHOR_FOLLOWERS, +}; + +namespace { +auto anchors = std::array{{ + {3, 0, 201}, {5, 0, 101}, {7, 0, 1}, {9, 7, 1}, {11, 3, 1}, +}}; +} // namespace + Config::Config() : output_upper_thres(1024 * 1024), padding(0), peer_max_concurrent_streams(NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS), @@ -176,22 +192,27 @@ nghttp2_priority_spec Request::resolve_dep(int32_t pri) { } if (config.dep_idle) { - int32_t anchor_id = 0; + int32_t anchor_id; + int32_t weight; switch (pri) { - case REQ_PRI_HIGH: - anchor_id = ANCHOR_ID_HIGH; + case REQ_CSS: + anchor_id = anchors[ANCHOR_LEADERS].stream_id; + weight = 2; break; - case REQ_PRI_MEDIUM: - anchor_id = ANCHOR_ID_MEDIUM; + case REQ_UNBLOCK_JS: + anchor_id = anchors[ANCHOR_UNBLOCKED].stream_id; + weight = 2; break; - case REQ_PRI_LOW: - anchor_id = ANCHOR_ID_LOW; - break; - case REQ_PRI_LOWEST: - anchor_id = ANCHOR_ID_LOWEST; + case REQ_IMG: + anchor_id = anchors[ANCHOR_FOLLOWERS].stream_id; + weight = 12; break; + default: + anchor_id = anchors[ANCHOR_FOLLOWERS].stream_id; + weight = 2; } - nghttp2_priority_spec_init(&pri_spec, anchor_id, NGHTTP2_DEFAULT_WEIGHT, 0); + + nghttp2_priority_spec_init(&pri_spec, anchor_id, weight, 0); return pri_spec; } @@ -980,23 +1001,19 @@ int HttpClient::connection_made() { if (!config.no_dep && config.dep_idle) { // Create anchor stream nodes nghttp2_priority_spec pri_spec; - int32_t dep_stream_id = 0; - for (auto stream_id : - {ANCHOR_ID_HIGH, ANCHOR_ID_MEDIUM, ANCHOR_ID_LOW, ANCHOR_ID_LOWEST}) { - - nghttp2_priority_spec_init(&pri_spec, dep_stream_id, - NGHTTP2_DEFAULT_WEIGHT, 0); - rv = nghttp2_submit_priority(session, NGHTTP2_FLAG_NONE, stream_id, + for (auto &anchor : anchors) { + nghttp2_priority_spec_init(&pri_spec, anchor.dep_stream_id, anchor.weight, + 0); + rv = nghttp2_submit_priority(session, NGHTTP2_FLAG_NONE, anchor.stream_id, &pri_spec); if (rv != 0) { return -1; } - - dep_stream_id = stream_id; } - rv = nghttp2_session_set_next_stream_id(session, ANCHOR_ID_LOWEST + 2); + rv = nghttp2_session_set_next_stream_id( + session, anchors[ANCHOR_FOLLOWERS].stream_id + 2); if (rv != 0) { return -1; } @@ -1004,7 +1021,8 @@ int HttpClient::connection_made() { if (need_upgrade()) { // Amend the priority because we cannot send priority in // HTTP/1.1 Upgrade. - nghttp2_priority_spec_init(&pri_spec, ANCHOR_ID_HIGH, config.weight, 0); + auto &anchor = anchors[ANCHOR_FOLLOWERS]; + nghttp2_priority_spec_init(&pri_spec, anchor.stream_id, config.weight, 0); rv = nghttp2_submit_priority(session, NGHTTP2_FLAG_NONE, 1, &pri_spec); if (rv != 0) { @@ -1272,14 +1290,6 @@ void HttpClient::record_connect_end_time() { } void HttpClient::request_done(Request *req) { - if (req->pri == 0 && req->dep) { - assert(req->dep->deps.empty()); - - req->dep->deps.push_back(std::vector{req}); - - return; - } - if (req->stream_id % 2 == 0) { return; } @@ -2059,7 +2069,7 @@ int communicate( int32_t dep_stream_id = 0; if (!config.no_dep && config.dep_idle) { - dep_stream_id = ANCHOR_ID_HIGH; + dep_stream_id = anchors[ANCHOR_FOLLOWERS].stream_id; } nghttp2_priority_spec_init(&pri_spec, dep_stream_id, config.weight, 0); From 7323d4c639d703767c092301cf75f5489e0b191b Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 00:12:47 +0900 Subject: [PATCH 33/50] Fix bug that nghttp2_session_set_next_stream_id accepts invalid stream_id --- lib/includes/nghttp2/nghttp2.h | 4 +++- lib/nghttp2_session.c | 10 +++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 7d75f606..6f601244 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -2707,7 +2707,9 @@ NGHTTP2_EXTERN uint32_t * * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT` * The |next_stream_id| is strictly less than the value - * `nghttp2_session_get_next_stream_id()` returns. + * `nghttp2_session_get_next_stream_id()` returns; or + * |next_stream_id| is invalid (e.g., even integer for client, or + * odd integer for server). */ NGHTTP2_EXTERN int nghttp2_session_set_next_stream_id(nghttp2_session *session, int32_t next_stream_id); diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 9992c45a..6bec4ea8 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -6622,11 +6622,19 @@ int nghttp2_session_consume_stream(nghttp2_session *session, int32_t stream_id, int nghttp2_session_set_next_stream_id(nghttp2_session *session, int32_t next_stream_id) { - if (next_stream_id < 0 || + if (next_stream_id <= 0 || session->next_stream_id > (uint32_t)next_stream_id) { return NGHTTP2_ERR_INVALID_ARGUMENT; } + if (session->server) { + if (next_stream_id % 2) { + return NGHTTP2_ERR_INVALID_ARGUMENT; + } + } else if (next_stream_id % 2 == 0) { + return NGHTTP2_ERR_INVALID_ARGUMENT; + } + session->next_stream_id = next_stream_id; return 0; } From 57644e0256bd235cebe279f8f8123193a177269a Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 16 Apr 2015 22:34:54 +0900 Subject: [PATCH 34/50] Effectively revert 03c4092862dc64a1e8e90b40a13b78d4792da7ae This is not mandated by spec. Also it may work badly with Firefox style dependency tree usage. --- lib/nghttp2_stream.c | 83 ++++++------------------------------ lib/nghttp2_stream.h | 3 -- tests/nghttp2_session_test.c | 12 +++--- 3 files changed, 20 insertions(+), 78 deletions(-) diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 3e9d63d1..88cefb51 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -63,7 +63,6 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, stream->effective_weight = stream->weight; stream->sum_dep_weight = 0; stream->sum_norest_weight = 0; - stream->sum_top_weight = 0; stream->roots = roots; stream->root_prev = NULL; @@ -174,18 +173,6 @@ int32_t nghttp2_stream_dep_distributed_effective_weight(nghttp2_stream *stream, return nghttp2_max(1, weight); } -static int32_t -stream_dep_distributed_top_effective_weight(nghttp2_stream *stream, - int32_t weight) { - if (stream->sum_top_weight == 0) { - return stream->effective_weight; - } - - weight = stream->effective_weight * weight / stream->sum_top_weight; - - return nghttp2_max(1, weight); -} - static void stream_update_dep_set_rest(nghttp2_stream *stream); /* Updates effective_weight of descendant streams in subtree of @@ -195,10 +182,9 @@ static void stream_update_dep_effective_weight(nghttp2_stream *stream) { nghttp2_stream *si; DEBUGF(fprintf(stderr, "stream: update_dep_effective_weight " - "stream(%p)=%d, weight=%d, sum_norest_weight=%d, " - "sum_top_weight=%d\n", + "stream(%p)=%d, weight=%d, sum_norest_weight=%d\n", stream, stream->stream_id, stream->weight, - stream->sum_norest_weight, stream->sum_top_weight)); + stream->sum_norest_weight)); /* stream->sum_norest_weight == 0 means there is no NGHTTP2_STREAM_DPRI_TOP under stream */ @@ -207,47 +193,13 @@ static void stream_update_dep_effective_weight(nghttp2_stream *stream) { return; } - /* If there is no direct descendant whose dpri is - NGHTTP2_STREAM_DPRI_TOP, indirect descendants have the chance to - send data, so recursively set weight for descendants. */ - if (stream->sum_top_weight == 0) { - for (si = stream->dep_next; si; si = si->sib_next) { - if (si->dpri != NGHTTP2_STREAM_DPRI_REST) { - si->effective_weight = - nghttp2_stream_dep_distributed_effective_weight(stream, si->weight); - } - - stream_update_dep_effective_weight(si); - } - return; - } - - /* If there is at least one direct descendant whose dpri is - NGHTTP2_STREAM_DPRI_TOP, we won't give a chance to indirect - descendants, since closed or blocked stream's weight is - distributed among its siblings */ for (si = stream->dep_next; si; si = si->sib_next) { - if (si->dpri == NGHTTP2_STREAM_DPRI_TOP) { + if (si->dpri != NGHTTP2_STREAM_DPRI_REST) { si->effective_weight = - stream_dep_distributed_top_effective_weight(stream, si->weight); - - DEBUGF(fprintf(stderr, "stream: stream=%d top eweight=%d\n", - si->stream_id, si->effective_weight)); - - continue; + nghttp2_stream_dep_distributed_effective_weight(stream, si->weight); } - if (si->dpri == NGHTTP2_STREAM_DPRI_NO_ITEM) { - DEBUGF(fprintf(stderr, "stream: stream=%d no_item, ignored\n", - si->stream_id)); - - /* Since we marked NGHTTP2_STREAM_DPRI_TOP under si, we make - them NGHTTP2_STREAM_DPRI_REST again. */ - stream_update_dep_set_rest(si->dep_next); - } else { - DEBUGF( - fprintf(stderr, "stream: stream=%d rest, ignored\n", si->stream_id)); - } + stream_update_dep_effective_weight(si); } } @@ -343,25 +295,20 @@ static int stream_update_dep_queue_top(nghttp2_stream *stream, } /* - * Updates stream->sum_norest_weight and stream->sum_top_weight - * recursively. We have to gather effective sum of weight of - * descendants. If stream->dpri == NGHTTP2_STREAM_DPRI_NO_ITEM, we - * have to go deeper and check that any of its descendants has dpri - * value of NGHTTP2_STREAM_DPRI_TOP. If so, we have to add weight of - * its direct descendants to stream->sum_norest_weight. To make this - * work, this function returns 1 if any of its descendants has dpri - * value of NGHTTP2_STREAM_DPRI_TOP, otherwise 0. - * - * Calculating stream->sum_top_weight is very simple compared to - * stream->sum_norest_weight. It just adds up the weight of direct - * descendants whose dpri is NGHTTP2_STREAM_DPRI_TOP. + * Updates stream->sum_norest_weight recursively. We have to gather + * effective sum of weight of descendants. If stream->dpri == + * NGHTTP2_STREAM_DPRI_NO_ITEM, we have to go deeper and check that + * any of its descendants has dpri value of NGHTTP2_STREAM_DPRI_TOP. + * If so, we have to add weight of its direct descendants to + * stream->sum_norest_weight. To make this work, this function + * returns 1 if any of its descendants has dpri value of + * NGHTTP2_STREAM_DPRI_TOP, otherwise 0. */ static int stream_update_dep_sum_norest_weight(nghttp2_stream *stream) { nghttp2_stream *si; int rv; stream->sum_norest_weight = 0; - stream->sum_top_weight = 0; if (stream->dpri == NGHTTP2_STREAM_DPRI_TOP) { return 1; @@ -379,10 +326,6 @@ static int stream_update_dep_sum_norest_weight(nghttp2_stream *stream) { rv = 1; stream->sum_norest_weight += si->weight; } - - if (si->dpri == NGHTTP2_STREAM_DPRI_TOP) { - stream->sum_top_weight += si->weight; - } } return rv; diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h index e6395f75..65a7f10d 100644 --- a/lib/nghttp2_stream.h +++ b/lib/nghttp2_stream.h @@ -217,9 +217,6 @@ struct nghttp2_stream { descendant with dpri == NGHTTP2_STREAM_DPRI_TOP. We use this value to calculate effective weight. */ int32_t sum_norest_weight; - /* sum of weight of direct descendants whose dpri value is - NGHTTP2_STREAM_DPRI_TOP */ - int32_t sum_top_weight; nghttp2_stream_state state; /* status code from remote server */ int16_t status_code; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 1a4589c9..a685918b 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -6152,11 +6152,12 @@ void test_nghttp2_session_stream_attach_item(void) { CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_ITEM == a->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == b->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_ITEM == c->dpri); - CU_ASSERT(NGHTTP2_STREAM_DPRI_REST == d->dpri); + CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == d->dpri); - CU_ASSERT(16 * 16 / 16 == b->effective_weight); + CU_ASSERT(16 * 16 / 32 == b->effective_weight); + CU_ASSERT(16 * 16 / 32 == d->effective_weight); - CU_ASSERT(0 == dd->queued); + CU_ASSERT(1 == dd->queued); nghttp2_stream_detach_item(b, session); @@ -6322,12 +6323,13 @@ void test_nghttp2_session_stream_attach_item_subtree(void) { CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_ITEM == a->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == b->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_ITEM == c->dpri); - CU_ASSERT(NGHTTP2_STREAM_DPRI_REST == d->dpri); + CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == d->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == e->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_ITEM == f->dpri); CU_ASSERT(16 == b->effective_weight); - CU_ASSERT(16 * 16 / 16 == e->effective_weight); + CU_ASSERT(16 * 16 / 32 == e->effective_weight); + CU_ASSERT(16 * 16 / 32 == e->effective_weight); CU_ASSERT(32 == a->sum_norest_weight); CU_ASSERT(16 == c->sum_norest_weight); From 1a12a9b397f424ab608537f6a8c3de7e69d0c529 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 21:17:22 +0900 Subject: [PATCH 35/50] Penalize cycle according to effective weight --- lib/nghttp2_session.c | 5 +++-- lib/nghttp2_stream.c | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 6bec4ea8..3d5a25a2 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -2246,9 +2246,10 @@ static void session_outbound_item_schedule(nghttp2_session *session, int32_t weight) { size_t delta = item->frame.hd.length * NGHTTP2_MAX_WEIGHT / weight; - assert(session->last_cycle <= item->cycle); + if (session->last_cycle < item->cycle) { + session->last_cycle = item->cycle; + } - session->last_cycle = item->cycle; item->cycle = session->last_cycle + delta; } diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 88cefb51..6725eb0d 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -101,7 +101,12 @@ static int stream_push_item(nghttp2_stream *stream, nghttp2_session *session) { return 0; } - item->cycle = session->last_cycle; + /* Penalize item by delaying scheduling according to effective + weight. This will delay low priority stream, which is good. + OTOH, this may incur delay for high priority item. Will see. */ + item->cycle = + session->last_cycle + + NGHTTP2_DATA_PAYLOADLEN * NGHTTP2_MAX_WEIGHT / stream->effective_weight; switch (item->frame.hd.type) { case NGHTTP2_DATA: From d3561a63b11be8b1ad598cef52c37ff03525ffd6 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 21:25:31 +0900 Subject: [PATCH 36/50] nghttp: Depend on "leader" anchor if js is linked inside head element --- src/HtmlParser.cc | 24 ++++++++++++++++++++---- src/HtmlParser.h | 3 +++ src/nghttp.cc | 1 + 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/HtmlParser.cc b/src/HtmlParser.cc index 26559cce..51f6a12b 100644 --- a/src/HtmlParser.cc +++ b/src/HtmlParser.cc @@ -30,7 +30,8 @@ namespace nghttp2 { -ParserData::ParserData(const std::string &base_uri) : base_uri(base_uri) {} +ParserData::ParserData(const std::string &base_uri) + : base_uri(base_uri), inside_head(0) {} HtmlParser::HtmlParser(const std::string &base_uri) : base_uri_(base_uri), parser_ctx_(nullptr), parser_data_(base_uri) {} @@ -68,6 +69,9 @@ namespace { void start_element_func(void *user_data, const xmlChar *name, const xmlChar **attrs) { auto parser_data = static_cast(user_data); + if (util::strieq(reinterpret_cast(name), "head")) { + ++parser_data->inside_head; + } if (util::strieq(reinterpret_cast(name), "link")) { auto rel_attr = get_attr(attrs, "rel"); auto href_attr = get_attr(attrs, "href"); @@ -90,8 +94,20 @@ void start_element_func(void *user_data, const xmlChar *name, if (!src_attr) { return; } - // TODO if script is inside in head, this should be REQ_LEADERS. - add_link(parser_data, src_attr, REQ_UNBLOCK_JS); + if (parser_data->inside_head) { + add_link(parser_data, src_attr, REQ_JS); + } else { + add_link(parser_data, src_attr, REQ_UNBLOCK_JS); + } + } +} +} // namespace + +namespace { +void end_element_func(void *user_data, const xmlChar *name) { + auto parser_data = static_cast(user_data); + if (util::strieq(reinterpret_cast(name), "head")) { + --parser_data->inside_head; } } } // namespace @@ -113,7 +129,7 @@ xmlSAXHandler saxHandler = { nullptr, // startDocumentSAXFunc nullptr, // endDocumentSAXFunc &start_element_func, // startElementSAXFunc - nullptr, // endElementSAXFunc + &end_element_func, // endElementSAXFunc nullptr, // referenceSAXFunc nullptr, // charactersSAXFunc nullptr, // ignorableWhitespaceSAXFunc diff --git a/src/HtmlParser.h b/src/HtmlParser.h index 6d438212..694b6c9b 100644 --- a/src/HtmlParser.h +++ b/src/HtmlParser.h @@ -41,6 +41,7 @@ namespace nghttp2 { // Lower value has higher priority enum RequestPriority { REQ_CSS = 1, + REQ_JS, REQ_UNBLOCK_JS, REQ_IMG, REQ_OTHERS, @@ -49,6 +50,8 @@ enum RequestPriority { struct ParserData { std::string base_uri; std::vector> links; + // > 0 if we are inside "head" element. + int inside_head; ParserData(const std::string &base_uri); }; diff --git a/src/nghttp.cc b/src/nghttp.cc index 8f27e9a8..d4b02c3c 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -196,6 +196,7 @@ nghttp2_priority_spec Request::resolve_dep(int32_t pri) { int32_t weight; switch (pri) { case REQ_CSS: + case REQ_JS: anchor_id = anchors[ANCHOR_LEADERS].stream_id; weight = 2; break; From 436595df9818a2aa463f8eaece9bfc92355fdf2f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 21:31:11 +0900 Subject: [PATCH 37/50] nghttp: Remove --dep-idle option In this commit, we made --dep-idle behaviour by default. This is because the previous default behaviour is not reflect current usage of dependency priority and never will be because of fragility of tree due to stream closure. --- src/HtmlParser.cc | 6 +- src/HtmlParser.h | 12 ++-- src/nghttp.cc | 160 +++++++++++++--------------------------------- src/nghttp.h | 20 +----- 4 files changed, 53 insertions(+), 145 deletions(-) diff --git a/src/HtmlParser.cc b/src/HtmlParser.cc index 51f6a12b..bcbfd13f 100644 --- a/src/HtmlParser.cc +++ b/src/HtmlParser.cc @@ -53,13 +53,13 @@ const char *get_attr(const xmlChar **attrs, const char *name) { } // namespace namespace { -void add_link(ParserData *parser_data, const char *uri, RequestPriority pri) { +void add_link(ParserData *parser_data, const char *uri, ResourceType res_type) { auto u = xmlBuildURI( reinterpret_cast(uri), reinterpret_cast(parser_data->base_uri.c_str())); if (u) { parser_data->links.push_back( - std::make_pair(reinterpret_cast(u), pri)); + std::make_pair(reinterpret_cast(u), res_type)); free(u); } } @@ -177,7 +177,7 @@ int HtmlParser::parse_chunk_internal(const char *chunk, size_t size, int fin) { } } -const std::vector> & +const std::vector> & HtmlParser::get_links() const { return parser_data_.links; } diff --git a/src/HtmlParser.h b/src/HtmlParser.h index 694b6c9b..fc2f2e57 100644 --- a/src/HtmlParser.h +++ b/src/HtmlParser.h @@ -38,8 +38,7 @@ namespace nghttp2 { -// Lower value has higher priority -enum RequestPriority { +enum ResourceType { REQ_CSS = 1, REQ_JS, REQ_UNBLOCK_JS, @@ -49,7 +48,7 @@ enum RequestPriority { struct ParserData { std::string base_uri; - std::vector> links; + std::vector> links; // > 0 if we are inside "head" element. int inside_head; ParserData(const std::string &base_uri); @@ -62,7 +61,7 @@ public: HtmlParser(const std::string &base_uri); ~HtmlParser(); int parse_chunk(const char *chunk, size_t size, int fin); - const std::vector> &get_links() const; + const std::vector> &get_links() const; void clear_links(); private: @@ -79,14 +78,13 @@ class HtmlParser { public: HtmlParser(const std::string &base_uri) {} int parse_chunk(const char *chunk, size_t size, int fin) { return 0; } - const std::vector> & - get_links() const { + const std::vector> &get_links() const { return links_; } void clear_links() {} private: - std::vector> links_; + std::vector> links_; }; #endif // !HAVE_LIBXML2 diff --git a/src/nghttp.cc b/src/nghttp.cc index d4b02c3c..41994de9 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -61,9 +61,9 @@ namespace nghttp2 { -// The anchor stream nodes when --dep-idle is enabled. The stream ID -// = 1 is excluded since it is used as first stream in upgrade case. -// We follows the same dependency anchor nodes as Firefox does. +// The anchor stream nodes when --no-dep is not used. The stream ID = +// 1 is excluded since it is used as first stream in upgrade case. We +// follows the same dependency anchor nodes as Firefox does. struct Anchor { int32_t stream_id; // stream ID this anchor depends on @@ -95,7 +95,7 @@ Config::Config() timeout(0.), window_bits(-1), connection_window_bits(-1), verbose(0), null_out(false), remote_name(false), get_assets(false), stat(false), upgrade(false), continuation(false), no_content_length(false), - no_dep(false), dep_idle(false), hexdump(false) { + no_dep(false), hexdump(false) { nghttp2_option_new(&http2_option); nghttp2_option_set_peer_max_concurrent_streams(http2_option, peer_max_concurrent_streams); @@ -127,12 +127,10 @@ std::string strip_fragment(const char *raw_uri) { Request::Request(const std::string &uri, const http_parser_url &u, const nghttp2_data_provider *data_prd, int64_t data_length, - const nghttp2_priority_spec &pri_spec, - std::shared_ptr dep, int pri, int level) - : uri(uri), u(u), dep(std::move(dep)), pri_spec(pri_spec), - data_length(data_length), data_offset(0), response_len(0), - inflater(nullptr), html_parser(nullptr), data_prd(data_prd), - stream_id(-1), status(0), level(level), pri(pri), + const nghttp2_priority_spec &pri_spec, int level) + : uri(uri), u(u), pri_spec(pri_spec), data_length(data_length), + data_offset(0), response_len(0), inflater(nullptr), html_parser(nullptr), + data_prd(data_prd), stream_id(-1), status(0), level(level), expect_final_response(false) { http2::init_hdidx(res_hdidx); http2::init_hdidx(req_hdidx); @@ -171,83 +169,41 @@ std::string Request::make_reqpath() const { return path; } -int32_t Request::find_dep_stream_id(int start) { - for (auto i = start; i >= 0; --i) { - for (auto req : dep->deps[i]) { - return req->stream_id; - } - } - return -1; -} - -nghttp2_priority_spec Request::resolve_dep(int32_t pri) { +namespace { +nghttp2_priority_spec resolve_dep(int res_type) { nghttp2_priority_spec pri_spec; - int exclusive = 0; - int32_t stream_id = -1; - - nghttp2_priority_spec_default_init(&pri_spec); if (config.no_dep) { + nghttp2_priority_spec_default_init(&pri_spec); + return pri_spec; } - if (config.dep_idle) { - int32_t anchor_id; - int32_t weight; - switch (pri) { - case REQ_CSS: - case REQ_JS: - anchor_id = anchors[ANCHOR_LEADERS].stream_id; - weight = 2; - break; - case REQ_UNBLOCK_JS: - anchor_id = anchors[ANCHOR_UNBLOCKED].stream_id; - weight = 2; - break; - case REQ_IMG: - anchor_id = anchors[ANCHOR_FOLLOWERS].stream_id; - weight = 12; - break; - default: - anchor_id = anchors[ANCHOR_FOLLOWERS].stream_id; - weight = 2; - } - - nghttp2_priority_spec_init(&pri_spec, anchor_id, weight, 0); - return pri_spec; + int32_t anchor_id; + int32_t weight; + switch (res_type) { + case REQ_CSS: + case REQ_JS: + anchor_id = anchors[ANCHOR_LEADERS].stream_id; + weight = 2; + break; + case REQ_UNBLOCK_JS: + anchor_id = anchors[ANCHOR_UNBLOCKED].stream_id; + weight = 2; + break; + case REQ_IMG: + anchor_id = anchors[ANCHOR_FOLLOWERS].stream_id; + weight = 12; + break; + default: + anchor_id = anchors[ANCHOR_FOLLOWERS].stream_id; + weight = 2; } - if (pri == 0) { - return pri_spec; - } - - auto start = std::min(pri, (int)dep->deps.size() - 1); - - for (auto i = start; i >= 0; --i) { - if (dep->deps[i][0]->pri < pri) { - stream_id = find_dep_stream_id(i); - - if (i != (int)dep->deps.size() - 1) { - exclusive = 1; - } - - break; - } else if (dep->deps[i][0]->pri == pri) { - stream_id = find_dep_stream_id(i - 1); - - break; - } - } - - if (stream_id == -1) { - return pri_spec; - } - - nghttp2_priority_spec_init(&pri_spec, stream_id, NGHTTP2_DEFAULT_WEIGHT, - exclusive); - + nghttp2_priority_spec_init(&pri_spec, anchor_id, weight, 0); return pri_spec; } +} // namespace bool Request::is_ipv6_literal_addr() const { if (util::has_uri_field(u, UF_HOST)) { @@ -999,7 +955,7 @@ int HttpClient::connection_made() { return -1; } } - if (!config.no_dep && config.dep_idle) { + if (!config.no_dep) { // Create anchor stream nodes nghttp2_priority_spec pri_spec; @@ -1256,9 +1212,7 @@ void HttpClient::update_hostport() { bool HttpClient::add_request(const std::string &uri, const nghttp2_data_provider *data_prd, int64_t data_length, - const nghttp2_priority_spec &pri_spec, - std::shared_ptr dep, int pri, - int level) { + const nghttp2_priority_spec &pri_spec, int level) { http_parser_url u; memset(&u, 0, sizeof(u)); if (http_parser_parse_url(uri.c_str(), uri.size(), 0, &u) != 0) { @@ -1272,8 +1226,8 @@ bool HttpClient::add_request(const std::string &uri, path_cache.insert(uri); } - reqvec.push_back(make_unique(uri, u, data_prd, data_length, pri_spec, - std::move(dep), pri, level)); + reqvec.push_back( + make_unique(uri, u, data_prd, data_length, pri_spec, level)); return true; } @@ -1294,26 +1248,6 @@ void HttpClient::request_done(Request *req) { if (req->stream_id % 2 == 0) { return; } - - auto itr = std::begin(req->dep->deps); - for (; itr != std::end(req->dep->deps); ++itr) { - if ((*itr)[0]->pri == req->pri) { - (*itr).push_back(req); - - break; - } - - if ((*itr)[0]->pri > req->pri) { - auto v = std::vector{req}; - req->dep->deps.insert(itr, std::move(v)); - - break; - } - } - - if (itr == std::end(req->dep->deps)) { - req->dep->deps.push_back(std::vector{req}); - } } #ifdef HAVE_JANSSON @@ -1496,7 +1430,7 @@ void update_html_parser(HttpClient *client, Request *req, const uint8_t *data, for (auto &p : req->html_parser->get_links()) { auto uri = strip_fragment(p.first.c_str()); - auto pri = p.second; + auto res_type = p.second; http_parser_url u; memset(&u, 0, sizeof(u)); @@ -1505,10 +1439,9 @@ void update_html_parser(HttpClient *client, Request *req, const uint8_t *data, util::fieldeq(uri.c_str(), u, req->uri.c_str(), req->u, UF_HOST) && util::porteq(uri.c_str(), u, req->uri.c_str(), req->u)) { // No POST data for assets - auto pri_spec = req->resolve_dep(pri); + auto pri_spec = resolve_dep(res_type); - if (client->add_request(uri, nullptr, 0, pri_spec, req->dep, pri, - req->level + 1)) { + if (client->add_request(uri, nullptr, 0, pri_spec, req->level + 1)) { submit_request(client, config.headers, client->reqvec.back().get()); } @@ -1671,7 +1604,7 @@ int on_begin_headers_callback(nghttp2_session *session, nghttp2_priority_spec_default_init(&pri_spec); - auto req = make_unique("", u, nullptr, 0, pri_spec, nullptr); + auto req = make_unique("", u, nullptr, 0, pri_spec); req->stream_id = stream_id; nghttp2_session_set_stream_user_data(session, stream_id, req.get()); @@ -2069,7 +2002,7 @@ int communicate( nghttp2_priority_spec pri_spec; int32_t dep_stream_id = 0; - if (!config.no_dep && config.dep_idle) { + if (!config.no_dep) { dep_stream_id = anchors[ANCHOR_FOLLOWERS].stream_id; } @@ -2077,9 +2010,8 @@ int communicate( for (auto req : requests) { for (int i = 0; i < config.multiply; ++i) { - auto dep = std::make_shared(); client.add_request(std::get<0>(req), std::get<1>(req), std::get<2>(req), - pri_spec, std::move(dep)); + pri_spec); } } client.update_hostport(); @@ -2436,7 +2368,6 @@ Options: --no-content-length Don't send content-length header field. --no-dep Don't send dependency based priority hint to server. - --dep-idle Use idle streams as anchor nodes to express priority. --hexdump Display the incoming traffic in hexadecimal (Canonical hex+ASCII display). If SSL/TLS is used, decrypted data are used. @@ -2480,7 +2411,6 @@ int main(int argc, char **argv) { {"version", no_argument, &flag, 5}, {"no-content-length", no_argument, &flag, 6}, {"no-dep", no_argument, &flag, 7}, - {"dep-idle", no_argument, &flag, 8}, {"trailer", required_argument, &flag, 9}, {"hexdump", no_argument, &flag, 10}, {nullptr, 0, nullptr, 0}}; @@ -2638,10 +2568,6 @@ int main(int argc, char **argv) { // no-dep option config.no_dep = true; break; - case 8: - // dep-idle option - config.dep_idle = true; - break; case 9: { // trailer option auto header = optarg; diff --git a/src/nghttp.h b/src/nghttp.h index bb2fc449..02421fc5 100644 --- a/src/nghttp.h +++ b/src/nghttp.h @@ -83,7 +83,6 @@ struct Config { bool continuation; bool no_content_length; bool no_dep; - bool dep_idle; bool hexdump; }; @@ -103,18 +102,11 @@ struct RequestTiming { RequestTiming() : state(RequestState::INITIAL) {} }; -struct Request; - -struct Dependency { - std::vector> deps; -}; - struct Request { // For pushed request, |uri| is empty and |u| is zero-cleared. Request(const std::string &uri, const http_parser_url &u, const nghttp2_data_provider *data_prd, int64_t data_length, - const nghttp2_priority_spec &pri_spec, - std::shared_ptr dep, int pri = 0, int level = 0); + const nghttp2_priority_spec &pri_spec, int level = 0); ~Request(); void init_inflater(); @@ -124,10 +116,6 @@ struct Request { std::string make_reqpath() const; - int32_t find_dep_stream_id(int start); - - nghttp2_priority_spec resolve_dep(int32_t pri); - bool is_ipv6_literal_addr() const; bool response_pseudo_header_allowed(int16_t token) const; @@ -145,7 +133,6 @@ struct Request { // URI without fragment std::string uri; http_parser_url u; - std::shared_ptr dep; nghttp2_priority_spec pri_spec; RequestTiming timing; int64_t data_length; @@ -159,8 +146,6 @@ struct Request { int status; // Recursion level: 0: first entity, 1: entity linked from first entity int level; - // RequestPriority value defined in HtmlParser.h - int pri; http2::HeaderIndex res_hdidx; // used for incoming PUSH_PROMISE http2::HeaderIndex req_hdidx; @@ -220,8 +205,7 @@ struct HttpClient { void update_hostport(); bool add_request(const std::string &uri, const nghttp2_data_provider *data_prd, int64_t data_length, - const nghttp2_priority_spec &pri_spec, - std::shared_ptr dep, int pri = 0, int level = 0); + const nghttp2_priority_spec &pri_spec, int level = 0); void record_start_time(); void record_domain_lookup_end_time(); From 3bdf78e8aff6b44b8896b940aeb0db2a30afb987 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 23:06:07 +0900 Subject: [PATCH 38/50] Document nghttp's dependency based priority --- doc/nghttp.h2r | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/doc/nghttp.h2r b/doc/nghttp.h2r index a27f416f..92a8fa73 100644 --- a/doc/nghttp.h2r +++ b/doc/nghttp.h2r @@ -1,3 +1,54 @@ +DEPENDENCY BASED PRIORITY +------------------------- + +nghttp sends priority hints to server by default unless +:option:`--no-dep` is used. nghttp mimics the way Firefox employs to +manages dependency using idle streams. We follows the behaviour of +Firefox Nightly as of April, 2015, and nghttp's behaviour is very +static and could be different from Firefox in detail. But reproducing +the same behaviour of Firefox is not our goal. The goal is provide +the easy way to test out the dependency priority in server +implementation. + +When connection is established, nghttp sends 5 PRIORITY frames to idle +streams 3, 5, 7, 9 and 11 to create "anchor" nodes in dependency +tree:: + + +-----+ + |id=0 | + +-----+ + ^ ^ ^ + w=201 / | \ w=1 + / | \ + / w=101| \ + +-----+ +-----+ +-----+ + |id=3 | |id=5 | |id=7 | + +-----+ +-----+ +-----+ + ^ ^ + w=1 | w=1 | + | | + +-----+ +-----+ + |id=11| |id=9 | + +-----+ +-----+ + +In the above figure, ``id`` means stream ID, and ``w`` means weight. +The stream 0 is non-existence stream, and forms the root of the tree. +The stream 7 and 9 are not used for now. + +The URIs given in the command-line depend on stream 11 with the weight +given in :option:`-p` option, which defaults to 16. + +If :option:`-a` option is used, nghttp parses the resource pointed by +URI given in command-line as html, and extracts resource links from +it. When requesting those resources, nghttp uses dependency according +to its resource type. + +For CSS, and Javascript files inside "head" element, they depend on +stream 3 with the weight 2. The Javascript files outside "head" +element depend on stream 5 with the weight 2. The mages depend on +stream 11 with the weight 12. The other resources (e.g., icon) depend +on stream 11 with the weight 2. + SEE ALSO -------- From b948c5457d0ccc4e0ab1188b86cff6931b3f0aaf Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 23:13:42 +0900 Subject: [PATCH 39/50] Specify program directive --- help2rst.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/help2rst.py b/help2rst.py index c5126ffc..4beda7b0 100755 --- a/help2rst.py +++ b/help2rst.py @@ -61,6 +61,8 @@ def help2man(infile): description.append(line) print ''' +.. program:: {cmdname} + {cmdname}(1) {cmdnameunderline} From cfabce6e707de5ed49b44d649f99b926e37a3cc4 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 23:14:23 +0900 Subject: [PATCH 40/50] Update man pages --- doc/h2load.1 | 2 +- doc/h2load.1.rst | 2 ++ doc/nghttp.1 | 70 +++++++++++++++++++++++++++++++++++++++++------ doc/nghttp.1.rst | 57 +++++++++++++++++++++++++++++++++++--- doc/nghttpd.1 | 6 ++-- doc/nghttpd.1.rst | 2 ++ doc/nghttpx.1 | 2 +- doc/nghttpx.1.rst | 2 ++ 8 files changed, 125 insertions(+), 18 deletions(-) diff --git a/doc/h2load.1 b/doc/h2load.1 index 399cc7b0..7cec73be 100644 --- a/doc/h2load.1 +++ b/doc/h2load.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "H2LOAD" "1" "April 10, 2015" "0.7.11" "nghttp2" +.TH "H2LOAD" "1" "April 17, 2015" "0.7.12-DEV" "nghttp2" .SH NAME h2load \- HTTP/2 benchmarking tool . diff --git a/doc/h2load.1.rst b/doc/h2load.1.rst index 22c2a7d9..ff1115c8 100644 --- a/doc/h2load.1.rst +++ b/doc/h2load.1.rst @@ -1,4 +1,6 @@ +.. program:: h2load + h2load(1) ========= diff --git a/doc/nghttp.1 b/doc/nghttp.1 index 000b3fde..ee031126 100644 --- a/doc/nghttp.1 +++ b/doc/nghttp.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTP" "1" "April 10, 2015" "0.7.11" "nghttp2" +.TH "NGHTTP" "1" "April 17, 2015" "0.7.12-DEV" "nghttp2" .SH NAME nghttp \- HTTP/2 experimental client . @@ -104,8 +104,8 @@ Add a header to the requests. Example: \fI\%\-H\fP\(aq:method: PUT\(aq .B \-\-trailer=
Add a trailer header to the requests.
must not include pseudo header field (header field name starting -with \(aq:\(aq). To send trailer, one must use \fI\-d\fP option to -send request body. Example: \fI\-\-trailer\fP \(aqfoo: bar\(aq. +with \(aq:\(aq). To send trailer, one must use \fI\%\-d\fP option to +send request body. Example: \fI\%\-\-trailer\fP \(aqfoo: bar\(aq. .UNINDENT .INDENT 0.0 .TP @@ -135,7 +135,7 @@ requested twice. This option disables it too. .TP .B \-u, \-\-upgrade Perform HTTP Upgrade for HTTP/2. This option is ignored -if the request URI has https scheme. If \fI\-d\fP is used, the +if the request URI has https scheme. If \fI\%\-d\fP is used, the HTTP upgrade request is performed with OPTIONS method. .UNINDENT .INDENT 0.0 @@ -192,11 +192,6 @@ Don\(aqt send dependency based priority hint to server. .UNINDENT .INDENT 0.0 .TP -.B \-\-dep\-idle -Use idle streams as anchor nodes to express priority. -.UNINDENT -.INDENT 0.0 -.TP .B \-\-hexdump Display the incoming traffic in hexadecimal (Canonical hex+ASCII display). If SSL/TLS is used, decrypted data @@ -215,6 +210,63 @@ Display this help and exit. .sp The argument is an integer and an optional unit (e.g., 10K is 10 * 1024). Units are K, M and G (powers of 1024). +.SH DEPENDENCY BASED PRIORITY +.sp +nghttp sends priority hints to server by default unless +\fI\%\-\-no\-dep\fP is used. nghttp mimics the way Firefox employs to +manages dependency using idle streams. We follows the behaviour of +Firefox Nightly as of April, 2015, and nghttp\(aqs behaviour is very +static and could be different from Firefox in detail. But reproducing +the same behaviour of Firefox is not our goal. The goal is provide +the easy way to test out the dependency priority in server +implementation. +.sp +When connection is established, nghttp sends 5 PRIORITY frames to idle +streams 3, 5, 7, 9 and 11 to create "anchor" nodes in dependency +tree: +.INDENT 0.0 +.INDENT 3.5 +.sp +.nf +.ft C + +\-\-\-\-\-+ + |id=0 | + +\-\-\-\-\-+ + ^ ^ ^ + w=201 / | \e w=1 + / | \e + / w=101| \e + +\-\-\-\-\-+ +\-\-\-\-\-+ +\-\-\-\-\-+ + |id=3 | |id=5 | |id=7 | + +\-\-\-\-\-+ +\-\-\-\-\-+ +\-\-\-\-\-+ + ^ ^ +w=1 | w=1 | + | | + +\-\-\-\-\-+ +\-\-\-\-\-+ + |id=11| |id=9 | + +\-\-\-\-\-+ +\-\-\-\-\-+ +.ft P +.fi +.UNINDENT +.UNINDENT +.sp +In the above figure, \fBid\fP means stream ID, and \fBw\fP means weight. +The stream 0 is non\-existence stream, and forms the root of the tree. +The stream 7 and 9 are not used for now. +.sp +The URIs given in the command\-line depend on stream 11 with the weight +given in \fI\%\-p\fP option, which defaults to 16. +.sp +If \fI\%\-a\fP option is used, nghttp parses the resource pointed by +URI given in command\-line as html, and extracts resource links from +it. When requesting those resources, nghttp uses dependency according +to its resource type. +.sp +For CSS, and Javascript files inside "head" element, they depend on +stream 3 with the weight 2. The Javascript files outside "head" +element depend on stream 5 with the weight 2. The mages depend on +stream 11 with the weight 12. The other resources (e.g., icon) depend +on stream 11 with the weight 2. .SH SEE ALSO .sp \fInghttpd(1)\fP, \fInghttpx(1)\fP, \fIh2load(1)\fP diff --git a/doc/nghttp.1.rst b/doc/nghttp.1.rst index 7000c7ac..6e8d0f3a 100644 --- a/doc/nghttp.1.rst +++ b/doc/nghttp.1.rst @@ -1,4 +1,6 @@ +.. program:: nghttp + nghttp(1) ========= @@ -143,10 +145,6 @@ OPTIONS Don't send dependency based priority hint to server. -.. option:: --dep-idle - - Use idle streams as anchor nodes to express priority. - .. option:: --hexdump Display the incoming traffic in hexadecimal (Canonical @@ -166,6 +164,57 @@ OPTIONS The argument is an integer and an optional unit (e.g., 10K is 10 * 1024). Units are K, M and G (powers of 1024). +DEPENDENCY BASED PRIORITY +------------------------- + +nghttp sends priority hints to server by default unless +:option:`--no-dep` is used. nghttp mimics the way Firefox employs to +manages dependency using idle streams. We follows the behaviour of +Firefox Nightly as of April, 2015, and nghttp's behaviour is very +static and could be different from Firefox in detail. But reproducing +the same behaviour of Firefox is not our goal. The goal is provide +the easy way to test out the dependency priority in server +implementation. + +When connection is established, nghttp sends 5 PRIORITY frames to idle +streams 3, 5, 7, 9 and 11 to create "anchor" nodes in dependency +tree:: + + +-----+ + |id=0 | + +-----+ + ^ ^ ^ + w=201 / | \ w=1 + / | \ + / w=101| \ + +-----+ +-----+ +-----+ + |id=3 | |id=5 | |id=7 | + +-----+ +-----+ +-----+ + ^ ^ + w=1 | w=1 | + | | + +-----+ +-----+ + |id=11| |id=9 | + +-----+ +-----+ + +In the above figure, ``id`` means stream ID, and ``w`` means weight. +The stream 0 is non-existence stream, and forms the root of the tree. +The stream 7 and 9 are not used for now. + +The URIs given in the command-line depend on stream 11 with the weight +given in :option:`-p` option, which defaults to 16. + +If :option:`-a` option is used, nghttp parses the resource pointed by +URI given in command-line as html, and extracts resource links from +it. When requesting those resources, nghttp uses dependency according +to its resource type. + +For CSS, and Javascript files inside "head" element, they depend on +stream 3 with the weight 2. The Javascript files outside "head" +element depend on stream 5 with the weight 2. The mages depend on +stream 11 with the weight 12. The other resources (e.g., icon) depend +on stream 11 with the weight 2. + SEE ALSO -------- diff --git a/doc/nghttpd.1 b/doc/nghttpd.1 index 7684de21..a9cdcd35 100644 --- a/doc/nghttpd.1 +++ b/doc/nghttpd.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPD" "1" "April 10, 2015" "0.7.11" "nghttp2" +.TH "NGHTTPD" "1" "April 17, 2015" "0.7.12-DEV" "nghttp2" .SH NAME nghttpd \- HTTP/2 experimental server . @@ -63,7 +63,7 @@ address determined by getaddrinfo is used. .INDENT 0.0 .TP .B \-D, \-\-daemon -Run in a background. If \fI\-D\fP is used, the current working +Run in a background. If \fI\%\-D\fP is used, the current working directory is changed to \(aq\fI/\fP\(aq. Therefore if this option is used, \fI\%\-d\fP option must be specified. .UNINDENT @@ -109,7 +109,7 @@ Push resources s when is requested. This option can be used repeatedly to specify multiple push configurations. and s are relative to document root. See \fI\%\-\-htdocs\fP option. -Example: \fI\-p\fP/=/foo.png \fI\-p\fP/doc=/bar.css +Example: \fI\%\-p\fP/=/foo.png \fI\%\-p\fP/doc=/bar.css .UNINDENT .INDENT 0.0 .TP diff --git a/doc/nghttpd.1.rst b/doc/nghttpd.1.rst index 537963d1..77cc01d0 100644 --- a/doc/nghttpd.1.rst +++ b/doc/nghttpd.1.rst @@ -1,4 +1,6 @@ +.. program:: nghttpd + nghttpd(1) ========== diff --git a/doc/nghttpx.1 b/doc/nghttpx.1 index e309995f..66e8caf2 100644 --- a/doc/nghttpx.1 +++ b/doc/nghttpx.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPX" "1" "April 10, 2015" "0.7.11" "nghttp2" +.TH "NGHTTPX" "1" "April 17, 2015" "0.7.12-DEV" "nghttp2" .SH NAME nghttpx \- HTTP/2 experimental proxy . diff --git a/doc/nghttpx.1.rst b/doc/nghttpx.1.rst index 4aff82cd..99058a41 100644 --- a/doc/nghttpx.1.rst +++ b/doc/nghttpx.1.rst @@ -1,4 +1,6 @@ +.. program:: nghttpx + nghttpx(1) ========== From 0b41e20d54972c983777eeb291cd27e818bbf3b9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 23:33:06 +0900 Subject: [PATCH 41/50] nghttp: Show stream ID in statistics output --- src/nghttp.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/nghttp.cc b/src/nghttp.cc index 41994de9..f03704f5 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -1876,7 +1876,7 @@ see http://www.w3.org/TR/resource-timing/#processing-model sorted by 'complete' -responseEnd requestStart process code size request path)" << std::endl; +id responseEnd requestStart process code size request path)" << std::endl; const auto &base = client.timing.connect_end_time; for (const auto &req : reqs) { @@ -1888,8 +1888,9 @@ responseEnd requestStart process code size request path)" << std::endl; req->timing.response_end_time - req->timing.request_start_time); auto pushed = req->stream_id % 2 == 0; - std::cout << std::setw(11) << ("+" + util::format_duration(response_end)) - << " " << (pushed ? "*" : " ") << std::setw(11) + std::cout << std::setw(3) << req->stream_id << " " << std::setw(11) + << ("+" + util::format_duration(response_end)) << " " + << (pushed ? "*" : " ") << std::setw(11) << ("+" + util::format_duration(request_start)) << " " << std::setw(8) << util::format_duration(total) << " " << std::setw(4) << req->status << " " << std::setw(4) From c4e994c97d21ab7bd5032b6887a4ada73b641a90 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 23:34:28 +0900 Subject: [PATCH 42/50] nghttp: Add --no-push option to disable server push --- src/nghttp.cc | 17 +++++++++++++++-- src/nghttp.h | 1 + 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/nghttp.cc b/src/nghttp.cc index f03704f5..56e09ec9 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -95,7 +95,7 @@ Config::Config() timeout(0.), window_bits(-1), connection_window_bits(-1), verbose(0), null_out(false), remote_name(false), get_assets(false), stat(false), upgrade(false), continuation(false), no_content_length(false), - no_dep(false), hexdump(false) { + no_dep(false), hexdump(false), no_push(false) { nghttp2_option_new(&http2_option); nghttp2_option_set_peer_max_concurrent_streams(http2_option, peer_max_concurrent_streams); @@ -737,6 +737,13 @@ size_t populate_settings(nghttp2_settings_entry *iv) { iv[niv].value = config.header_table_size; ++niv; } + + if (config.no_push) { + iv[niv].settings_id = NGHTTP2_SETTINGS_ENABLE_PUSH; + iv[niv].value = 0; + ++niv; + } + return niv; } } // namespace @@ -745,7 +752,7 @@ int HttpClient::on_upgrade_connect() { ssize_t rv; record_connect_end_time(); assert(!reqvec.empty()); - std::array iv; + std::array iv; size_t niv = populate_settings(iv.data()); assert(settings_payload.size() >= 8 * niv); rv = nghttp2_pack_settings_payload(settings_payload.data(), @@ -2372,6 +2379,7 @@ Options: --hexdump Display the incoming traffic in hexadecimal (Canonical hex+ASCII display). If SSL/TLS is used, decrypted data are used. + --no-push Disable server push. --version Display version information and exit. -h, --help Display this help and exit. @@ -2414,6 +2422,7 @@ int main(int argc, char **argv) { {"no-dep", no_argument, &flag, 7}, {"trailer", required_argument, &flag, 9}, {"hexdump", no_argument, &flag, 10}, + {"no-push", no_argument, &flag, 11}, {nullptr, 0, nullptr, 0}}; int option_index = 0; int c = getopt_long(argc, argv, "M:Oab:c:d:gm:np:r:hH:vst:uw:W:", @@ -2597,6 +2606,10 @@ int main(int argc, char **argv) { // hexdump option config.hexdump = true; break; + case 11: + // no-push option + config.no_push = true; + break; } break; default: diff --git a/src/nghttp.h b/src/nghttp.h index 02421fc5..67e94722 100644 --- a/src/nghttp.h +++ b/src/nghttp.h @@ -84,6 +84,7 @@ struct Config { bool no_content_length; bool no_dep; bool hexdump; + bool no_push; }; enum class RequestState { INITIAL, ON_REQUEST, ON_RESPONSE, ON_COMPLETE }; From a3fa2574733b0104f48e87c1d6de42a2ee188b7f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 23:46:19 +0900 Subject: [PATCH 43/50] Fix compile error with Android NDK r10d + --enable-werror --- lib/nghttp2_hd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index e023229e..8008e259 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -1006,6 +1006,9 @@ static uint8_t pack_first_byte(int indexing_mode) { default: assert(0); } + /* This is required to compile with android NDK r10d + + --enable-werror */ + return 0; } static int emit_indname_block(nghttp2_bufs *bufs, size_t idx, From 85671a69bf6b47d134fff3653f6f0638dbbff718 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 18 Apr 2015 16:47:46 +0900 Subject: [PATCH 44/50] Update doc --- lib/nghttp2_hd.h | 1 + lib/nghttp2_session.c | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/lib/nghttp2_hd.h b/lib/nghttp2_hd.h index 2d53d57a..4b76d1ce 100644 --- a/lib/nghttp2_hd.h +++ b/lib/nghttp2_hd.h @@ -51,6 +51,7 @@ /* Exported for unit test */ #define NGHTTP2_STATIC_TABLE_LENGTH 61 +/* Generated by genlibtokenlookup.py */ typedef enum { NGHTTP2_TOKEN__AUTHORITY = 0, NGHTTP2_TOKEN__METHOD = 1, diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 3d5a25a2..108a30c1 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -2244,12 +2244,17 @@ static int session_close_stream_on_goaway(nghttp2_session *session, static void session_outbound_item_schedule(nghttp2_session *session, nghttp2_outbound_item *item, int32_t weight) { + /* Schedule next write. Offset proportional to the write size. + Stream with heavier weight is scheduled earlier. */ size_t delta = item->frame.hd.length * NGHTTP2_MAX_WEIGHT / weight; if (session->last_cycle < item->cycle) { session->last_cycle = item->cycle; } + /* We pretend to ignore overflow given that the value range of + item->cycle, which is uint64_t. nghttp2 won't explode even when + overflow occurs, there might be some disturbance of priority. */ item->cycle = session->last_cycle + delta; } From 102ea7c0bb94bfb24a34093ab66730fd2fd9ea44 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 19 Apr 2015 16:26:47 +0900 Subject: [PATCH 45/50] nghttpd: Cache fd Implement fd caching for static files. The response body for such as 404 was dynamically generated previously, but now it is written in temporally file and its fd is cached. Currently, cache is reference counted and expired when count becomes 0. This makes caching is not effective other than "busy" period, but we don't need this feature if we are not busy. --- src/HttpServer.cc | 288 ++++++++++++++++++++++++++++++---------------- src/HttpServer.h | 24 +++- 2 files changed, 209 insertions(+), 103 deletions(-) diff --git a/src/HttpServer.cc b/src/HttpServer.cc index e8377a6e..19cdbed7 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -57,11 +57,6 @@ namespace nghttp2 { namespace { -const std::string STATUS_200 = "200"; -const std::string STATUS_301 = "301"; -const std::string STATUS_304 = "304"; -const std::string STATUS_400 = "400"; -const std::string STATUS_404 = "404"; const std::string DEFAULT_HTML = "index.html"; const std::string NGHTTPD_SERVER = "nghttpd nghttp2/" NGHTTP2_VERSION; } // namespace @@ -171,9 +166,10 @@ void fill_callback(nghttp2_session_callbacks *callbacks, const Config *config); class Sessions { public: - Sessions(struct ev_loop *loop, const Config *config, SSL_CTX *ssl_ctx) - : loop_(loop), config_(config), ssl_ctx_(ssl_ctx), callbacks_(nullptr), - next_session_id_(1), tstamp_cached_(ev_now(loop)), + Sessions(HttpServer *sv, struct ev_loop *loop, const Config *config, + SSL_CTX *ssl_ctx) + : sv_(sv), loop_(loop), config_(config), ssl_ctx_(ssl_ctx), + callbacks_(nullptr), next_session_id_(1), tstamp_cached_(ev_now(loop)), cached_date_(util::http_date(tstamp_cached_)) { nghttp2_session_callbacks_new(&callbacks_); @@ -244,9 +240,37 @@ public: } return cached_date_; } + FileEntry *get_cached_fd(const std::string &path) { + auto i = fd_cache_.find(path); + if (i == std::end(fd_cache_)) { + return nullptr; + } + auto &ent = (*i).second; + ++ent.usecount; + return &ent; + } + FileEntry *cache_fd(const std::string &path, const FileEntry &ent) { + auto rv = fd_cache_.emplace(path, ent); + return &(*rv.first).second; + } + void release_fd(const std::string &path) { + auto i = fd_cache_.find(path); + if (i == std::end(fd_cache_)) { + return; + } + auto &ent = (*i).second; + if (--ent.usecount == 0) { + close(ent.fd); + fd_cache_.erase(i); + } + } + const HttpServer *get_server() const { return sv_; } private: std::set handlers_; + // cache for file descriptors to read file. + std::map fd_cache_; + HttpServer *sv_; struct ev_loop *loop_; const Config *config_; SSL_CTX *ssl_ctx_; @@ -257,7 +281,8 @@ private: }; Stream::Stream(Http2Handler *handler, int32_t stream_id) - : handler(handler), body_left(0), stream_id(stream_id), file(-1) { + : handler(handler), file_ent(nullptr), body_length(0), body_offset(0), + stream_id(stream_id) { auto config = handler->get_config(); ev_timer_init(&rtimer, stream_timeout_cb, 0., config->stream_read_timeout); ev_timer_init(&wtimer, stream_timeout_cb, 0., config->stream_write_timeout); @@ -270,8 +295,9 @@ Stream::Stream(Http2Handler *handler, int32_t stream_id) } Stream::~Stream() { - if (file != -1) { - close(file); + if (file_ent != nullptr) { + auto sessions = handler->get_sessions(); + sessions->release_fd(file_ent->path); } auto loop = handler->get_loop(); @@ -835,12 +861,12 @@ ssize_t file_read_callback(nghttp2_session *session, int32_t stream_id, auto hd = static_cast(user_data); auto stream = hd->get_stream(stream_id); - size_t nread = std::min(stream->body_left, static_cast(length)); + auto nread = std::min(stream->body_length - stream->body_offset, + static_cast(length)); *data_flags |= NGHTTP2_DATA_FLAG_NO_COPY; - stream->body_left -= nread; - if (nread == 0 || stream->body_left <= 0) { + if (nread == 0 || stream->body_length == stream->body_offset + nread) { *data_flags |= NGHTTP2_DATA_FLAG_EOF; auto config = hd->get_config(); @@ -872,59 +898,27 @@ ssize_t file_read_callback(nghttp2_session *session, int32_t stream_id, } namespace { -void prepare_status_response(Stream *stream, Http2Handler *hd, - const std::string &status) { - int pipefd[2]; - if (status == STATUS_304 || pipe(pipefd) == -1) { - hd->submit_response(status, stream->stream_id, 0); - return; - } - std::string body; - body.reserve(256); - body = ""; - body += status; - body += "

"; - body += status; - body += "

"; - body += NGHTTPD_SERVER; - body += " at port "; - body += util::utos(hd->get_config()->port); - body += "
"; - body += ""; +void prepare_status_response(Stream *stream, Http2Handler *hd, int status) { + auto sessions = hd->get_sessions(); + auto status_page = sessions->get_server()->get_status_page(status); + auto file_ent = &status_page->file_ent; + + // we don't set stream->file_ent since we don't want to expire it. + stream->body_length = file_ent->length; + nghttp2_data_provider data_prd; + data_prd.source.fd = file_ent->fd; + data_prd.read_callback = file_read_callback; Headers headers; - if (hd->get_config()->error_gzip) { - gzFile write_fd = gzdopen(pipefd[1], "w"); - gzwrite(write_fd, body.c_str(), body.size()); - gzclose(write_fd); - headers.emplace_back("content-encoding", "gzip"); - } else { - ssize_t rv; - - while ((rv = write(pipefd[1], body.c_str(), body.size())) == -1 && - errno == EINTR) - ; - - if (rv != static_cast(body.size())) { - std::cerr << "Could not write all response body: " << rv << std::endl; - } - } - close(pipefd[1]); - - stream->file = pipefd[0]; - stream->body_left = body.size(); - nghttp2_data_provider data_prd; - data_prd.source.fd = pipefd[0]; - data_prd.read_callback = file_read_callback; headers.emplace_back("content-type", "text/html; charset=UTF-8"); - hd->submit_response(status, stream->stream_id, headers, &data_prd); + hd->submit_response(status_page->status, stream->stream_id, headers, + &data_prd); } } // namespace namespace { void prepare_redirect_response(Stream *stream, Http2Handler *hd, - const std::string &path, - const std::string &status) { + const std::string &path, int status) { auto scheme = http2::get_header(stream->hdidx, http2::HD__SCHEME, stream->headers); auto authority = @@ -941,7 +935,10 @@ void prepare_redirect_response(Stream *stream, Http2Handler *hd, auto headers = Headers{{"location", redirect_url}}; - hd->submit_response(status, stream->stream_id, headers, nullptr); + auto sessions = hd->get_sessions(); + auto status_page = sessions->get_server()->get_status_page(status); + + hd->submit_response(status_page->status, stream->stream_id, headers, nullptr); } } // namespace @@ -975,7 +972,7 @@ void prepare_response(Stream *stream, Http2Handler *hd, url = util::percentDecode(url.begin(), url.end()); if (!util::check_path(url)) { - prepare_status_response(stream, hd, STATUS_404); + prepare_status_response(stream, hd, 404); return; } auto push_itr = hd->get_config()->push.find(url); @@ -992,51 +989,66 @@ void prepare_response(Stream *stream, Http2Handler *hd, if (path[path.size() - 1] == '/') { path += DEFAULT_HTML; } - int file = open(path.c_str(), O_RDONLY | O_BINARY); - if (file == -1) { - prepare_status_response(stream, hd, STATUS_404); - return; - } + auto sessions = hd->get_sessions(); + auto file_ent = sessions->get_cached_fd(path); - struct stat buf; + if (file_ent == nullptr) { + int file = open(path.c_str(), O_RDONLY | O_BINARY); + if (file == -1) { + prepare_status_response(stream, hd, 404); - if (fstat(file, &buf) == -1) { - close(file); - prepare_status_response(stream, hd, STATUS_404); - - return; - } - - if (buf.st_mode & S_IFDIR) { - close(file); - - if (query_pos == std::string::npos) { - reqpath += "/"; - } else { - reqpath.insert(query_pos, "/"); + return; } - prepare_redirect_response(stream, hd, reqpath, STATUS_301); + struct stat buf; + + if (fstat(file, &buf) == -1) { + close(file); + prepare_status_response(stream, hd, 404); + + return; + } + + if (buf.st_mode & S_IFDIR) { + close(file); + + if (query_pos == std::string::npos) { + reqpath += "/"; + } else { + reqpath.insert(query_pos, "/"); + } + + prepare_redirect_response(stream, hd, reqpath, 301); + + return; + } + + if (last_mod_found && buf.st_mtime <= last_mod) { + close(file); + prepare_status_response(stream, hd, 304); + + return; + } + + file_ent = sessions->cache_fd( + path, FileEntry(path, buf.st_size, buf.st_mtime, file)); + } else if (last_mod_found && file_ent->mtime <= last_mod) { + sessions->release_fd(file_ent->path); + prepare_status_response(stream, hd, 304); return; } - stream->file = file; - stream->body_left = buf.st_size; + stream->file_ent = file_ent; + stream->body_length = file_ent->length; nghttp2_data_provider data_prd; - data_prd.source.fd = file; + data_prd.source.fd = file_ent->fd; data_prd.read_callback = file_read_callback; - if (last_mod_found && buf.st_mtime <= last_mod) { - prepare_status_response(stream, hd, STATUS_304); - - return; - } - - hd->submit_file_response(STATUS_200, stream, buf.st_mtime, buf.st_size, + hd->submit_file_response("200", stream, file_ent->mtime, file_ent->length, &data_prd); } } // namespace @@ -1220,6 +1232,7 @@ int send_data_callback(nghttp2_session *session, nghttp2_frame *frame, auto hd = static_cast(user_data); auto wb = hd->get_wb(); auto padlen = frame->data.padlen; + auto stream = hd->get_stream(frame->hd.stream_id); if (wb->wleft() < 9 + length + padlen) { return NGHTTP2_ERR_WOULDBLOCK; @@ -1237,17 +1250,18 @@ int send_data_callback(nghttp2_session *session, nghttp2_frame *frame, while (length) { ssize_t nread; - while ((nread = read(fd, p, length)) == -1 && errno == EINTR) + while ((nread = pread(fd, p, length, stream->body_offset)) == -1 && + errno == EINTR) ; if (nread == -1) { - auto stream = hd->get_stream(frame->hd.stream_id); remove_stream_read_timeout(stream); remove_stream_write_timeout(stream); return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } + stream->body_offset += nread; length -= nread; p += nread; } @@ -1380,7 +1394,7 @@ void run_worker(Worker *worker) { class AcceptHandler { public: - AcceptHandler(Sessions *sessions, const Config *config) + AcceptHandler(HttpServer *sv, Sessions *sessions, const Config *config) : sessions_(sessions), config_(config), next_worker_(0) { if (config_->num_worker == 1) { return; @@ -1392,7 +1406,7 @@ public: auto worker = make_unique(); auto loop = ev_loop_new(0); worker->sessions = - make_unique(loop, config_, sessions_->get_ssl_ctx()); + make_unique(sv, loop, config_, sessions_->get_ssl_ctx()); ev_async_init(&worker->w, worker_acceptcb); worker->w.data = worker.get(); ev_async_start(loop, &worker->w); @@ -1476,7 +1490,61 @@ void acceptcb(struct ev_loop *loop, ev_io *w, int revents) { } } // namespace -HttpServer::HttpServer(const Config *config) : config_(config) {} +namespace { +FileEntry make_status_body(int status, uint16_t port) { + std::string body; + body = ""; + body += http2::get_status_string(status); + body += "

"; + body += http2::get_status_string(status); + body += "

"; + body += NGHTTPD_SERVER; + body += " at port "; + body += util::utos(port); + body += "
"; + body += ""; + + char tempfn[] = "/tmp/nghttpd.temp.XXXXXX"; + int fd = mkstemp(tempfn); + if (fd == -1) { + auto error = errno; + std::cerr << "Could not open status response body file: errno=" << error; + assert(0); + } + unlink(tempfn); + ssize_t nwrite; + while ((nwrite = write(fd, body.c_str(), body.size())) == -1 && + errno == EINTR) + ; + if (nwrite == -1) { + auto error = errno; + std::cerr << "Could not write status response body into file: errno=" + << error; + assert(0); + } + + return FileEntry(util::utos(status), nwrite, 0, fd); +} +} // namespace + +// index into HttpServer::status_pages_ +enum { + IDX_200, + IDX_301, + IDX_304, + IDX_400, + IDX_404, +}; + +HttpServer::HttpServer(const Config *config) : config_(config) { + status_pages_ = std::vector{ + {"200", make_status_body(200, config_->port)}, + {"301", make_status_body(301, config_->port)}, + {"304", make_status_body(304, config_->port)}, + {"400", make_status_body(400, config_->port)}, + {"404", make_status_body(404, config_->port)}, + }; +} namespace { int next_proto_cb(SSL *s, const unsigned char **data, unsigned int *len, @@ -1497,14 +1565,14 @@ int verify_callback(int preverify_ok, X509_STORE_CTX *ctx) { } // namespace namespace { -int start_listen(struct ev_loop *loop, Sessions *sessions, +int start_listen(HttpServer *sv, struct ev_loop *loop, Sessions *sessions, const Config *config) { addrinfo hints; int r; bool ok = false; const char *addr = nullptr; - auto acceptor = std::make_shared(sessions, config); + auto acceptor = std::make_shared(sv, sessions, config); auto service = util::utos(config->port); memset(&hints, 0, sizeof(addrinfo)); @@ -1699,8 +1767,8 @@ int HttpServer::run() { auto loop = EV_DEFAULT; - Sessions sessions(loop, config_, ssl_ctx); - if (start_listen(loop, &sessions, config_) != 0) { + Sessions sessions(this, loop, config_, ssl_ctx); + if (start_listen(this, loop, &sessions, config_) != 0) { std::cerr << "Could not listen" << std::endl; return -1; } @@ -1711,4 +1779,22 @@ int HttpServer::run() { const Config *HttpServer::get_config() const { return config_; } +const StatusPage *HttpServer::get_status_page(int status) const { + switch (status) { + case 200: + return &status_pages_[IDX_200]; + case 301: + return &status_pages_[IDX_301]; + case 304: + return &status_pages_[IDX_304]; + case 400: + return &status_pages_[IDX_400]; + case 404: + return &status_pages_[IDX_404]; + default: + assert(0); + } + return nullptr; +} + } // namespace nghttp2 diff --git a/src/HttpServer.h b/src/HttpServer.h index 62d50a5d..d6272f4a 100644 --- a/src/HttpServer.h +++ b/src/HttpServer.h @@ -78,14 +78,27 @@ struct Config { class Http2Handler; +struct FileEntry { + FileEntry(std::string path, int64_t length, int64_t mtime, int fd) + : path(std::move(path)), length(length), mtime(mtime), dlprev(nullptr), + dlnext(nullptr), fd(fd), usecount(1) {} + std::string path; + int64_t length; + int64_t mtime; + FileEntry *dlprev, *dlnext; + int fd; + int usecount; +}; + struct Stream { Headers headers; Http2Handler *handler; + FileEntry *file_ent; ev_timer rtimer; ev_timer wtimer; - int64_t body_left; + int64_t body_length; + int64_t body_offset; int32_t stream_id; - int file; http2::HeaderIndex hdidx; Stream(Http2Handler *handler, int32_t stream_id); ~Stream(); @@ -160,14 +173,21 @@ private: int fd_; }; +struct StatusPage { + std::string status; + FileEntry file_ent; +}; + class HttpServer { public: HttpServer(const Config *config); int listen(); int run(); const Config *get_config() const; + const StatusPage *get_status_page(int status) const; private: + std::vector status_pages_; const Config *config_; }; From 80f0e99f00e5c956a4692c21f7e5139e0cac6b72 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 19 Apr 2015 18:07:57 +0900 Subject: [PATCH 46/50] Bump up version number to 0.7.12, LT revision to 13:1:8 --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index ca2eddb7..a3daedf9 100644 --- a/configure.ac +++ b/configure.ac @@ -25,13 +25,13 @@ dnl Do not change user variables! dnl http://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html AC_PREREQ(2.61) -AC_INIT([nghttp2], [0.7.12-DEV], [t-tujikawa@users.sourceforge.net]) +AC_INIT([nghttp2], [0.7.12], [t-tujikawa@users.sourceforge.net]) LT_PREREQ([2.2.6]) LT_INIT() dnl See versioning rule: dnl http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html AC_SUBST(LT_CURRENT, 13) -AC_SUBST(LT_REVISION, 0) +AC_SUBST(LT_REVISION, 1) AC_SUBST(LT_AGE, 8) major=`echo $PACKAGE_VERSION |cut -d. -f1 | sed -e "s/[^0-9]//g"` From a9b54a1bfaff68f1fdce2d3f36622c8e3d2b5ee1 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 19 Apr 2015 18:10:54 +0900 Subject: [PATCH 47/50] Update man pages --- doc/h2load.1 | 2 +- doc/nghttp.1 | 7 ++++++- doc/nghttp.1.rst | 4 ++++ doc/nghttpd.1 | 2 +- doc/nghttpx.1 | 2 +- 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/doc/h2load.1 b/doc/h2load.1 index 7cec73be..740c3ff1 100644 --- a/doc/h2load.1 +++ b/doc/h2load.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "H2LOAD" "1" "April 17, 2015" "0.7.12-DEV" "nghttp2" +.TH "H2LOAD" "1" "April 19, 2015" "0.7.12" "nghttp2" .SH NAME h2load \- HTTP/2 benchmarking tool . diff --git a/doc/nghttp.1 b/doc/nghttp.1 index ee031126..ad6d288a 100644 --- a/doc/nghttp.1 +++ b/doc/nghttp.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTP" "1" "April 17, 2015" "0.7.12-DEV" "nghttp2" +.TH "NGHTTP" "1" "April 19, 2015" "0.7.12" "nghttp2" .SH NAME nghttp \- HTTP/2 experimental client . @@ -199,6 +199,11 @@ are used. .UNINDENT .INDENT 0.0 .TP +.B \-\-no\-push +Disable server push. +.UNINDENT +.INDENT 0.0 +.TP .B \-\-version Display version information and exit. .UNINDENT diff --git a/doc/nghttp.1.rst b/doc/nghttp.1.rst index 6e8d0f3a..ff9fc95f 100644 --- a/doc/nghttp.1.rst +++ b/doc/nghttp.1.rst @@ -151,6 +151,10 @@ OPTIONS hex+ASCII display). If SSL/TLS is used, decrypted data are used. +.. option:: --no-push + + Disable server push. + .. option:: --version Display version information and exit. diff --git a/doc/nghttpd.1 b/doc/nghttpd.1 index a9cdcd35..ba26ef6c 100644 --- a/doc/nghttpd.1 +++ b/doc/nghttpd.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPD" "1" "April 17, 2015" "0.7.12-DEV" "nghttp2" +.TH "NGHTTPD" "1" "April 19, 2015" "0.7.12" "nghttp2" .SH NAME nghttpd \- HTTP/2 experimental server . diff --git a/doc/nghttpx.1 b/doc/nghttpx.1 index 66e8caf2..c28d83a3 100644 --- a/doc/nghttpx.1 +++ b/doc/nghttpx.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPX" "1" "April 17, 2015" "0.7.12-DEV" "nghttp2" +.TH "NGHTTPX" "1" "April 19, 2015" "0.7.12" "nghttp2" .SH NAME nghttpx \- HTTP/2 experimental proxy . From 28bde2cef02432ffc7eedf07a982caba01bce9f1 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 19 Apr 2015 18:11:11 +0900 Subject: [PATCH 48/50] Update bash_completion --- doc/bash_completion/nghttp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/bash_completion/nghttp b/doc/bash_completion/nghttp index 53a0cc8c..6bb7a3e4 100644 --- a/doc/bash_completion/nghttp +++ b/doc/bash_completion/nghttp @@ -8,7 +8,7 @@ _nghttp() _get_comp_words_by_ref cur prev case $cur in -*) - COMPREPLY=( $( compgen -W '--verbose --no-dep --get-assets --har --header-table-size --multiply --padding --hexdump --dep-idle --continuation --connection-window-bits --peer-max-concurrent-streams --timeout --data --no-content-length --version --color --cert --upgrade --remote-name --trailer --weight --help --key --null-out --window-bits --stat --header ' -- "$cur" ) ) + COMPREPLY=( $( compgen -W '--no-push --verbose --no-dep --get-assets --har --header-table-size --multiply --padding --hexdump --continuation --connection-window-bits --peer-max-concurrent-streams --timeout --data --no-content-length --version --color --cert --upgrade --remote-name --trailer --weight --help --key --null-out --window-bits --stat --header ' -- "$cur" ) ) ;; *) _filedir From 91ad7e150eb97bed433021feb98ec20b3c90fccf Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 19 Apr 2015 18:21:26 +0900 Subject: [PATCH 49/50] Never indexing still can use header field name in dynamic table --- lib/nghttp2_hd.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index 8008e259..b26fbbc3 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -1208,24 +1208,20 @@ static search_result search_hd_table(nghttp2_hd_context *context, } } - /* Search dynamic table first, so that we can find recently used - entry first */ - if (indexing_mode != NGHTTP2_HD_NEVER_INDEXING) { - for (i = 0; i < context->hd_table.len; ++i) { - nghttp2_hd_entry *ent = hd_ringbuf_get(&context->hd_table, i); - if (ent->token != token || (token == -1 && !name_eq(&ent->nv, nv))) { - continue; - } + for (i = 0; i < context->hd_table.len; ++i) { + nghttp2_hd_entry *ent = hd_ringbuf_get(&context->hd_table, i); + if (ent->token != token || (token == -1 && !name_eq(&ent->nv, nv))) { + continue; + } - if (res.index == -1) { - res.index = (ssize_t)(i + NGHTTP2_STATIC_TABLE_LENGTH); - } + if (res.index == -1) { + res.index = (ssize_t)(i + NGHTTP2_STATIC_TABLE_LENGTH); + } - if (value_eq(&ent->nv, nv)) { - res.index = (ssize_t)(i + NGHTTP2_STATIC_TABLE_LENGTH); - res.name_value_match = 1; - return res; - } + if (indexing_mode != NGHTTP2_HD_NEVER_INDEXING && value_eq(&ent->nv, nv)) { + res.index = (ssize_t)(i + NGHTTP2_STATIC_TABLE_LENGTH); + res.name_value_match = 1; + return res; } } From 787d40129ba6618ef45213258614abc456204bca Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 19 Apr 2015 18:32:58 +0900 Subject: [PATCH 50/50] Bump up version number to 0.7.13-DEV --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index a3daedf9..8cb65717 100644 --- a/configure.ac +++ b/configure.ac @@ -25,7 +25,7 @@ dnl Do not change user variables! dnl http://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html AC_PREREQ(2.61) -AC_INIT([nghttp2], [0.7.12], [t-tujikawa@users.sourceforge.net]) +AC_INIT([nghttp2], [0.7.13-DEV], [t-tujikawa@users.sourceforge.net]) LT_PREREQ([2.2.6]) LT_INIT() dnl See versioning rule: