nghttpx: Fix bug that data buffered in SSL object are not read

This is same issue described in https://github.com/h2o/h2o/issues/268.
That is if SSL object has decrypted data buffered inside it, and
application does not read it for some reason (e.g., rate limit), we
have to check the existence of data using SSL_pending.  This is
because buffered data inside SSL is not notified by io watcher.  It is
obvious, but we totally missed it.

nghttpx code normally reads everything until SSL_read returns error
(want-read).  But if rate limit is involved, we stop reading early.
Also in HTTP/1 code, while processing one request, we just read until
buffer is filled up.  In these cases, we may suffer from this problem.

This commit fixes this problem, by performing SSL_pending() and if it
has buffered data and read io watcher is enabled, we feed event using
ev_feed_event().

src/shrpx_client_handler.cc

@@ -560,7 +560,14 @@ int ClientHandler::validate_next_proto() {
 int ClientHandler::do_read() { return read_(*this); }
 int ClientHandler::do_write() { return write_(*this); }
 
-int ClientHandler::on_read() { return on_read_(*this); }
+int ClientHandler::on_read() {
+  auto rv = on_read_(*this);
+  if (rv != 0) {
+    return rv;
+  }
+  conn_.handle_tls_pending_read();
+  return 0;
+}
 int ClientHandler::on_write() { return on_write_(*this); }
 
 const std::string &ClientHandler::get_ipaddr() const { return ipaddr_; }

src/shrpx_connection.cc

@@ -41,7 +41,7 @@ Connection::Connection(struct ev_loop *loop, int fd, SSL *ssl,
                        size_t read_burst, IOCb writecb, IOCb readcb,
                        TimerCb timeoutcb, void *data)
     : tls{ssl}, wlimit(loop, &wev, write_rate, write_burst),
-      rlimit(loop, &rev, read_rate, read_burst), writecb(writecb),
+      rlimit(loop, &rev, read_rate, read_burst, ssl), writecb(writecb),
       readcb(readcb), timeoutcb(timeoutcb), loop(loop), data(data), fd(fd) {
 
   ev_io_init(&wev, writecb, fd, EV_WRITE);
@@ -303,4 +303,11 @@ ssize_t Connection::read_clear(void *data, size_t len) {
   return nread;
 }
 
+void Connection::handle_tls_pending_read() {
+  if (!ev_is_active(&rev)) {
+    return;
+  }
+  rlimit.handle_tls_pending_read();
+}
+
 } // namespace shrpx

src/shrpx_connection.h

@@ -83,6 +83,8 @@ struct Connection {
   ssize_t writev_clear(struct iovec *iov, int iovcnt);
   ssize_t read_clear(void *data, size_t len);
 
+  void handle_tls_pending_read();
+
   TLSConnection tls;
   ev_io wev;
   ev_io rev;

src/shrpx_rate_limit.cc

@@ -35,8 +35,9 @@ void regencb(struct ev_loop *loop, ev_timer *w, int revents) {
 }
 } // namespace
 
-RateLimit::RateLimit(struct ev_loop *loop, ev_io *w, size_t rate, size_t burst)
-    : w_(w), loop_(loop), rate_(rate), burst_(burst), avail_(burst),
+RateLimit::RateLimit(struct ev_loop *loop, ev_io *w, size_t rate, size_t burst,
+                     SSL *ssl)
+    : w_(w), loop_(loop), ssl_(ssl), rate_(rate), burst_(burst), avail_(burst),
       startw_req_(false) {
   ev_timer_init(&t_, regencb, 0., 1.);
   t_.data = this;
@@ -45,9 +46,7 @@ RateLimit::RateLimit(struct ev_loop *loop, ev_io *w, size_t rate, size_t burst)
   }
 }
 
-RateLimit::~RateLimit() {
-  ev_timer_stop(loop_, &t_);
-}
+RateLimit::~RateLimit() { ev_timer_stop(loop_, &t_); }
 
 size_t RateLimit::avail() const {
   if (rate_ == 0) {
@@ -79,6 +78,7 @@ void RateLimit::regen() {
 
   if (avail_ > 0 && startw_req_) {
     ev_io_start(loop_, w_);
+    handle_tls_pending_read();
   }
 }
 
@@ -86,6 +86,7 @@ void RateLimit::startw() {
   startw_req_ = true;
   if (rate_ == 0 || avail_ > 0) {
     ev_io_start(loop_, w_);
+    handle_tls_pending_read();
     return;
   }
 }
@@ -95,4 +96,14 @@ void RateLimit::stopw() {
   ev_io_stop(loop_, w_);
 }
 
+void RateLimit::handle_tls_pending_read() {
+  if (!ssl_ || SSL_pending(ssl_) == 0) {
+    return;
+  }
+
+  // Note that ev_feed_event works without starting watcher, but we
+  // only call this function if watcher is active.
+  ev_feed_event(loop_, w_, EV_READ);
+}
+
 } // namespace shrpx

src/shrpx_rate_limit.h

@@ -29,21 +29,31 @@
 
 #include <ev.h>
 
+#include <openssl/ssl.h>
+
 namespace shrpx {
 
 class RateLimit {
 public:
-  RateLimit(struct ev_loop *loop, ev_io *w, size_t rate, size_t burst);
+  // We need |ssl| object to check that it has unread decrypted bytes.
+  RateLimit(struct ev_loop *loop, ev_io *w, size_t rate, size_t burst,
+            SSL *ssl = nullptr);
   ~RateLimit();
   size_t avail() const;
   void drain(size_t n);
   void regen();
   void startw();
   void stopw();
+  // Feeds event if ssl_ object has unread decrypted bytes.  This is
+  // required since it is buffered in ssl_ object, io event is not
+  // generated unless new incoming data is received.
+  void handle_tls_pending_read();
+
 private:
-  ev_io *w_;
   ev_timer t_;
+  ev_io *w_;
   struct ev_loop *loop_;
+  SSL *ssl_;
   size_t rate_;
   size_t burst_;
   size_t avail_;