nghttpx: Fixups for HTTP/1 backend TLS support

src/shrpx_http_downstream_connection.cc

@@ -920,12 +920,12 @@ int HttpDownstreamConnection::write_tls() {
   auto upstream = downstream_->get_upstream();
   auto input = downstream_->get_request_buf();
 
-  std::array<struct iovec, 1> iov;
+  struct iovec iov;
 
   while (input->rleft() > 0) {
-    auto iovcnt = input->riovec(iov.data(), iov.size());
+    auto iovcnt = input->riovec(&iov, 1);
     assert(iovcnt == 1);
-    auto nwrite = conn_.write_tls(iov[0].iov_base, iov[0].iov_len);
+    auto nwrite = conn_.write_tls(iov.iov_base, iov.iov_len);
 
     if (nwrite == 0) {
       return 0;
@@ -976,8 +976,8 @@ int HttpDownstreamConnection::process_input(const uint8_t *data,
   auto htperr = HTTP_PARSER_ERRNO(&response_htp_);
 
   if (htperr != HPE_OK) {
-    // Handling early return (in other words, response was hijacked
+    // Handling early return (in other words, response was hijacked by
-    // by mruby scripting).
+    // mruby scripting).
     if (downstream_->get_response_state() == Downstream::MSG_COMPLETE) {
       return SHRPX_ERR_DCONN_CANCELED;
     }

src/shrpx_ssl.cc

@@ -646,10 +646,12 @@ int select_h1_next_proto_cb(SSL *ssl, unsigned char **out,
                             unsigned int inlen, void *arg) {
   auto end = in + inlen;
   for (; in < end;) {
-    if (util::streq_l(NGHTTP2_H1_1_ALPN, in, end - in)) {
+    if (util::streq_l(NGHTTP2_H1_1_ALPN, in, in[0] + 1)) {
+      *out = const_cast<unsigned char *>(in) + 1;
+      *outlen = in[0];
       return SSL_TLSEXT_ERR_OK;
     }
-    in += in[0];
+    in += in[0] + 1;
   }
 
   return SSL_TLSEXT_ERR_NOACK;
@@ -739,7 +741,9 @@ SSL_CTX *create_ssl_client_context(
     }
   }
 
-  if (get_config()->conn.downstream.proto == PROTO_HTTP2) {
+  auto &downstreamconf = get_config()->conn.downstream;
+
+  if (downstreamconf.proto == PROTO_HTTP2) {
     // NPN selection callback
     SSL_CTX_set_next_proto_select_cb(ssl_ctx, select_h2_next_proto_cb, nullptr);
 

src/shrpx_worker.cc

@@ -323,8 +323,8 @@ void Worker::cache_cl_tls_session(const DownstreamAddr *addr,
     auto &v = (*it).second;
     assert(!v.empty());
     auto sess = v.front();
-    SSL_SESSION_free(sess);
     v.pop_front();
+    SSL_SESSION_free(sess);
     if (v.empty()) {
       cl_tls_session_cache_.erase(it);
     }

src/shrpx_worker.h

@@ -145,7 +145,14 @@ public:
   mruby::MRubyContext *get_mruby_context() const;
 #endif // HAVE_MRUBY
 
+  // Caches |session| which is associated to downstream address
+  // |addr|.  The caller is responsible to increment the reference
+  // count of |session|, since this function does not do so.
   void cache_cl_tls_session(const DownstreamAddr *addr, SSL_SESSION *session);
+  // Returns cached session associated |addr|.  If non-nullptr value
+  // is returned, its cache entry was successfully removed from cache.
+  // If no cache entry is found associated to |addr|, nullptr will be
+  // returned.
   SSL_SESSION *reuse_cl_tls_session(const DownstreamAddr *addr);
 
 private:
@@ -161,9 +168,20 @@ private:
   DownstreamConnectionPool dconn_pool_;
   WorkerStat worker_stat_;
   std::vector<DownstreamGroup> dgrps_;
+
+  // Cache for SSL_SESSION for downstream connections.  SSL_SESSION is
+  // associated to downstream address.  One address has multiple
+  // SSL_SESSION objects.  New SSL_SESSION is appended to the deque.
+  // When doing eviction due to storage limitation, the SSL_SESSION
+  // which sits at the front of deque is removed.
   std::unordered_map<const DownstreamAddr *, std::deque<SSL_SESSION *>>
       cl_tls_session_cache_;
+  // This is the order of address added to cl_tls_session_cache_ in
+  // order to evict oldest entry first.  The invariant is the sum of
+  // SSL_SESSION in cl_tls_session_cache_ ==
+  // cl_tls_session_order_.size().
   std::deque<const DownstreamAddr *> cl_tls_session_order_;
+
   std::unique_ptr<MemcachedDispatcher> session_cache_memcached_dispatcher_;
 #ifdef HAVE_MRUBY
   std::unique_ptr<mruby::MRubyContext> mruby_ctx_;