From: Willy Tarreau <w@1wt.eu>
Date: Tue, 28 Feb 2017 08:48:11 +0000 (+0100)
Subject: MINOR: http: don't close when redirect location doesn't start with "/"
X-Git-Tag: v1.8-dev1~134
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=19b1412e021451d4c7ac39750b556efaaf8639bf;p=thirdparty%2Fhaproxy.git

MINOR: http: don't close when redirect location doesn't start with "/"

In 1.4-dev5 when we started to implement keep-alive, commit a9679ac
("[MINOR] http: make the conditional redirect support keep-alive")
added a specific check was added to support keep-alive on redirect
rules but only when the location would start with a "/" indicating
the client would come back to the same server.

But nowadays most applications put http:// or https:// in front of
each and every location, and continuing to perform a close there is
counter-efficient, especially when multiple objects are fetched at
once from a same origin which redirects them to the correct origin
(eg: after an http to https forced upgrade).

It's about time to get rid of this old trick as it causes more harm
than good at an era where persistent connections are omnipresent.

Special thanks to Ciprian Dorin Craciun for providing convincing
arguments with a pretty valid use case and proposing this draft
patch which addresses the issue he was facing.

This change although not exactly a bug fix should be backported
to 1.7 to adapt better to existing infrastructure.
---

diff --git a/src/proto_http.c b/src/proto_http.c
index 5ad295676a..2d567c16ae 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -4032,7 +4032,6 @@ static int http_apply_redirect_rule(struct redirect_rule *rule, struct stream *s
 	struct http_msg *req = &txn->req;
 	struct http_msg *res = &txn->rsp;
 	const char *msg_fmt;
-	const char *location;
 	struct chunk *chunk;
 	int ret = 0;
 
@@ -4063,8 +4062,6 @@ static int http_apply_redirect_rule(struct redirect_rule *rule, struct stream *s
 	if (unlikely(!chunk_strcpy(chunk, msg_fmt)))
 		goto leave;
 
-	location = chunk->str + chunk->len;
-
 	switch(rule->type) {
 	case REDIRECT_TYPE_SCHEME: {
 		const char *path;
@@ -4228,17 +4225,12 @@ static int http_apply_redirect_rule(struct redirect_rule *rule, struct stream *s
 		chunk->len += rule->cookie_len;
 	}
 
-	/* add end of headers and the keep-alive/close status.
-	 * We may choose to set keep-alive if the Location begins
-	 * with a slash, because the client will come back to the
-	 * same server.
-	 */
+	/* add end of headers and the keep-alive/close status. */
 	txn->status = rule->code;
 	/* let's log the request time */
 	s->logs.tv_request = now;
 
-	if (*location == '/' &&
-	    (req->flags & HTTP_MSGF_XFER_LEN) &&
+	if ((req->flags & HTTP_MSGF_XFER_LEN) &&
 	    ((!(req->flags & HTTP_MSGF_TE_CHNK) && !req->body_len) || (req->msg_state == HTTP_MSG_DONE)) &&
 	    ((txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_SCL ||
 	     (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL)) {