From: Matthieu Guegan <mguegan@virtua.ch>
Date: Mon, 5 Dec 2016 10:35:54 +0000 (+0100)
Subject: BUG/MINOR: http: don't send an extra CRLF after a Set-Cookie in a redirect
X-Git-Tag: v1.8-dev1~305
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=35088f960de9e3331573d118adbbe956c7fbbf7d;p=thirdparty%2Fhaproxy.git

BUG/MINOR: http: don't send an extra CRLF after a Set-Cookie in a redirect

By investigating a keep-alive issue with CloudFlare, we[1] found that
when using the 'set-cookie' option in a redirect (302) HAproxy is adding
an extra `\r\n`.

Triggering rule :

`http-request redirect location / set-cookie Cookie=value if [...]`

Expected result :

```
HTTP/1.1 302 Found
Cache-Control: no-cache
Content-length: 0
Location: /
Set-Cookie: Cookie=value; path=/;
Connection: close
```

Actual result :

```
HTTP/1.1 302 Found
Cache-Control: no-cache
Content-length: 0
Location: /
Set-Cookie: Cookie=value; path=/;

Connection: close
```

This extra `\r\n` seems to be harmless with another HAproxy instance in
front of it (sanitizing) or when using a browser. But we confirm that
the CloudFlare NGINX implementation is not able to handle this. It
seems that both 'Content-length: 0' and extra carriage return broke RFC
(to be confirmed).

When looking into the code, this carriage-return was already present in
1.3.X versions but just before closing the connection which was ok I
think. Then, with 1.4.X the keep-alive feature was added and this piece
of code remains unchanged.

[1] all credit for the bug finding goes to CloudFlare Support Team

[wt: the bug was indeed present since the Set-Cookie was introduced
 in 1.3.16, by commit 0140f25 ("[MINOR] redirect: add support for
 "set-cookie" and "clear-cookie"") so backporting to all supported
 versions is desired]
---

diff --git a/src/proto_http.c b/src/proto_http.c
index a37e2909fa..5ac9bf3cfe 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -4185,8 +4185,6 @@ static int http_apply_redirect_rule(struct redirect_rule *rule, struct stream *s
 		trash.len += 14;
 		memcpy(trash.str + trash.len, rule->cookie_str, rule->cookie_len);
 		trash.len += rule->cookie_len;
-		memcpy(trash.str + trash.len, "\r\n", 2);
-		trash.len += 2;
 	}
 
 	/* add end of headers and the keep-alive/close status.