Le 02/01/2020 à 10:58, Willy Tarreau a écrit :
Thanks Kevin, it seems I missed this one.
On Fri, Dec 27, 2019 at 05:38:15PM +0800, Kevin Zhu wrote:
global
log 127.0.0.1 local4 info
defaults
mode http
timeout connect 10s
timeout client 300s
timeout server 300s
listen inhttp
option httplog
log global
bind ipv4@:80
log-format %ci:%cp/%b/%si:%sp\ %ST\ %ts\ %U/%B\ %{+Q}r\ %sslv\ %hr
http-response redirect location https://www.google.com if TRUE
use_backend be_1
backend be_1
server out 10.20.136.221:8080
source 0.0.0.0 usesrc 10.20.136.225
[root@localhost haproxy]#
[root@localhost haproxy]# ./haproxy-master -f haproxy.conf.master
HAProxy log:
Dec 27 03:23:56 127.0.0.1 haproxy-master[19682]: Proxy inhttp started.
Dec 27 03:24:10 127.0.0.1 haproxy-master[19682]:
10.20.136.222:63654/be_1/10.20.136.221:8080 302 LR 89/404 "GET /ping
HTTP/1.1"
Client host:
[root@localhost ~]# curl http://10.20.136.225/ping -v
* Trying 10.20.136.225...
* TCP_NODELAY set
* Connected to 10.20.136.225 (10.20.136.225) port 80 (#0)
GET /ping HTTP/1.1
Host: 10.20.136.225
User-Agent: curl/7.29.0
Accept: */*
< HTTP/1.1 200 OK
< content-type: text/plain; charset=utf-8
< date: Thu, 26 Dec 2019 19:24:10 GMT
< content-length: 23
<
This is response body.
* Curl_http_done: called premature == 0
* Connection #0 to host 10.20.136.225 left intact
[root@localhost ~]#
The config file have rule:
http-response redirect location https://www.google.com if TRUE
and HAProxy log show the response code is 302, but client got server's
response, not redirect.
OK that's clearer now. I tried your config and the results vary a lot
depending on versions, I think there were multiple breakage stages:
- 1.8 works as expected
- 1.9 and 2.0 in legacy mode returns 200 but apparently limited to the
size of the redirect so it misses some headers and the final CRLF,
and hangs till the client timeout strikes
- 1.9 and 2.0 in HTX mode returns the whole 200 response
- 2.1 and above only support HTX and behave like 2.0 for this
I'll rather wait for Christopher next week to double-check your patch and
to see if the issue is deeper or just related to this, especially for the
HTX case which looks odd.
Hi Kevin,
Indeed there is a bug in HTX mode when a redirect is performed on the response
path. The response is not truncated. So your fix is valid. I've attached an
updated patch with a commit message to describe the bug.
On the legacy mode (<= 2.0), there is also a bug but not in the HTTP part. It is
a bug in co_inject(). It appends data at the end of input, not the end of
output. The bug was introduced when the buffer API was refactored. The bug is
also attached.
I'll push the patches and backport them as far as 1.9 very soon. Thanks.
--
Christopher Faulet
>From 96b363963f4a4a63823718966798f177a72936b6 Mon Sep 17 00:00:00 2001
From: Kevin Zhu <ip0...@gmail.com>
Date: Tue, 7 Jan 2020 09:42:55 +0100
Subject: [PATCH 1/2] BUG/MEDIUM: http-ana: Truncate the response when a
redirect rule is applied
When a redirect rule is executed on the response path, we must truncate the
received response. Otherwise, the redirect is appended after the response, which
is sent to the client. So it is obviously a bug because the redirect is not
performed. With bodyless responses, it is the "only" bug. But if the response
has a body, the result may be invalid. If the payload is not fully received yet
when the redirect is performed, an internal error is reported.
It must be backported as far as 1.9.
---
src/http_ana.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/http_ana.c b/src/http_ana.c
index ee00d2c76..268796d2e 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -2526,6 +2526,8 @@ int http_apply_redirect_rule(struct redirect_rule *rule, struct stream *s, struc
close = 1;
htx = htx_from_buf(&res->buf);
+ /* Trim any possible response */
+ channel_htx_truncate(&s->res, htx);
flags = (HTX_SL_F_IS_RESP|HTX_SL_F_VER_11|HTX_SL_F_XFER_LEN|HTX_SL_F_BODYLESS);
sl = htx_add_stline(htx, HTX_BLK_RES_SL, flags, ist("HTTP/1.1"), status, reason);
if (!sl)
@@ -2553,6 +2555,8 @@ int http_apply_redirect_rule(struct redirect_rule *rule, struct stream *s, struc
if (!htx_add_endof(htx, HTX_BLK_EOH) || !htx_add_endof(htx, HTX_BLK_EOM))
goto fail;
+ htx_to_buf(htx, &res->buf);
+
/* let's log the request time */
s->logs.tv_request = now;
--
2.24.1
>From 584348be636fcc9f41b80ef0fde03c7899d75cd7 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@haproxy.com>
Date: Tue, 7 Jan 2020 10:01:57 +0100
Subject: [PATCH 2/2] BUG/MINOR: channel: inject output data at the end of
output
In co_inject(), data must be inserted at the end of output, not the end of
input. For the record, this function does not take care of input data which are
supposed to not exist. But the caller may reset input data after or before the
call. It is its own choice.
This bug, among other effects, is visible when a redirect is performed on
the response path, on legacy HTTP mode (so for HAProxy < 2.1). The redirect
response is appended after the server response when it should overwrite it.
Thanks to Kevin Zhu <ip0...@gmail.com> to report the bug. It must be backported
as far as 1.9.
---
src/channel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/channel.c b/src/channel.c
index d4a46ffed..8b0854ef5 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -96,7 +96,7 @@ int co_inject(struct channel *chn, const char *msg, int len)
if (len > max)
return max;
- memcpy(ci_tail(chn), msg, len);
+ memcpy(co_tail(chn), msg, len);
b_add(&chn->buf, len);
c_adv(chn, len);
chn->total += len;
--
2.24.1
