From 82eeaf2fae39ab7eeeac5d1f29e964bf0168c04f Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 29 Dec 2009 12:09:05 +0100 Subject: [PATCH] [MEDIUM] http: properly handle "option forceclose" The "forceclose" option used to close the output channel to the server once it started to respond. While this happened to work with most servers, some of them considered this as a connection abort and immediately stopped responding. Now that we're aware of the end of a request and response, we're able to trivially handle this option and properly close both sides when the server's response is complete. During this change it appeared that forwarding could be allowed when the BF_SHUTW_NOW flag was set on a buffer, which obviously is not acceptable and was causing some trouble. This has been fixed too and is the reason for the MEDIUM status on this patch. --- doc/configuration.txt | 6 ++---- src/proto_http.c | 10 ++++++++++ src/session.c | 12 ++++-------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 4cafff8a1..e69aa7c9e 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -2393,10 +2393,8 @@ no option forceclose global session times in the logs. When this happens, it is possible to use "option forceclose". It will - actively close the outgoing server channel as soon as the server begins to - reply and only if the request buffer is empty. Note that this should NOT be - used if CONNECT requests are expected between the client and the server. This - option implicitly enables the "httpclose" option. + actively close the outgoing server channel as soon as the server has finished + to respond. This option implicitly enables the "httpclose" option. If this option has been enabled in a "defaults" section, it can be disabled in a specific instance by prepending the "no" keyword before it. diff --git a/src/proto_http.c b/src/proto_http.c index 60a3177b6..a5de8804d 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -3317,6 +3317,11 @@ int http_request_forward_body(struct session *s, struct buffer *req, int an_bit) * to reset the transaction here. */ + if ((s->fe->options | s->be->options) & PR_O_FORCE_CLO) { + /* option forceclose is set, let's enforce it now that the transfer is complete. */ + buffer_abort(req); + } + if (req->flags & (BF_SHUTW|BF_SHUTW_NOW)) { if (req->flags & BF_OUT_EMPTY) msg->msg_state = HTTP_MSG_CLOSED; @@ -4256,6 +4261,11 @@ int http_response_forward_body(struct session *s, struct buffer *res, int an_bit * to reset the transaction here. */ + if ((s->fe->options | s->be->options) & PR_O_FORCE_CLO) { + /* option forceclose is set, let's enforce it now that the transfer is complete. */ + buffer_abort(res); + } + if (res->flags & (BF_SHUTW|BF_SHUTW_NOW)) { if (res->flags & BF_OUT_EMPTY) msg->msg_state = HTTP_MSG_CLOSED; diff --git a/src/session.c b/src/session.c index 220a5d4fa..2261c932d 100644 --- a/src/session.c +++ b/src/session.c @@ -1000,7 +1000,7 @@ resync_stream_interface: * everything. We configure the buffer to forward indefinitely. */ if (!s->req->analysers && - !(s->req->flags & (BF_HIJACK|BF_SHUTW)) && + !(s->req->flags & (BF_HIJACK|BF_SHUTW|BF_SHUTW_NOW)) && (s->req->prod->state >= SI_ST_EST) && (s->req->to_forward != BUF_INFINITE_FORWARD)) { /* This buffer is freewheeling, there's no analyser nor hijacker @@ -1042,13 +1042,9 @@ resync_stream_interface: * happen either because the input is closed or because we want to force a close * once the server has begun to respond. */ - if ((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_AUTO_CLOSE)) == BF_AUTO_CLOSE) { - if (unlikely((s->req->flags & BF_SHUTR) || - ((s->req->cons->state == SI_ST_EST) && - (s->be->options & PR_O_FORCE_CLO) && - (s->rep->flags & BF_READ_ACTIVITY)))) + if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_AUTO_CLOSE|BF_SHUTR)) == + (BF_AUTO_CLOSE|BF_SHUTR))) buffer_shutw_now(s->req); - } /* shutdown(write) pending */ if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_OUT_EMPTY)) == (BF_SHUTW_NOW|BF_OUT_EMPTY))) @@ -1121,7 +1117,7 @@ resync_stream_interface: * everything. We configure the buffer to forward indefinitely. */ if (!s->rep->analysers && - !(s->rep->flags & (BF_HIJACK|BF_SHUTW)) && + !(s->rep->flags & (BF_HIJACK|BF_SHUTW|BF_SHUTW_NOW)) && (s->rep->prod->state >= SI_ST_EST) && (s->rep->to_forward != BUF_INFINITE_FORWARD)) { /* This buffer is freewheeling, there's no analyser nor hijacker