nghttpd: Rewrite using bufferevent (again) for simplicity

src/HttpServer.cc

@@ -43,7 +43,7 @@
 
 #include <event.h>
 #include <event2/listener.h>
-#include <event2/bufferevent.h>
+#include <event2/bufferevent_ssl.h>
 
 #ifdef  __cplusplus
 extern "C" {
@@ -336,25 +336,14 @@ Http2Handler::Http2Handler(Sessions *sessions,
     session_(nullptr),
     sessions_(sessions),
     ssl_(ssl),
-    rev_(nullptr),
+    bev_(nullptr),
-    wev_(nullptr),
     settings_timerev_(nullptr),
-    pending_data_(nullptr),
-    pending_datalen_(0),
     fd_(fd)
-{
+{}
-  nghttp2_buf_wrap_init(&sendbuf_, sendbufarray_, sizeof(sendbufarray_));
-}
 
 Http2Handler::~Http2Handler()
 {
   on_session_closed(this, session_id_);
-  if(rev_) {
-    event_free(rev_);
-  }
-  if(wev_) {
-    event_free(wev_);
-  }
   if(settings_timerev_) {
     event_free(settings_timerev_);
   }
@@ -363,6 +352,10 @@ Http2Handler::~Http2Handler()
     SSL_set_shutdown(ssl_, SSL_RECEIVED_SHUTDOWN);
     SSL_shutdown(ssl_);
   }
+  if(bev_) {
+    bufferevent_disable(bev_, EV_READ | EV_WRITE);
+    bufferevent_free(bev_);
+  }
   if(ssl_) {
     SSL_free(ssl_);
   }
@@ -376,308 +369,93 @@ void Http2Handler::remove_self()
 }
 
 namespace {
-void rev_cb(evutil_socket_t fd, short what, void *arg)
+void readcb(bufferevent *bev, void *arg)
 {
   int rv;
   auto handler = static_cast<Http2Handler*>(arg);
 
-  if(what & EV_READ) {
+  rv = handler->on_read();
-    rv = handler->on_read();
+  if(rv == -1) {
-    if(rv == -1) {
+    delete_handler(handler);
-      delete_handler(handler);
-    }
   }
 }
 } // namespace
 
 namespace {
-void wev_cb(evutil_socket_t fd, short what, void *arg)
+void writecb(bufferevent *bev, void *arg)
 {
   int rv;
   auto handler = static_cast<Http2Handler*>(arg);
 
-  if(what & EV_WRITE) {
+  rv = handler->on_write();
-    rv = handler->on_write();
+  if(rv == -1) {
-    if(rv == -1) {
+    delete_handler(handler);
-      delete_handler(handler);
-    }
   }
 }
 } // namespace
 
-int Http2Handler::handle_ssl_temporal_error(int err)
-{
-  auto sslerr = SSL_get_error(ssl_, err);
-
-  switch(sslerr) {
-  case SSL_ERROR_WANT_READ:
-    event_add(rev_, nullptr);
-    return 1;
-  case SSL_ERROR_WANT_WRITE:
-    event_add(wev_, nullptr);
-    return 1;
-  }
-
-  return -1;
-}
-
-int Http2Handler::tls_write(const uint8_t *data, size_t datalen)
-{
-  int rv;
-  size_t max_avail;
-
-  // OpenSSL sends at most 16K bytes
-  max_avail = ssl_ ?
-    std::min((ssize_t)16384, nghttp2_buf_avail(&sendbuf_)) :
-    nghttp2_buf_avail(&sendbuf_);
-
-  if(max_avail < datalen) {
-    if(nghttp2_buf_len(&sendbuf_) > 0) {
-      rv = tls_write_pending();
-
-      if(rv == -1) {
-        return -1;
-      }
-
-      if(rv == 1) {
-        pending_data_ = data;
-        pending_datalen_ = datalen;
-
-        return 1;
-      }
-    }
-
-    assert(nghttp2_buf_avail(&sendbuf_) >= (ssize_t)datalen);
-  }
-  //std::cerr << "DBG: copy " << datalen << " bytes" << std::endl;
-  sendbuf_.last = nghttp2_cpymem(sendbuf_.last, data, datalen);
-
-  return 0;
-}
-
-int Http2Handler::tls_write_pending()
-{
-  int rv;
-
-  if(nghttp2_buf_len(&sendbuf_) == 0) {
-    return 0;
-  }
-
-  for(;;) {
-    if(ssl_) {
-      ERR_clear_error();
-
-      rv = SSL_write(ssl_, sendbuf_.pos, nghttp2_buf_len(&sendbuf_));
-
-      if(rv == 0) {
-        return -1;
-      }
-      if(rv < 0) {
-        return handle_ssl_temporal_error(rv);
-      }
-    } else {
-      while((rv = write(fd_, sendbuf_.pos, nghttp2_buf_len(&sendbuf_))) &&
-            rv == -1 && errno == EINTR);
-
-      if(rv == 0) {
-        continue;
-      }
-      if(rv < 0) {
-        if(errno == EAGAIN || errno == EWOULDBLOCK) {
-          event_add(wev_, nullptr);
-          return 1;
-        }
-        return -1;
-      }
-    }
-
-    sendbuf_.pos += rv;
-
-    if(nghttp2_buf_len(&sendbuf_) == 0) {
-      nghttp2_buf_reset(&sendbuf_);
-
-      if(pending_data_) {
-        assert(nghttp2_buf_avail(&sendbuf_) >= (ssize_t)pending_datalen_);
-        sendbuf_.last = nghttp2_cpymem(sendbuf_.last,
-                                      pending_data_, pending_datalen_);
-        pending_data_ = nullptr;
-        pending_datalen_ = 0;
-
-        continue;
-      }
-
-      return 0;
-    }
-  }
-}
-
 namespace {
-void tls_handshake_cb(evutil_socket_t fd, short what, void *arg)
+void eventcb(bufferevent *bev, short events, void *arg)
 {
-  int rv;
   auto handler = static_cast<Http2Handler*>(arg);
 
-  if(what & (EV_READ | EV_WRITE)) {
+  if(events & (BEV_EVENT_EOF | BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
-    rv = handler->tls_handshake();
+    delete_handler(handler);
-    if(rv == -1) {
+
-      delete_handler(handler);
+    return;
-      return;
+  }
+
+  if(events & BEV_EVENT_CONNECTED) {
+    if(handler->get_sessions()->get_config()->verbose) {
+      std::cerr << "SSL/TLS handshake completed" << std::endl;
     }
-    if(rv == 1) {
+
+    if(handler->verify_npn_result() != 0) {
+      delete_handler(handler);
+
       return;
     }
 
-    rv = handler->on_connect();
+    if(handler->on_connect() != 0) {
-    if(rv != 0) {
       delete_handler(handler);
+
       return;
     }
   }
 }
 } // namespace
 
-int Http2Handler::tls_handshake()
-{
-  int rv;
-
-  ERR_clear_error();
-
-  rv = SSL_accept(ssl_);
-  if(rv == 0) {
-    return -1;
-  }
-  if(rv < 0) {
-    auto sslerr = SSL_get_error(ssl_, rv);
-
-    switch(sslerr) {
-    case SSL_ERROR_NONE:
-    case SSL_ERROR_WANT_X509_LOOKUP:
-    case SSL_ERROR_ZERO_RETURN:
-      break;
-    case SSL_ERROR_WANT_READ:
-      event_add(rev_, nullptr);
-      return 1;
-    case SSL_ERROR_WANT_WRITE:
-      event_add(wev_, nullptr);
-      return 1;
-    }
-  }
-
-  if(sessions_->get_config()->verbose) {
-    std::cerr << "SSL/TLS handshake completed" << std::endl;
-  }
-
-  if(verify_npn_result() != 0) {
-    return -1;
-  }
-
-  event_del(rev_);
-  event_del(wev_);
-
-  event_assign(rev_, sessions_->get_evbase(), fd_, EV_READ, rev_cb, this);
-  event_assign(wev_, sessions_->get_evbase(), fd_, EV_WRITE, wev_cb, this);
-
-  return 0;
-}
-
 int Http2Handler::setup_bev()
 {
+  auto evbase = sessions_->get_evbase();
+
   if(ssl_) {
-    rev_ = event_new(sessions_->get_evbase(), fd_, EV_READ, tls_handshake_cb,
+    bev_ = bufferevent_openssl_socket_new(evbase, fd_, ssl_,
-                     this);
+                                          BUFFEREVENT_SSL_ACCEPTING,
-    wev_ = event_new(sessions_->get_evbase(), fd_, EV_WRITE, tls_handshake_cb,
+                                          BEV_OPT_DEFER_CALLBACKS);
-                     this);
   } else {
-    rev_ = event_new(sessions_->get_evbase(), fd_, EV_READ, rev_cb, this);
+    bev_ = bufferevent_socket_new(evbase, fd_, BEV_OPT_DEFER_CALLBACKS);
-    wev_ = event_new(sessions_->get_evbase(), fd_, EV_WRITE, wev_cb, this);
   }
 
-  event_add(rev_, nullptr);
+  bufferevent_enable(bev_, EV_READ);
-  // TODO set up timeout here
+  bufferevent_setcb(bev_, readcb, writecb, eventcb, this);
 
   return 0;
 }
 
-int Http2Handler::wait_events()
+int Http2Handler::send()
-{
-  int active = 0;
-
-  if(nghttp2_session_want_read(session_)) {
-    event_add(rev_, nullptr);
-    active = 1;
-  }
-
-  if(nghttp2_session_want_write(session_)) {
-    event_add(wev_, nullptr);
-    active = 1;
-  }
-
-  if(pending_datalen_ > 0) {
-    active = 1;
-  }
-
-  return active ? 0 : -1;
-}
-
-int Http2Handler::on_read()
 {
   int rv;
   uint8_t buf[16384];
-  uint8_t *bufp;
+  auto output = bufferevent_get_output(bev_);
-  size_t nread;
+  util::EvbufferBuffer evbbuf(output, buf, sizeof(buf));
-
-  if(ssl_) {
-    ERR_clear_error();
-    rv = SSL_read(ssl_, buf, sizeof(buf));
-
-    if(rv == 0) {
-      return -1;
-    }
-    if(rv < 0) {
-      return handle_ssl_temporal_error(rv);
-    }
-  } else {
-    while((rv = read(fd_, buf, sizeof(buf))) && rv == -1 && errno == EINTR);
-
-    if(rv == 0) {
-      return -1;
-    }
-    if(rv < 0) {
-      if(errno == EAGAIN || errno == EWOULDBLOCK) {
-        event_add(rev_, nullptr);
-        return 1;
-      }
-      return -1;
-    }
-  }
-
-  nread = rv;
-  bufp = buf;
-
-  rv = nghttp2_session_mem_recv(session_, bufp, nread);
-  if(rv < 0) {
-    std::cerr << "nghttp2_session_mem_recv() returned error: "
-              << nghttp2_strerror(rv) << std::endl;
-    return -1;
-  }
-
-  return wait_events();
-}
-
-int Http2Handler::on_write()
-{
-  int rv;
-
-  //std::cerr << "DBG: on_write" << std::endl;
-
-  rv = tls_write_pending();
-  if(rv != 0) {
-    return rv;
-  }
-
   for(;;) {
-    const uint8_t *data;
+    // Check buffer length and break if it is large enough.
+    if(evbuffer_get_length(output) + evbbuf.get_buflen() >= 65536) {
+      break;
+    }
 
+    const uint8_t *data;
     auto datalen = nghttp2_session_mem_send(session_, &data);
 
     if(datalen < 0) {
@@ -685,24 +463,63 @@ int Http2Handler::on_write()
                 << nghttp2_strerror(datalen) << std::endl;
       return -1;
     }
-
     if(datalen == 0) {
       break;
     }
-
+    rv = evbbuf.add(data, datalen);
-    rv = tls_write(data, datalen);
-
     if(rv != 0) {
-      return rv;
+      std::cerr << "evbuffer_add() failed" << std::endl;
+      return -1;
     }
   }
 
-  rv = tls_write_pending();
+  rv = evbbuf.flush();
   if(rv != 0) {
-    return rv;
+    std::cerr << "evbuffer_add() failed" << std::endl;
+    return -1;
   }
 
-  return wait_events();
+  if(nghttp2_session_want_read(session_) == 0 &&
+     nghttp2_session_want_write(session_) == 0 &&
+     evbuffer_get_length(output) == 0) {
+
+    return -1;
+  }
+
+  return 0;
+}
+
+int Http2Handler::on_read()
+{
+  int rv;
+
+  auto input = bufferevent_get_input(bev_);
+
+  for(;;) {
+    auto len = evbuffer_get_contiguous_space(input);
+
+    if(len == 0) {
+      break;
+    }
+
+    auto data = evbuffer_pullup(input, len);
+
+    rv = nghttp2_session_mem_recv(session_, data, len);
+    if(rv < 0) {
+      std::cerr << "nghttp2_session_mem_recv() returned error: "
+                << nghttp2_strerror(rv) << std::endl;
+      return -1;
+    }
+
+    evbuffer_drain(input, len);
+  }
+
+  return send();
+}
+
+int Http2Handler::on_write()
+{
+  return send();
 }
 
 namespace {

src/HttpServer.h

@@ -40,6 +40,7 @@
 #include <openssl/ssl.h>
 
 #include <event2/event.h>
+#include <event2/bufferevent.h>
 
 #include <nghttp2/nghttp2.h>
 
@@ -104,12 +105,11 @@ public:
 
   void remove_self();
   int setup_bev();
+  int send();
   int on_read();
   int on_write();
   int on_connect();
   int verify_npn_result();
-  int sendcb(const uint8_t *data, size_t len);
-  int recvcb(uint8_t *buf, size_t len);
 
   int submit_file_response(const std::string& status,
                            Stream *stream,
@@ -141,25 +141,15 @@ public:
   const Config* get_config() const;
   void remove_settings_timer();
   void terminate_session(uint32_t error_code);
-  int tls_handshake();
 private:
-  int handle_ssl_temporal_error(int err);
-  int tls_write(const uint8_t *data, size_t datalen);
-  int tls_write_pending();
-  int wait_events();
-
   std::map<int32_t, std::unique_ptr<Stream>> id2stream_;
-  nghttp2_buf sendbuf_;
   int64_t session_id_;
   nghttp2_session *session_;
   Sessions *sessions_;
-  SSL* ssl_;
+  SSL *ssl_;
-  event *rev_, *wev_;
+  bufferevent *bev_;
   event *settings_timerev_;
-  const uint8_t *pending_data_;
-  size_t pending_datalen_;
   int fd_;
-  uint8_t sendbufarray_[65536];
 };
 
 class HttpServer {