From 542fd6420b5eb2b669591c6bed18bccb6f8ca0df Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Mon, 19 Nov 2012 02:11:46 +0900
Subject: [PATCH] Fix recursive HttpsUpstream::on_read() call

Don't call HttpsUpstream::resume_read() from the call tree of
on_read().  Avoid parsing next http data after parse error.
---
 src/shrpx_https_upstream.cc             |  7 +++++-
 src/shrpx_spdy_downstream_connection.cc | 32 ++++++++-----------------
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc
index 75a5db82..5b0c2f05 100644
--- a/src/shrpx_https_upstream.cc
+++ b/src/shrpx_https_upstream.cc
@@ -194,11 +194,11 @@ int htp_msg_completecb(http_parser *htp)
   HttpsUpstream *upstream;
   upstream = reinterpret_cast<HttpsUpstream*>(htp->data);
   Downstream *downstream = upstream->get_downstream();
+  downstream->set_request_state(Downstream::MSG_COMPLETE);
   rv = downstream->end_upload_data();
   if(rv != 0) {
     return -1;
   }
-  downstream->set_request_state(Downstream::MSG_COMPLETE);
   // Stop further processing to complete this request
   http_parser_pause(htp, 1);
   return 0;
@@ -238,10 +238,13 @@ int HttpsUpstream::on_read()
   // Well, actually header length + some body bytes
   current_header_length_ += nread;
   Downstream *downstream = get_downstream();
+
   http_errno htperr = HTTP_PARSER_ERRNO(htp_);
   if(htperr == HPE_PAUSED) {
     if(downstream->get_request_state() == Downstream::CONNECT_FAIL) {
       get_client_handler()->set_should_close_after_write(true);
+      // Following paues_read is needed to avoid reading next data.
+      pause_read(SHRPX_MSG_BLOCK);
       if(error_reply(503) != 0) {
         return -1;
       }
@@ -264,6 +267,7 @@ int HttpsUpstream::on_read()
         LOG(WARNING) << "Request Header too long:" << current_header_length_
                      << " bytes";
         get_client_handler()->set_should_close_after_write(true);
+        pause_read(SHRPX_MSG_BLOCK);
         if(error_reply(400) != 0) {
           return -1;
         }
@@ -281,6 +285,7 @@ int HttpsUpstream::on_read()
                 << http_errno_description(htperr);
     }
     get_client_handler()->set_should_close_after_write(true);
+    pause_read(SHRPX_MSG_BLOCK);
     if(error_reply(400) != 0) {
       return -1;
     }
diff --git a/src/shrpx_spdy_downstream_connection.cc b/src/shrpx_spdy_downstream_connection.cc
index 0f88e511..bd6c8ec8 100644
--- a/src/shrpx_spdy_downstream_connection.cc
+++ b/src/shrpx_spdy_downstream_connection.cc
@@ -77,20 +77,6 @@ SpdyDownstreamConnection::~SpdyDownstreamConnection()
   }
 }
 
-namespace {
-void body_buf_cb(evbuffer *body, size_t oldlen, size_t newlen, void *arg)
-{
-  SpdyDownstreamConnection *dconn;
-  dconn = reinterpret_cast<SpdyDownstreamConnection*>(arg);
-  if(newlen == 0) {
-    Downstream *downstream = dconn->get_downstream();
-    if(downstream) {
-      downstream->get_upstream()->resume_read(SHRPX_NO_BUFFER);
-    }
-  }
-}
-} // namespace
-
 int SpdyDownstreamConnection::init_request_body_buf()
 {
   int rv;
@@ -105,7 +91,7 @@ int SpdyDownstreamConnection::init_request_body_buf()
     if(request_body_buf_ == 0) {
       return -1;
     }
-    evbuffer_setcb(request_body_buf_, body_buf_cb, this);
+    evbuffer_setcb(request_body_buf_, 0, this);
   }
   return 0;
 }
@@ -312,17 +298,19 @@ int SpdyDownstreamConnection::push_upload_data_chunk(const uint8_t *data,
     LOG(FATAL) << "evbuffer_add() failed";
     return -1;
   }
+  if(downstream_->get_downstream_stream_id() != -1) {
+    spdylay_session_resume_data(session_,
+                                downstream_->get_downstream_stream_id());
+    rv = send();
+    if(rv != 0) {
+      return -1;
+    }
+  }
   size_t bodylen = evbuffer_get_length(request_body_buf_);
   if(bodylen > Downstream::DOWNSTREAM_OUTPUT_UPPER_THRES) {
     downstream_->get_upstream()->pause_read(SHRPX_NO_BUFFER);
   }
-  if(downstream_->get_downstream_stream_id() != -1) {
-    spdylay_session_resume_data(session_,
-                                downstream_->get_downstream_stream_id());
-    return send();
-  } else {
-    return 0;
-  }
+  return 0;
 }
 
 int SpdyDownstreamConnection::end_upload_data()