Le 15/01/2019 à 21:07, PiBa-NL a écrit :
Hi Christopher,
Op 15-1-2019 om 10:48 schreef Christopher Faulet:
Le 14/01/2019 à 21:53, PiBa-NL a écrit :
Hi Christopher,
Op 14-1-2019 om 11:17 schreef Christopher Faulet:
Le 12/01/2019 à 23:23, PiBa-NL a écrit :
Hi List,
I've configured haproxy with htx and when i try to filter the stats
webpage.
Sending this request: "GET /?;csv;scope=b1" to '2.0-dev0-762475e
2019/01/10' it will crash with the trace below.
1.9.0 and 1.9.1 are also affected.
Can someone take a look? Thanks in advance.
A regtest is attached that reproduces the behavior, and which i think
could be included into the haproxy repository.
Pieter,
Here is the patch that should fix this issue. This was "just" an
oversight when the stats applet has been adapted to support the HTX.
If it's ok for you, I'll also merge your regtest.
Thanks
It seems the patch did not change/fix the crash.? Below looks pretty
much the same as previously. Did i fail to apply the patch properly.? It
seems to have 'applied' properly checking a few lines of the touched
code manually. As for the regtest, yes please merge that if its okay
as-is, perhaps after the fix is also ready :).
Hi Pieter,
Sorry, I made my patch too quickly. It seemed ok, but obviously not...
This new one should do the trick.
Well.. 'something' changed, still crashing though.. but at a different
place.
Rah ! I'll probably need some rest. I've done my tests without the HTX
enabled... It's a bit embarrassing and not really responsible.
Anyway, here is a new patch, again. Willy, I hope it will be good for
the release 1.9.2.
--
Christopher Faulet
>From cacd3205bbe5c1a0bf123631178b61e1f6e9ffc1 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Mon, 14 Jan 2019 11:07:34 +0100
Subject: [PATCH] BUG/MEDIUM: stats: Get the rigth scope pointer depending on
HTX is used or not
For HTX streams, the scope pointer is relative to the URI in the start-line. But
for streams using the legacy HTTP representation, the scope pointer is relative
to the beginning of output data in the channel's buffer. So we must be carefull
to use the right one depending on the HTX is used or not.
Because the start-line is used to get de scope pointer, it is important to keep
it after the parsing of post paramters. So now, instead of removing blocks when
read in the function stats_process_http_post(), we just move on next, leaving it
in the HTX message.
Thanks to Pieter (PiBa-NL) to report this bug.
This patch must be backported to 1.9.
---
src/proto_htx.c | 4 ++--
src/stats.c | 62 ++++++++++++++++++++++++++++++++++---------------
2 files changed, 45 insertions(+), 21 deletions(-)
diff --git a/src/proto_htx.c b/src/proto_htx.c
index 236bfd04d..9fa820653 100644
--- a/src/proto_htx.c
+++ b/src/proto_htx.c
@@ -4887,8 +4887,8 @@ static int htx_handle_stats(struct stream *s, struct channel *req)
h += strlen(STAT_SCOPE_INPUT_NAME) + 1;
h2 = h;
- appctx->ctx.stats.scope_str = h2 - s->txn->uri;
- while (h <= end) {
+ appctx->ctx.stats.scope_str = h2 - HTX_SL_REQ_UPTR(sl);
+ while (h < end) {
if (*h == ';' || *h == '&' || *h == ' ')
break;
itx++;
diff --git a/src/stats.c b/src/stats.c
index ebd95d3f0..a7c12e120 100644
--- a/src/stats.c
+++ b/src/stats.c
@@ -55,6 +55,7 @@
#include <proto/fd.h>
#include <proto/freq_ctr.h>
#include <proto/frontend.h>
+#include <proto/http_htx.h>
#include <proto/log.h>
#include <proto/pattern.h>
#include <proto/pipe.h>
@@ -257,6 +258,23 @@ static int stats_putchk(struct channel *chn, struct htx *htx, struct buffer *chk
return 1;
}
+static const char *stats_scope_ptr(struct appctx *appctx, struct stream_interface *si)
+{
+ const char *p;
+
+ if (IS_HTX_STRM(si_strm(si))) {
+ struct channel *req = si_oc(si);
+ struct htx *htx = htxbuf(&req->buf);
+ struct ist uri = htx_sl_req_uri(http_find_stline(htx));
+
+ p = uri.ptr;
+ }
+ else
+ p = co_head(si_oc(si));
+
+ return p + appctx->ctx.stats.scope_str;
+}
+
/*
* http_stats_io_handler()
* -> stats_dump_stat_to_buffer() // same as above, but used for CSV or HTML
@@ -1912,8 +1930,10 @@ static void stats_dump_html_px_hdr(struct stream_interface *si, struct proxy *px
/* scope_txt = search pattern + search query, appctx->ctx.stats.scope_len is always <= STAT_SCOPE_TXT_MAXLEN */
scope_txt[0] = 0;
if (appctx->ctx.stats.scope_len) {
+ const char *scope_ptr = stats_scope_ptr(appctx, si);
+
strcpy(scope_txt, STAT_SCOPE_PATTERN);
- memcpy(scope_txt + strlen(STAT_SCOPE_PATTERN), co_head(si_oc(si)) + appctx->ctx.stats.scope_str, appctx->ctx.stats.scope_len);
+ memcpy(scope_txt + strlen(STAT_SCOPE_PATTERN), scope_ptr, appctx->ctx.stats.scope_len);
scope_txt[strlen(STAT_SCOPE_PATTERN) + appctx->ctx.stats.scope_len] = 0;
}
@@ -2075,9 +2095,12 @@ int stats_dump_proxy_to_buffer(struct stream_interface *si, struct htx *htx,
/* if the user has requested a limited output and the proxy
* name does not match, skip it.
*/
- if (appctx->ctx.stats.scope_len &&
- strnistr(px->id, strlen(px->id), co_head(si_oc(si)) + appctx->ctx.stats.scope_str, appctx->ctx.stats.scope_len) == NULL)
- return 1;
+ if (appctx->ctx.stats.scope_len) {
+ const char *scope_ptr = stats_scope_ptr(appctx, si);
+
+ if (strnistr(px->id, strlen(px->id), scope_ptr, appctx->ctx.stats.scope_len) == NULL)
+ return 1;
+ }
if ((appctx->ctx.stats.flags & STAT_BOUND) &&
(appctx->ctx.stats.iid != -1) &&
@@ -2347,6 +2370,7 @@ static void stats_dump_html_info(struct stream_interface *si, struct uri_auth *u
struct appctx *appctx = __objt_appctx(si->end);
unsigned int up = (now.tv_sec - start_date.tv_sec);
char scope_txt[STAT_SCOPE_TXT_MAXLEN + sizeof STAT_SCOPE_PATTERN];
+ const char *scope_ptr = stats_scope_ptr(appctx, si);
/* WARNING! this has to fit the first packet too.
* We are around 3.5 kB, add adding entries will
@@ -2405,7 +2429,7 @@ static void stats_dump_html_info(struct stream_interface *si, struct uri_auth *u
);
/* scope_txt = search query, appctx->ctx.stats.scope_len is always <= STAT_SCOPE_TXT_MAXLEN */
- memcpy(scope_txt, co_head(si_oc(si)) + appctx->ctx.stats.scope_str, appctx->ctx.stats.scope_len);
+ memcpy(scope_txt, scope_ptr, appctx->ctx.stats.scope_len);
scope_txt[appctx->ctx.stats.scope_len] = '\0';
chunk_appendf(&trash,
@@ -2417,7 +2441,7 @@ static void stats_dump_html_info(struct stream_interface *si, struct uri_auth *u
scope_txt[0] = 0;
if (appctx->ctx.stats.scope_len) {
strcpy(scope_txt, STAT_SCOPE_PATTERN);
- memcpy(scope_txt + strlen(STAT_SCOPE_PATTERN), co_head(si_oc(si)) + appctx->ctx.stats.scope_str, appctx->ctx.stats.scope_len);
+ memcpy(scope_txt + strlen(STAT_SCOPE_PATTERN), scope_ptr, appctx->ctx.stats.scope_len);
scope_txt[strlen(STAT_SCOPE_PATTERN) + appctx->ctx.stats.scope_len] = 0;
}
@@ -2719,7 +2743,13 @@ static int stats_process_http_post(struct stream_interface *si)
struct htx_blk *blk;
size_t count = co_data(&s->req);
- /* Remove the headers */
+ /* we need more data */
+ if (htx->extra || htx->data > count) {
+ appctx->ctx.stats.st_code = STAT_STATUS_NONE;
+ return 0;
+ }
+
+ /* Skip the headers */
blk = htx_get_head_blk(htx);
while (count && blk) {
enum htx_blk_type type = htx_get_blk_type(blk);
@@ -2731,25 +2761,18 @@ static int stats_process_http_post(struct stream_interface *si)
}
count -= sz;
- co_set_data(&s->req, co_data(&s->req) - sz);
- blk = htx_remove_blk(htx, blk);
+ blk = htx_get_next_blk(htx, blk);
if (type == HTX_BLK_EOH)
break;
}
/* too large request */
- if (htx->data + htx->extra > b_size(temp)) {
+ if (count > b_size(temp)) {
appctx->ctx.stats.st_code = STAT_STATUS_EXCD;
goto out;
}
- /* we need more data */
- if (htx->extra || htx->data > count) {
- appctx->ctx.stats.st_code = STAT_STATUS_NONE;
- return 0;
- }
-
while (count && blk) {
enum htx_blk_type type = htx_get_blk_type(blk);
uint32_t sz = htx_get_blksz(blk);
@@ -2766,8 +2789,7 @@ static int stats_process_http_post(struct stream_interface *si)
}
count -= sz;
- co_set_data(&s->req, co_data(&s->req) - sz);
- blk = htx_remove_blk(htx, blk);
+ blk = htx_get_next_blk(htx, blk);
}
}
else {
@@ -3135,8 +3157,10 @@ static int stats_send_htx_redirect(struct stream_interface *si, struct htx *htx)
/* scope_txt = search pattern + search query, appctx->ctx.stats.scope_len is always <= STAT_SCOPE_TXT_MAXLEN */
scope_txt[0] = 0;
if (appctx->ctx.stats.scope_len) {
+ const char *scope_ptr = stats_scope_ptr(appctx, si);
+
strcpy(scope_txt, STAT_SCOPE_PATTERN);
- memcpy(scope_txt + strlen(STAT_SCOPE_PATTERN), co_head(si_oc(si)) + appctx->ctx.stats.scope_str, appctx->ctx.stats.scope_len);
+ memcpy(scope_txt + strlen(STAT_SCOPE_PATTERN), scope_ptr, appctx->ctx.stats.scope_len);
scope_txt[strlen(STAT_SCOPE_PATTERN) + appctx->ctx.stats.scope_len] = 0;
}
--
2.20.1
