From 7b3f57cef8b988f2d2512f3b2edf4f40d6844eaf Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 27 Feb 2013 22:39:44 +0900 Subject: [PATCH] shrpx: Fix blocking upstream RST_STREAM and propagate REFUSED_STREAM This change fixes upstream RST_STREAM is blocked until SpdyUpstream::send() is called. Now downstream REFUSED_STREAM is propagated to upstream client so that client can reset request. The RST_STREAM error code when downstream went wrong is changed from CANCEL to INTERNAL_ERROR. --- src/shrpx_downstream.cc | 10 ++++ src/shrpx_downstream.h | 4 ++ src/shrpx_spdy_downstream_connection.cc | 2 +- src/shrpx_spdy_session.cc | 2 + src/shrpx_spdy_upstream.cc | 61 +++++++++++++++---------- 5 files changed, 55 insertions(+), 24 deletions(-) diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index e52da2d..e915f0a 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -489,4 +489,14 @@ int32_t Downstream::get_downstream_stream_id() const return downstream_stream_id_; } +uint32_t Downstream::get_response_rst_stream_status_code() const +{ + return response_rst_stream_status_code_; +} + +void Downstream::set_response_rst_stream_status_code(uint32_t status_code) +{ + response_rst_stream_status_code_ = status_code; +} + } // namespace shrpx diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index 06d0e4a..31b5eca 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -128,6 +128,8 @@ public: int get_response_state() const; int init_response_body_buf(); evbuffer* get_response_body_buf(); + uint32_t get_response_rst_stream_status_code() const; + void set_response_rst_stream_status_code(uint32_t status_code); // Call this method when there is incoming data in downstream // connection. @@ -164,6 +166,8 @@ private: // This buffer is used to temporarily store downstream response // body. Spdylay reads data from this in the callback. evbuffer *response_body_buf_; + // RST_STREAM status_code from downstream SPDY connection + uint32_t response_rst_stream_status_code_; int32_t recv_window_size_; }; diff --git a/src/shrpx_spdy_downstream_connection.cc b/src/shrpx_spdy_downstream_connection.cc index 262ba24..f836706 100644 --- a/src/shrpx_spdy_downstream_connection.cc +++ b/src/shrpx_spdy_downstream_connection.cc @@ -146,7 +146,7 @@ int SpdyDownstreamConnection::submit_rst_stream(Downstream *downstream) } rv = spdy_->submit_rst_stream(this, downstream->get_downstream_stream_id(), - SPDYLAY_CANCEL); + SPDYLAY_INTERNAL_ERROR); } } return rv; diff --git a/src/shrpx_spdy_session.cc b/src/shrpx_spdy_session.cc index 187db71..c9c6b93 100644 --- a/src/shrpx_spdy_session.cc +++ b/src/shrpx_spdy_session.cc @@ -755,6 +755,8 @@ void on_ctrl_recv_callback // upstream connection must be terminated. downstream->set_response_state(Downstream::MSG_RESET); } + downstream->set_response_rst_stream_status_code + (frame->rst_stream.status_code); call_downstream_readcb(spdy, downstream); } } diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index 89e2f34..5193c80 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -315,6 +315,19 @@ void on_unknown_ctrl_recv_callback(spdylay_session *session, } } // namespace +namespace { +uint32_t infer_upstream_rst_stream_status_code(uint32_t downstream_status_code) +{ + // Only propagate SPDYLAY_REFUSED_STREAM so that upstream client + // can resend request. + if(downstream_status_code != SPDYLAY_REFUSED_STREAM) { + return SPDYLAY_INTERNAL_ERROR; + } else { + return downstream_status_code; + } +} +} // namespace + SpdyUpstream::SpdyUpstream(uint16_t version, ClientHandler *handler) : handler_(handler), session_(0) @@ -454,31 +467,32 @@ void spdy_downstream_readcb(bufferevent *bev, void *ptr) // RST_STREAM to the upstream and delete downstream connection // here. Deleting downstream will be taken place at // on_stream_close_callback. - upstream->rst_stream(downstream, SPDYLAY_CANCEL); - downstream->set_downstream_connection(0); - delete dconn; - return; - } - - int rv = downstream->on_read(); - if(rv != 0) { - if(LOG_ENABLED(INFO)) { - DCLOG(INFO, dconn) << "HTTP parser failure"; - } - if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) { - upstream->rst_stream(downstream, SPDYLAY_INTERNAL_ERROR); - } else { - if(upstream->error_reply(downstream, 502) != 0) { - delete upstream->get_client_handler(); - return; - } - } - downstream->set_response_state(Downstream::MSG_COMPLETE); - // Clearly, we have to close downstream connection on http parser - // failure. + upstream->rst_stream(downstream, infer_upstream_rst_stream_status_code + (downstream->get_response_rst_stream_status_code())); downstream->set_downstream_connection(0); delete dconn; dconn = 0; + } else { + int rv = downstream->on_read(); + if(rv != 0) { + if(LOG_ENABLED(INFO)) { + DCLOG(INFO, dconn) << "HTTP parser failure"; + } + if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) { + upstream->rst_stream(downstream, SPDYLAY_INTERNAL_ERROR); + } else { + if(upstream->error_reply(downstream, 502) != 0) { + delete upstream->get_client_handler(); + return; + } + } + downstream->set_response_state(Downstream::MSG_COMPLETE); + // Clearly, we have to close downstream connection on http parser + // failure. + downstream->set_downstream_connection(0); + delete dconn; + dconn = 0; + } } if(upstream->send() != 0) { delete upstream->get_client_handler(); @@ -674,7 +688,8 @@ ssize_t spdy_data_read_callback(spdylay_session *session, ULOG(INFO, upstream) << "RST_STREAM to tunneled stream stream_id=" << stream_id; } - upstream->rst_stream(downstream, SPDYLAY_CANCEL); + upstream->rst_stream(downstream, infer_upstream_rst_stream_status_code + (downstream->get_response_rst_stream_status_code())); } } if(nread == 0 && *eof != 1) {