[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 e63e136..4e7f734 100644
--- 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 0e5f324..f2f312e 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -2321,7 +2321,7 @@
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 @@
/* 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 @@
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 @@
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 9b7277f..871333a 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -64,42 +64,23 @@
* 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 */