[BUG] stream_interface: fix retnclose and remove cond_close

The stream_int_cond_close() function was added to preserve the
contents of the response buffer because stream_int_retnclose()
was buggy. It flushed the response instead of flushing the
request. This caused issues with pipelined redirects followed
by error messages which ate the previous response.

This might even have caused object truncation on pipelined
requests followed by an error or by a server redirection.

Now that this is fixed, simply get rid of the now useless
function.

diff --git a/include/proto/stream_interface.h b/include/proto/stream_interface.h
index e63e1361808dfa4b97a165084648c211cbb4591d..4e7f7341959e1e20c6f0df9124fc546cb4b8fef7 100644 (file)
--- a/include/proto/stream_interface.h
+++ b/include/proto/stream_interface.h
@@ -32,7 +32,6 @@
 int stream_int_check_timeouts(struct stream_interface *si);
 void stream_int_report_error(struct stream_interface *si);
 void stream_int_retnclose(struct stream_interface *si, const struct chunk *msg);
-void stream_int_cond_close(struct stream_interface *si, const struct chunk *msg);
 
 /* functions used when running a stream interface as a task */
 void stream_int_update(struct stream_interface *si);

diff --git a/src/proto_http.c b/src/proto_http.c
index 0e5f324f26411a4d32e4dc672dddd7bf9d68b902..f2f312e3b756bfa82d21e82d62740536cced51e7 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -2321,7 +2321,7 @@ int http_wait_for_request(struct session *s, struct buffer *req, int an_bit)
                msg->msg_state = HTTP_MSG_RQBEFORE;
                req->analysers = 0;
                s->logs.logwait = 0;
-               stream_int_cond_close(req->prod, NULL);
+               stream_int_retnclose(req->prod, NULL);
                return 0;
        }
 
@@ -2882,8 +2882,7 @@ int http_process_req_common(struct session *s, struct buffer *req, int an_bit, s
                                /* keep-alive not possible */
                                memcpy(rdr.str + rdr.len, "\r\nConnection: close\r\n\r\n", 23);
                                rdr.len += 23;
-                               buffer_write(req->prod->ob, rdr.str, rdr.len);
-                               stream_int_cond_close(req->prod, NULL);
+                               stream_int_retnclose(req->prod, &rdr);
                                goto return_prx_cond;
                        }
                }
@@ -3910,10 +3909,7 @@ int http_request_forward_body(struct session *s, struct buffer *req, int an_bit)
        txn->req.msg_state = HTTP_MSG_ERROR;
        txn->status = 400;
        /* Note: we don't send any error if some data were already sent */
-       stream_int_cond_close(req->prod, (txn->rsp.msg_state < HTTP_MSG_BODY) ? error_message(s, HTTP_ERR_400) : NULL);
-
-       buffer_auto_read(req);
-       buffer_auto_close(req);
+       stream_int_retnclose(req->prod, (txn->rsp.msg_state < HTTP_MSG_BODY) ? error_message(s, HTTP_ERR_400) : NULL);
        req->analysers = 0;
        s->fe->counters.failed_req++;
        if (s->listener->counters)
@@ -4857,10 +4853,8 @@ int http_response_forward_body(struct session *s, struct buffer *res, int an_bit
  return_bad_res: /* let's centralize all bad resuests */
        txn->rsp.msg_state = HTTP_MSG_ERROR;
        txn->status = 502;
-       stream_int_cond_close(res->cons, NULL);
-
-       buffer_auto_close(res);
-       buffer_auto_read(res);
+       /* don't send any error message as we're in the body */
+       stream_int_retnclose(res->cons, NULL);
        res->analysers = 0;
        s->be->counters.failed_resp++;
        if (s->srv) {

diff --git a/src/stream_interface.c b/src/stream_interface.c
index 9b7277fad690656425c425fcbc036c9f0cd4350b..871333afabbe441adb9e6e2cb9677b29b26aabc5 100644 (file)
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -64,42 +64,23 @@ void stream_int_report_error(struct stream_interface *si)
  * and the request is cleared so that no server connection can be initiated.
  * The buffer is marked for read shutdown on the other side to protect the
  * message, and the buffer write is enabled. The message is contained in a
- * "chunk". If it is null, then an empty message is used. The reply buffer
- * doesn't need to be empty before this. The goal of this function is to
- * return error messages to a client.
+ * "chunk". If it is null, then an empty message is used. The reply buffer does
+ * not need to be empty before this, and its contents will not be overwritten.
+ * The primary goal of this function is to return error messages to a client.
  */
 void stream_int_retnclose(struct stream_interface *si, const struct chunk *msg)
 {
+       buffer_auto_read(si->ib);
        buffer_abort(si->ib);
-       buffer_erase(si->ob);
-       buffer_shutr_now(si->ob);
-       if (msg && msg->len)
+       buffer_auto_close(si->ib);
+       buffer_erase(si->ib);
+       if (likely(msg && msg->len))
                buffer_write(si->ob, msg->str, msg->len);
 
        si->ob->wex = tick_add_ifset(now_ms, si->ob->wto);
+       buffer_auto_read(si->ob);
        buffer_auto_close(si->ob);
-}
-
-/* This function aborts all of the client's requests and stops the server's
- * response with the data that are already present. If the response buffer
- * empty, then the message in arguments will be sent. This ensures that
- * we won't mix an HTTP response with pending data. The buffer is marked for
- * read shutdown on the server side to protect the message or any pending
- * data, and the buffer write is enabled. The message is contained in a
- * "chunk". If it is null, then an empty message is used. The reply buffer
- * doesn't need to be empty before this. The goal of this function is to
- * return error messages to a client but only when it is possible.
- */
-void stream_int_cond_close(struct stream_interface *si, const struct chunk *msg)
-{
-       buffer_abort(si->ib);
-       buffer_erase(si->ib); /* remove any pending request */
        buffer_shutr_now(si->ob);
-       if (!si->ob->l && msg && msg->len)
-               buffer_write(si->ob, msg->str, msg->len);
-
-       si->ob->wex = tick_add_ifset(now_ms, si->ob->wto);
-       buffer_auto_close(si->ob);
 }
 
 /* default update function for scheduled tasks, not used for embedded tasks */