Le 15/09/2017 à 08:36, Илья Шипицин a écrit :
great, thank for the feedback.
there're few things like that
[src/flt_http_comp.c:926] -> [src/flt_http_comp.c:926]: (warning) Either
the condition 'txn' is redundant or there is possible null pointer
dereference: txn.
[src/flt_spoe.c:2765] -> [src/flt_spoe.c:2766]: (warning) Either the
condition 'ctx==NULL' is redundant or there is possible null pointer
dereference: ctx.
[src/haproxy.c:568]: (error) Common realloc mistake: 'next_argv' nulled
but not freed upon failure
[src/server.c:3993] -> [src/server.c:3995]: (warning) Either the
condition 'nameserver' is redundant or there is possible null pointer
dereference: nameserver.
[src/ssl_sock.c:4403] -> [src/ssl_sock.c:4397]: (warning) Either the
condition '!bind_conf' is redundant or there is possible null pointer
dereference: bind_conf.
Hi,
You're right, there are bugs there. The worst is on the compression
filter. I attached patches to fix them.
Willy, could you merge it please ? Some of them must be backported in 1.7.
Thanks,
--
Christopher Faulet
>From 362bf07d61b06469bff839886d52db24daa2aa5e Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Fri, 15 Sep 2017 10:14:43 +0200
Subject: [PATCH 1/5] BUG/MEDIUM: compression: Fix check on txn in
smp_fetch_res_comp_algo
The check was totally messed up. In the worse case, it led to a crash, when
res.comp_algo sample fetch was retrieved on uncompressed response (with the
compression enabled).
This patch must be backported in 1.7.
---
src/flt_http_comp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c
index 64c669d54..4d5332832 100644
--- a/src/flt_http_comp.c
+++ b/src/flt_http_comp.c
@@ -923,7 +923,7 @@ smp_fetch_res_comp_algo(const struct arg *args, struct sample *smp,
struct filter *filter;
struct comp_state *st;
- if (!(txn || !(txn->rsp.flags & HTTP_MSGF_COMPRESSING)))
+ if (!txn || !(txn->rsp.flags & HTTP_MSGF_COMPRESSING))
return 0;
list_for_each_entry(filter, &strm_flt(smp->strm)->filters, list) {
--
2.13.5
>From 30bb79ce1a47666d5d9cd20636e97b7589e34dd7 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Fri, 15 Sep 2017 11:39:36 +0200
Subject: [PATCH 2/5] BUG/MINOR: compression: Check response headers before
http-response rules eval
This is required if we want to use res.comp or res.comp_algo sample fetches in
http-response rules.
This patch must be backported in 1.7.
---
src/flt_http_comp.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c
index 4d5332832..0ed252895 100644
--- a/src/flt_http_comp.c
+++ b/src/flt_http_comp.c
@@ -103,6 +103,12 @@ comp_start_analyze(struct stream *s, struct filter *filter, struct channel *chn)
st->initialized = 0;
st->finished = 0;
filter->ctx = st;
+
+ /* Register post-analyzer on AN_RES_WAIT_HTTP because we need to
+ * analyze response headers before http-response rules execution
+ * to be sure we can use res.comp and res.comp_algo sample
+ * fetches */
+ filter->post_analyzers |= AN_RES_WAIT_HTTP;
}
return 1;
}
@@ -135,7 +141,8 @@ comp_http_headers(struct stream *s, struct filter *filter, struct http_msg *msg)
if (!(msg->chn->flags & CF_ISRESP))
select_compression_request_header(st, s, msg);
else {
- select_compression_response_header(st, s, msg);
+ /* Response headers have already been checked in
+ * comp_http_post_analyze callback. */
if (st->comp_algo) {
register_data_filter(s, msg->chn, filter);
st->hdrs_len = s->txn->rsp.sov;
@@ -147,6 +154,26 @@ comp_http_headers(struct stream *s, struct filter *filter, struct http_msg *msg)
}
static int
+comp_http_post_analyze(struct stream *s, struct filter *filter,
+ struct channel *chn, unsigned an_bit)
+{
+ struct http_txn *txn = s->txn;
+ struct http_msg *msg = &txn->rsp;
+ struct comp_state *st = filter->ctx;
+
+ if (an_bit != AN_RES_WAIT_HTTP)
+ goto end;
+
+ if (!strm_fe(s)->comp && !s->be->comp)
+ goto end;
+
+ select_compression_response_header(st, s, msg);
+
+ end:
+ return 1;
+}
+
+static int
comp_http_data(struct stream *s, struct filter *filter, struct http_msg *msg)
{
struct comp_state *st = filter->ctx;
@@ -768,6 +795,7 @@ struct flt_ops comp_ops = {
.channel_start_analyze = comp_start_analyze,
.channel_end_analyze = comp_end_analyze,
+ .channel_post_analyze = comp_http_post_analyze,
.http_headers = comp_http_headers,
.http_data = comp_http_data,
--
2.13.5
>From eef0b960798c3db93923f2c920f27bab6e53286e Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Fri, 15 Sep 2017 11:51:18 +0200
Subject: [PATCH 3/5] BUG/MINOR: spoe: Don't rely on SPOE ctx in debug message
when its creation failed
If the SPOE context creation failed, we must not try to use it in the debug
message used to notice the error.
This patch must be backported in 1.7.
---
src/flt_spoe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/flt_spoe.c b/src/flt_spoe.c
index 2d62e5a8c..08eb0b813 100644
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -2762,7 +2762,7 @@ spoe_start(struct stream *s, struct filter *filter)
SPOE_PRINTF(stderr, "%d.%06d [SPOE/%-15s] %s: stream=%p"
" - failed to create SPOE context\n",
(int)now.tv_sec, (int)now.tv_usec, agent->id,
- __FUNCTION__, ctx->strm);
+ __FUNCTION__, s);
send_log(s->be, LOG_EMERG,
"SPOE: [%s] failed to create SPOE context\n",
agent->id);
--
2.13.5
>From 1f63ad9ed0e00115fa59b5e61cbefc357da202fc Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Fri, 15 Sep 2017 11:55:45 +0200
Subject: [PATCH 4/5] BUG/MINOR: dns: Fix check on nameserver in
snr_resolution_cb
snr_resolution_cb can be called with <nameserver> parameter set to NULL. So we
must check it before using it. This is done most of time, except when we deal
with invalid DNS response.
---
src/server.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/server.c b/src/server.c
index ec2dbe894..ebfe0e5c0 100644
--- a/src/server.c
+++ b/src/server.c
@@ -3990,10 +3990,11 @@ int snr_resolution_cb(struct dns_requester *requester, struct dns_nameserver *na
return 1;
invalid:
- if (nameserver)
+ if (nameserver) {
nameserver->counters.invalid += 1;
- if (resolution->nb_responses >= nameserver->resolvers->count_nameservers)
- goto update_status;
+ if (resolution->nb_responses >= nameserver->resolvers->count_nameservers)
+ goto update_status;
+ }
snr_update_srv_status(s, has_no_ip);
return 0;
--
2.13.5
>From 44921b3c1966e9078249f2083ef1789ac0923802 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Fri, 15 Sep 2017 09:52:49 +0200
Subject: [PATCH 5/5] MINOR: ssl: Remove useless checks on bind_conf or
bind_conf->is_ssl
bind_conf always exists at these steps and it is always for SSL listeners.
---
src/ssl_sock.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 8a5d66fd2..989d7e1cf 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4226,9 +4226,6 @@ int ssl_sock_prepare_all_ctx(struct bind_conf *bind_conf)
struct sni_ctx *sni;
int err = 0;
- if (!bind_conf || !bind_conf->is_ssl)
- return 0;
-
/* Automatic memory computations need to know we use SSL there */
global.ssl_used_frontend = 1;
@@ -4333,9 +4330,6 @@ void ssl_sock_free_all_ctx(struct bind_conf *bind_conf)
struct ebmb_node *node, *back;
struct sni_ctx *sni;
- if (!bind_conf || !bind_conf->is_ssl)
- return;
-
node = ebmb_first(&bind_conf->sni_ctx);
while (node) {
sni = ebmb_entry(node, struct sni_ctx, name);
@@ -4400,7 +4394,7 @@ ssl_sock_load_ca(struct bind_conf *bind_conf)
EVP_PKEY *capkey = NULL;
int err = 0;
- if (!bind_conf || !bind_conf->generate_certs)
+ if (!bind_conf->generate_certs)
return err;
#if (defined SSL_CTRL_SET_TLSEXT_HOSTNAME && !defined SSL_NO_GENERATE_CERTIFICATES)
@@ -4453,9 +4447,6 @@ ssl_sock_load_ca(struct bind_conf *bind_conf)
void
ssl_sock_free_ca(struct bind_conf *bind_conf)
{
- if (!bind_conf)
- return;
-
if (bind_conf->ca_sign_pkey)
EVP_PKEY_free(bind_conf->ca_sign_pkey);
if (bind_conf->ca_sign_cert)
--
2.13.5
