[PATCH] BUG/MEDIUM: sample: Fix memory leak in sample_conv_jwt_member_query

2021-12-01 Thread Tim Duesterhus
The function leaked one full buffer per invocation. Fix this by simply removing
the call to alloc_trash_chunk(), the static chunk from get_trash_chunk() is
sufficient.

This bug was introduced in 0a72f5ee7c2a61bdb379436461269315c776b50a, which is
2.5-dev10. This fix needs to be backported to 2.5+.
---
 src/sample.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/sample.c b/src/sample.c
index 5abf4712a..63816be5d 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -3584,10 +3584,6 @@ static int sample_conv_jwt_member_query(const struct arg 
*args, struct sample *s
if (item_num < member + 1)
goto end;
 
-   decoded_header = alloc_trash_chunk();
-   if (!decoded_header)
-   goto end;
-
ret = base64urldec(items[member].start, items[member].length,
   decoded_header->area, decoded_header->size);
if (ret == -1)
-- 
2.34.1




[PATCH] CLEANUP: Wrap `accept4_broken = 1` into additional parenthesis

2021-11-20 Thread Tim Duesterhus
This makes it clear to static analysis tools that this assignment is
intentional and not a mistyped comparison.
---
 src/sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/sock.c b/src/sock.c
index e3d4a6e4c..f11c5b0c4 100644
--- a/src/sock.c
+++ b/src/sock.c
@@ -74,7 +74,7 @@ struct connection *sock_accept_conn(struct listener *l, int 
*status)
(((cfd = accept4(l->rx.fd, (struct sockaddr*)addr, ,
 SOCK_NONBLOCK | (master ? SOCK_CLOEXEC : 0))) == 
-1) &&
 (errno == ENOSYS || errno == EINVAL || errno == EBADF) &&
-(accept4_broken = 1)))
+((accept4_broken = 1
 #endif
{
laddr = sizeof(*conn->src);
-- 
2.34.0




[PATCH 5/6] CLEANUP: Apply ist.cocci

2021-11-08 Thread Tim Duesterhus
This is to make use of `chunk_istcat()`.
---
 src/cache.c  |  2 +-
 src/http_fetch.c |  2 +-
 src/http_htx.c   |  4 ++--
 src/mux_fcgi.c   | 10 +-
 src/tcpcheck.c   |  4 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index e871a7b30..ee42947c1 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -1644,7 +1644,7 @@ int sha1_hosturi(struct stream *s)
chunk_istcat(trash, ctx.value);
}
 
-   chunk_memcat(trash, uri.ptr, uri.len);
+   chunk_istcat(trash, uri);
 
/* hash everything */
blk_SHA1_Init(_ctx);
diff --git a/src/http_fetch.c b/src/http_fetch.c
index aa4965d0f..99dc89a51 100644
--- a/src/http_fetch.c
+++ b/src/http_fetch.c
@@ -874,7 +874,7 @@ static int smp_fetch_hdr_names(const struct arg *args, 
struct sample *smp, const
 
if (temp->data)
temp->area[temp->data++] = del;
-   chunk_memcat(temp, n.ptr, n.len);
+   chunk_istcat(temp, n);
}
 
smp->data.type = SMP_T_STR;
diff --git a/src/http_htx.c b/src/http_htx.c
index 6b06336e6..3535fa713 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -432,7 +432,7 @@ int http_replace_req_path(struct htx *htx, const struct ist 
path, int with_qs)
vsn = ist2(temp->area + meth.len, HTX_SL_REQ_VLEN(sl));
 
chunk_memcat(temp, uri.ptr, p.ptr - uri.ptr); /* uri: host part 
*/
-   chunk_memcat(temp, path.ptr, path.len);   /* uri: new path 
*/
+   chunk_istcat(temp, path); /* uri: new path 
*/
chunk_memcat(temp, p.ptr + plen, p.len - plen);   /* uri: QS part */
uri = ist2(temp->area + meth.len + vsn.len, uri.len - plen + path.len);
 
@@ -711,7 +711,7 @@ int http_update_authority(struct htx *htx, struct htx_sl 
*sl, const struct ist h
vsn = ist2(temp->area + meth.len, HTX_SL_REQ_VLEN(sl));
 
chunk_memcat(temp, uri.ptr, authority.ptr - uri.ptr);
-   chunk_memcat(temp, host.ptr, host.len);
+   chunk_istcat(temp, host);
chunk_memcat(temp, istend(authority), istend(uri) - istend(authority));
uri = ist2(temp->area + meth.len + vsn.len, host.len + uri.len - 
authority.len); /* uri */
 
diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index f20b46b71..ba3a54617 100644
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -1242,17 +1242,17 @@ static int fcgi_set_default_param(struct fcgi_conn 
*fconn, struct fcgi_strm *fst
if (!(params->mask & FCGI_SP_REQ_METH)) {
p  = htx_sl_req_meth(sl);
params->meth = ist2(b_tail(params->p), p.len);
-   chunk_memcat(params->p, p.ptr, p.len);
+   chunk_istcat(params->p, p);
}
if (!(params->mask & FCGI_SP_REQ_URI)) {
p = h1_get_uri(sl);
params->uri = ist2(b_tail(params->p), p.len);
-   chunk_memcat(params->p, p.ptr, p.len);
+   chunk_istcat(params->p, p);
}
if (!(params->mask & FCGI_SP_SRV_PROTO)) {
p  = htx_sl_req_vsn(sl);
params->vsn = ist2(b_tail(params->p), p.len);
-   chunk_memcat(params->p, p.ptr, p.len);
+   chunk_istcat(params->p, p);
}
if (!(params->mask & FCGI_SP_SRV_PORT)) {
char *end;
@@ -1361,7 +1361,7 @@ static int fcgi_set_default_param(struct fcgi_conn 
*fconn, struct fcgi_strm *fst
/* Decode the path. it must first be copied to keep the URI
 * untouched.
 */
-   chunk_memcat(params->p, path.ptr, path.len);
+   chunk_istcat(params->p, path);
path.ptr = b_tail(params->p) - path.len;
len = url_decode(ist0(path), 0);
if (len < 0)
@@ -1415,7 +1415,7 @@ static int fcgi_set_default_param(struct fcgi_conn 
*fconn, struct fcgi_strm *fst
struct ist sn = params->scriptname;
 
params->scriptname = ist2(b_tail(params->p), 
len+fconn->app->index.len);
-   chunk_memcat(params->p, sn.ptr, sn.len);
+   chunk_istcat(params->p, sn);
chunk_memcat(params->p, fconn->app->index.ptr, 
fconn->app->index.len);
}
}
diff --git a/src/tcpcheck.c b/src/tcpcheck.c
index bf497fec5..294e49bcc 100644
--- a/src/tcpcheck.c
+++ b/src/tcpcheck.c
@@ -429,7 +429,7 @@ static void tcpcheck_expect_onerror_message(struct buffer 
*msg, struct check *ch
 * 4. Otherwise produce the generic tcp-check info message
 */
if (istlen(info)) {
-   chunk_strncat(msg, istptr(info), istlen(info));
+   chunk_istcat(msg, info);
goto comment;
}
else if (!LIST_ISEMPTY(>expect.onerror_fmt)) {
@@ -517,7 +517,7 @@ static void tcpcheck_expect_onsuccess_message(struct buffer 
*msg, struct check *
 * 4. Otherwise produce 

[PATCH 6/6] CLEANUP: chunk: Remove duplicated chunk_Xcat implementation

2021-11-08 Thread Tim Duesterhus
Delegate chunk_istcat, chunk_cat and chunk_strncat to the most generic
chunk_memcat.
---
 include/haproxy/chunk.h | 41 +
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/include/haproxy/chunk.h b/include/haproxy/chunk.h
index af9ef816b..05fd16121 100644
--- a/include/haproxy/chunk.h
+++ b/include/haproxy/chunk.h
@@ -107,28 +107,6 @@ static inline int chunk_cpy(struct buffer *chk, const 
struct buffer *src)
return 1;
 }
 
-/* appends chunk  after . Returns 0 in case of failure. */
-static inline int chunk_cat(struct buffer *chk, const struct buffer *src)
-{
-   if (unlikely(chk->data + src->data > chk->size))
-   return 0;
-
-   memcpy(chk->area + chk->data, src->area, src->data);
-   chk->data += src->data;
-   return 1;
-}
-
-/* appends ist  after . Returns 0 in case of failure. */
-static inline int chunk_istcat(struct buffer *chk, const struct ist src)
-{
-   if (unlikely(chk->data + src.len > chk->size))
-   return 0;
-
-   memcpy(chk->area + chk->data, src.ptr, src.len);
-   chk->data += src.len;
-   return 1;
-}
-
 /* copies memory area  into  for  bytes. Returns 0 in
  * case of failure. No trailing zero is added.
  */
@@ -158,6 +136,18 @@ static inline int chunk_memcat(struct buffer *chk, const 
char *src,
return 1;
 }
 
+/* appends ist  after . Returns 0 in case of failure. */
+static inline int chunk_istcat(struct buffer *chk, const struct ist src)
+{
+   return chunk_memcat(chk, istptr(src), istlen(src));
+}
+
+/* appends chunk  after . Returns 0 in case of failure. */
+static inline int chunk_cat(struct buffer *chk, const struct buffer *src)
+{
+   return chunk_memcat(chk, src->area, src->data);
+}
+
 /* copies str into  followed by a trailing zero. Returns 0 in
  * case of failure.
  */
@@ -218,12 +208,7 @@ static inline int chunk_strcat(struct buffer *chk, const 
char *str)
  */
 static inline int chunk_strncat(struct buffer *chk, const char *str, int nb)
 {
-   if (unlikely(chk->data + nb >= chk->size))
-   return 0;
-
-   memcpy(chk->area + chk->data, str, nb);
-   chk->data += nb;
-   return 1;
+   return chunk_memcat(chk, str, nb);
 }
 
 /* Adds a trailing zero to the current chunk and returns the pointer to the
-- 
2.33.1




[PATCH 0/6] Probably final Coccinelle Cleanup

2021-11-08 Thread Tim Duesterhus
Hi Willy,

find my (probably :-) ) final CLEANUP series for 2.5.

Regarding the final patch:

'chunk_strncat()' appears to be completely redundant, it simply passes through
the arguments and even takes an int instead of a size_t. Should it be removed?

Best regards
Tim Düsterhus

Tim Duesterhus (6):
  DEV: coccinelle: Add rule to use `isttrim()` where possible
  CLEANUP: Apply ist.cocci
  DEV: coccinelle: Add rule to use `chunk_istcat()` instead of
`chunk_memcat()`
  DEV: coccinelle: Add rule to use `chunk_istcat()` instead of
`chunk_strncat()`
  CLEANUP: Apply ist.cocci
  CLEANUP: chunk: Remove duplicated chunk_Xcat implementation

 dev/coccinelle/ist.cocci | 24 +++
 include/haproxy/chunk.h  | 41 +---
 src/cache.c  |  5 ++---
 src/flt_trace.c  |  3 +--
 src/hlua.c   |  6 ++
 src/http_ana.c   |  3 +--
 src/http_fetch.c |  2 +-
 src/http_htx.c   |  4 ++--
 src/log.c|  6 ++
 src/mux_fcgi.c   | 10 +-
 src/tcpcheck.c   |  4 ++--
 11 files changed, 55 insertions(+), 53 deletions(-)

-- 
2.33.1




[PATCH 1/6] DEV: coccinelle: Add rule to use `isttrim()` where possible

2021-11-08 Thread Tim Duesterhus
This replaces `if (i.len > e) i.len = e;` by `isttrim(i, e)`.
---
 dev/coccinelle/ist.cocci | 8 
 1 file changed, 8 insertions(+)

diff --git a/dev/coccinelle/ist.cocci b/dev/coccinelle/ist.cocci
index 5b6aa6b2c..7e9a6ac05 100644
--- a/dev/coccinelle/ist.cocci
+++ b/dev/coccinelle/ist.cocci
@@ -44,6 +44,14 @@ struct ist i;
 - (\(i.ptr\|istptr(i)\) + \(i.len\|istlen(i)\))
 + istend(i)
 
+@@
+struct ist i;
+expression e;
+@@
+
+- if (\(i.len\|istlen(i)\) > e) { i.len = e; }
++ i = isttrim(i, e);
+
 @@
 struct ist i;
 @@
-- 
2.33.1




[PATCH 3/6] DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_memcat()`

2021-11-08 Thread Tim Duesterhus
This replaces `chunk_memcat()` with `chunk_istcat()` if the parameters are the
ist's `.ptr` and `.len`.
---
 dev/coccinelle/ist.cocci | 8 
 1 file changed, 8 insertions(+)

diff --git a/dev/coccinelle/ist.cocci b/dev/coccinelle/ist.cocci
index 7e9a6ac05..4945141b2 100644
--- a/dev/coccinelle/ist.cocci
+++ b/dev/coccinelle/ist.cocci
@@ -52,6 +52,14 @@ expression e;
 - if (\(i.len\|istlen(i)\) > e) { i.len = e; }
 + i = isttrim(i, e);
 
+@@
+struct ist i;
+struct buffer *b;
+@@
+
+- chunk_memcat(b, \(i.ptr\|istptr(i)\) , \(i.len\|istlen(i)\));
++ chunk_istcat(b, i);
+
 @@
 struct ist i;
 @@
-- 
2.33.1




[PATCH 2/6] CLEANUP: Apply ist.cocci

2021-11-08 Thread Tim Duesterhus
Make use of the new rules to use `isttrim()`.
---
 src/cache.c | 3 +--
 src/flt_trace.c | 3 +--
 src/hlua.c  | 6 ++
 src/http_ana.c  | 3 +--
 src/log.c   | 6 ++
 5 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index ba2b63c49..e871a7b30 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -622,8 +622,7 @@ cache_store_http_payload(struct stream *s, struct filter 
*filter, struct http_ms
case HTX_BLK_DATA:
v = htx_get_blk_value(htx, blk);
v = istadv(v, offset);
-   if (v.len > len)
-   v.len = len;
+   v = isttrim(v, len);
 
info = (type << 28) + v.len;
chunk_memcat(, (char *), 
sizeof(info));
diff --git a/src/flt_trace.c b/src/flt_trace.c
index b3efea6f9..5aabcb2b0 100644
--- a/src/flt_trace.c
+++ b/src/flt_trace.c
@@ -146,8 +146,7 @@ trace_htx_hexdump(struct htx *htx, unsigned int offset, 
unsigned int len)
v = istadv(v, offset);
offset = 0;
 
-   if (v.len > len)
-   v.len = len;
+   v = isttrim(v, len);
len -= v.len;
if (type == HTX_BLK_DATA)
trace_hexdump(v);
diff --git a/src/hlua.c b/src/hlua.c
index c2e56b3b9..94f656234 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -6329,8 +6329,7 @@ static int _hlua_http_msg_dup(struct http_msg *msg, 
lua_State *L, size_t offset,
case HTX_BLK_DATA:
v = htx_get_blk_value(htx, blk);
v = istadv(v, offset);
-   if (v.len > len)
-   v.len = len;
+   v = isttrim(v, len);
 
luaL_addlstring(, v.ptr, v.len);
ret += v.len;
@@ -6431,8 +6430,7 @@ static void _hlua_http_msg_delete(struct http_msg *msg, 
struct filter *filter, s
goto end;
v = htx_get_blk_value(htx, blk);
v.ptr += htxret.ret;
-   if (v.len > len)
-   v.len  = len;
+   v = isttrim(v, len);
blk = htx_replace_blk_value(htx, blk, v, IST_NULL);
len -= v.len;
ret += v.len;
diff --git a/src/http_ana.c b/src/http_ana.c
index 9d11284a5..c037261cf 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -4912,8 +4912,7 @@ static void http_capture_headers(struct htx *htx, char 
**cap, struct cap_hdr *ca
}
 
v = htx_get_blk_value(htx, blk);
-   if (v.len > h->len)
-   v.len = h->len;
+   v = isttrim(v, h->len);
 
memcpy(cap[h->index], v.ptr, v.len);
cap[h->index][v.len]=0;
diff --git a/src/log.c b/src/log.c
index 81bf97b34..e7607c2c4 100644
--- a/src/log.c
+++ b/src/log.c
@@ -1665,8 +1665,7 @@ static inline void __do_send_log(struct logsrv *logsrv, 
int nblogger, int level,
struct ist msg;
 
msg = ist2(message, size);
-   if (msg.len > logsrv->maxlen)
-   msg.len = logsrv->maxlen;
+   msg = isttrim(msg, logsrv->maxlen);
 
sent = sink_write(logsrv->sink, , 1, level, 
logsrv->facility, metadata);
}
@@ -1674,8 +1673,7 @@ static inline void __do_send_log(struct logsrv *logsrv, 
int nblogger, int level,
struct ist msg;
 
msg = ist2(message, size);
-   if (msg.len > logsrv->maxlen)
-   msg.len = logsrv->maxlen;
+   msg = isttrim(msg, logsrv->maxlen);
 
sent = fd_write_frag_line(*plogfd, logsrv->maxlen, msg_header, 
nbelem, , 1, 1);
}
-- 
2.33.1




[PATCH 4/6] DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_strncat()`

2021-11-08 Thread Tim Duesterhus
This replaces `chunk_strncat()` with `chunk_istcat()` if the parameters are the
ist's `.ptr` and `.len`.
---
 dev/coccinelle/ist.cocci | 8 
 1 file changed, 8 insertions(+)

diff --git a/dev/coccinelle/ist.cocci b/dev/coccinelle/ist.cocci
index 4945141b2..680afbade 100644
--- a/dev/coccinelle/ist.cocci
+++ b/dev/coccinelle/ist.cocci
@@ -60,6 +60,14 @@ struct buffer *b;
 - chunk_memcat(b, \(i.ptr\|istptr(i)\) , \(i.len\|istlen(i)\));
 + chunk_istcat(b, i);
 
+@@
+struct ist i;
+struct buffer *b;
+@@
+
+- chunk_strncat(b, \(i.ptr\|istptr(i)\) , \(i.len\|istlen(i)\));
++ chunk_istcat(b, i);
+
 @@
 struct ist i;
 @@
-- 
2.33.1




[PATCH 4/4] CLEANUP: Re-apply xalloc_size.cocci

2021-11-06 Thread Tim Duesterhus
Use a consistent size as the parameter for the *alloc family.
---
 src/ev_evports.c | 2 +-
 src/hlua.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ev_evports.c b/src/ev_evports.c
index 710d51236..73e97517c 100644
--- a/src/ev_evports.c
+++ b/src/ev_evports.c
@@ -293,7 +293,7 @@ static int init_evports_per_thread()
int fd;
 
evports_evlist_max = global.tune.maxpollevents;
-   evports_evlist = calloc(evports_evlist_max, sizeof (port_event_t));
+   evports_evlist = calloc(evports_evlist_max, sizeof(*evports_evlist));
if (evports_evlist == NULL) {
goto fail_alloc;
}
diff --git a/src/hlua.c b/src/hlua.c
index 086a8e0be..c2e56b3b9 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -7089,7 +7089,7 @@ struct http_hdr *hlua_httpclient_table_to_hdrs(lua_State 
*L)
 
if (hdr_num) {
/* alloc and copy the headers in the httpclient struct */
-   result = calloc((hdr_num + 1), sizeof(*hdrs));
+   result = calloc((hdr_num + 1), sizeof(*result));
if (!result)
goto skip_headers;
memcpy(result, hdrs, sizeof(struct http_hdr) * (hdr_num + 1));
-- 
2.33.1




[PATCH 1/4] DEV: coccinelle: Remove unused `expression e`

2021-11-06 Thread Tim Duesterhus
Introduced in ef00c533e1ed37b414aab912f492be794ab589cc.
---
 dev/coccinelle/ist.cocci | 1 -
 1 file changed, 1 deletion(-)

diff --git a/dev/coccinelle/ist.cocci b/dev/coccinelle/ist.cocci
index 97ce0a2ad..598ffa3e2 100644
--- a/dev/coccinelle/ist.cocci
+++ b/dev/coccinelle/ist.cocci
@@ -31,7 +31,6 @@ struct ist i;
 
 @@
 struct ist i;
-expression e;
 @@
 
 - i.ptr++;
-- 
2.33.1




[PATCH 3/4] CLEANUP: Apply ist.cocci

2021-11-06 Thread Tim Duesterhus
Make use of the new rules to use `istend()`.
---
 src/h1.c   |  4 ++--
 src/h2.c   |  2 +-
 src/hlua.c |  2 +-
 src/http_htx.c | 11 ++-
 src/htx.c  | 11 +++
 src/tcpcheck.c |  3 ++-
 6 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/src/h1.c b/src/h1.c
index e0ba8d768..99b9c2993 100644
--- a/src/h1.c
+++ b/src/h1.c
@@ -110,7 +110,7 @@ int h1_parse_xfer_enc_header(struct h1m *h1m, struct ist 
value)
h1m->flags |= H1_MF_XFER_ENC;
 
word.ptr = value.ptr - 1; // -1 for next loop's pre-increment
-   e = value.ptr + value.len;
+   e = istend(value);
 
while (++word.ptr < e) {
/* skip leading delimiter and blanks */
@@ -229,7 +229,7 @@ void h1_parse_upgrade_header(struct h1m *h1m, struct ist 
value)
h1m->flags &= ~H1_MF_UPG_WEBSOCKET;
 
word.ptr = value.ptr - 1; // -1 for next loop's pre-increment
-   e = value.ptr + value.len;
+   e = istend(value);
 
while (++word.ptr < e) {
/* skip leading delimiter and blanks */
diff --git a/src/h2.c b/src/h2.c
index dd1f7d9b6..49a1252e9 100644
--- a/src/h2.c
+++ b/src/h2.c
@@ -62,7 +62,7 @@ static int has_forbidden_char(const struct ist ist, const 
char *start)
(1U << (uint8_t)*start) & ((1<<13) | (1<<10) | (1<<0)))
return 1;
start++;
-   } while (start < ist.ptr + ist.len);
+   } while (start < istend(ist));
return 0;
 }
 
diff --git a/src/hlua.c b/src/hlua.c
index e9d4391f7..086a8e0be 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -4750,7 +4750,7 @@ static int hlua_applet_http_new(lua_State *L, struct 
appctx *ctx)
char *p, *q, *end;
 
p = path.ptr;
-   end = path.ptr + path.len;
+   end = istend(path);
q = p;
while (q < end && *q != '?')
q++;
diff --git a/src/http_htx.c b/src/http_htx.c
index 8028cfc99..d93cc3797 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -195,7 +195,8 @@ static int __http_find_header(const struct htx *htx, const 
void *pattern, struct
if (istlen(n) < istlen(name))
goto next_blk;
 
-   n = ist2(istptr(n) + istlen(n) - istlen(name), 
istlen(name));
+   n = ist2(istend(n) - istlen(name),
+istlen(name));
if (!isteqi(n, name))
goto next_blk;
break;
@@ -219,8 +220,8 @@ static int __http_find_header(const struct htx *htx, const 
void *pattern, struct
ctx->lws_before++;
}
if (!(flags & HTTP_FIND_FL_FULL))
-   v.len = http_find_hdr_value_end(v.ptr, v.ptr + v.len) - 
v.ptr;
-   while (v.len && HTTP_IS_LWS(*(v.ptr + v.len - 1))) {
+   v.len = http_find_hdr_value_end(v.ptr, istend(v)) - 
v.ptr;
+   while (v.len && HTTP_IS_LWS(*(istend(v) - 1))) {
v.len--;
ctx->lws_after++;
}
@@ -710,7 +711,7 @@ int http_update_authority(struct htx *htx, struct htx_sl 
*sl, const struct ist h
 
chunk_memcat(temp, uri.ptr, authority.ptr - uri.ptr);
chunk_memcat(temp, host.ptr, host.len);
-   chunk_memcat(temp, authority.ptr + authority.len, uri.ptr + uri.len - 
(authority.ptr + authority.len));
+   chunk_memcat(temp, istend(authority), istend(uri) - istend(authority));
uri = ist2(temp->area + meth.len + vsn.len, host.len + uri.len - 
authority.len); /* uri */
 
return http_replace_stline(htx, meth, uri, vsn);
@@ -917,7 +918,7 @@ int http_str_to_htx(struct buffer *buf, struct ist raw, 
char **errmsg)
 
h1m_init_res();
h1m.flags |= H1_MF_NO_PHDR;
-   ret = h1_headers_to_hdr_list(raw.ptr, raw.ptr + raw.len,
+   ret = h1_headers_to_hdr_list(raw.ptr, istend(raw),
 hdrs, sizeof(hdrs)/sizeof(hdrs[0]), , 
);
if (ret <= 0) {
memprintf(errmsg, "unabled to parse headers (error offset: 
%d)", h1m.err_pos);
diff --git a/src/htx.c b/src/htx.c
index 8c6e368e7..940989c50 100644
--- a/src/htx.c
+++ b/src/htx.c
@@ -601,11 +601,13 @@ struct htx_blk *htx_replace_blk_value(struct htx *htx, 
struct htx_blk *blk,
if (delta <= 0) {
/* compression: copy new data first then move the end */
memcpy(old.ptr, new.ptr, new.len);
-   memmove(old.ptr + new.len, old.ptr + old.len, (v.ptr + 
v.len) - (old.ptr + old.len));
+   memmove(old.ptr + new.len, istend(old),
+   istend(v) - istend(old));
}
else {
/* expansion: move the end 

[PATCH 2/4] DEV: coccinelle: Add rule to use `istend()` where possible

2021-11-06 Thread Tim Duesterhus
This replaces `i.ptr + i.len` by `istend()`.
---
 dev/coccinelle/ist.cocci | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/dev/coccinelle/ist.cocci b/dev/coccinelle/ist.cocci
index 598ffa3e2..5b6aa6b2c 100644
--- a/dev/coccinelle/ist.cocci
+++ b/dev/coccinelle/ist.cocci
@@ -41,6 +41,13 @@ struct ist i;
 struct ist i;
 @@
 
+- (\(i.ptr\|istptr(i)\) + \(i.len\|istlen(i)\))
++ istend(i)
+
+@@
+struct ist i;
+@@
+
 - i.ptr != NULL
 + isttest(i)
 
-- 
2.33.1




[PATCH 2/2] CLEANUP: Apply ist.cocci

2021-11-04 Thread Tim Duesterhus
Make use of the new rules to use `istnext()`.
---
 src/cache.c| 24 
 src/http_htx.c | 12 
 src/mqtt.c |  2 +-
 3 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index feab63f07..ba2b63c49 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -2180,49 +2180,49 @@ static int parse_encoding_value(struct ist encoding, 
unsigned int *encoding_valu
 
switch (*encoding.ptr) {
case 'a':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "aes128gcm", 
VARY_ENCODING_AES128GCM);
break;
case 'b':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "br", 
VARY_ENCODING_BR);
break;
case 'c':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "compress", 
VARY_ENCODING_COMPRESS);
break;
case 'd':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "deflate", 
VARY_ENCODING_DEFLATE);
break;
case 'e':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "exi", 
VARY_ENCODING_EXI);
break;
case 'g':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "gzip", 
VARY_ENCODING_GZIP);
break;
case 'i':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "identity", 
VARY_ENCODING_IDENTITY);
break;
case 'p':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "pack200-gzip", 
VARY_ENCODING_PACK200_GZIP);
break;
case 'x':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "x-gzip", 
VARY_ENCODING_GZIP);
if (!*encoding_value)
*encoding_value = CHECK_ENCODING(encoding, 
"x-compress", VARY_ENCODING_COMPRESS);
break;
case 'z':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = CHECK_ENCODING(encoding, "zstd", 
VARY_ENCODING_ZSTD);
break;
case '*':
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
*encoding_value = VARY_ENCODING_STAR;
break;
default:
@@ -2238,7 +2238,7 @@ static int parse_encoding_value(struct ist encoding, 
unsigned int *encoding_valu
return -1;
 
if (has_null_weight) {
-   encoding = istadv(encoding, 1);
+   encoding = istnext(encoding);
 
encoding = http_trim_leading_spht(encoding);
 
diff --git a/src/http_htx.c b/src/http_htx.c
index bfdcaef86..8028cfc99 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -146,8 +146,7 @@ static int __http_find_header(const struct htx *htx, const 
void *pattern, struct
goto next_blk;
/* Skip comma */
if (*(v.ptr) == ',') {
-   v.ptr++;
-   v.len--;
+   v = istnext(v);
}
 
goto return_hdr;
@@ -216,8 +215,7 @@ static int __http_find_header(const struct htx *htx, const 
void *pattern, struct
ctx->lws_before = 0;
ctx->lws_after = 0;
while (v.len && HTTP_IS_LWS(*v.ptr)) {
-   v.ptr++;
-   v.len--;
+   v = istnext(v);
ctx->lws_before++;
}
if (!(flags & HTTP_FIND_FL_FULL))
@@ -457,16 +455,14 @@ int http_replace_req_query(struct htx *htx, const struct 
ist query)
uri = htx_sl_req_uri(sl);
q = uri;
while (q.len > 0 && *(q.ptr) != '?') {
-   q.ptr++;
-   q.len--;
+   q = istnext(q);
}
 
/* skip the question mark or indicate that we must insert it
 * (but only if the format string is not empty then).
 */
if (q.len) {
-   q.ptr++;
-   q.len--;
+   q = istnext(q);
}
else if 

[PATCH 1/2] DEV: coccinelle: Add rule to use `istnext()` where possible

2021-11-04 Thread Tim Duesterhus
This matches both `istadv(..., 1)` as well as raw `.ptr++` uses.
---
 dev/coccinelle/ist.cocci | 16 
 1 file changed, 16 insertions(+)

diff --git a/dev/coccinelle/ist.cocci b/dev/coccinelle/ist.cocci
index c3243302f..97ce0a2ad 100644
--- a/dev/coccinelle/ist.cocci
+++ b/dev/coccinelle/ist.cocci
@@ -26,6 +26,22 @@ expression e;
 struct ist i;
 @@
 
+- i = istadv(i, 1);
++ i = istnext(i);
+
+@@
+struct ist i;
+expression e;
+@@
+
+- i.ptr++;
+- i.len--;
++ i = istnext(i);
+
+@@
+struct ist i;
+@@
+
 - i.ptr != NULL
 + isttest(i)
 
-- 
2.33.1




[PATCH] REGTESTS: Use `feature cmd` for 2.5+ tests (2)

2021-11-04 Thread Tim Duesterhus
This patch effectively is identical to 7ba98480cc5b2ede0fd4cca162959f66beb82c82.
---
 reg-tests/connection/cli_src_dst.vtc| 3 +--
 reg-tests/http-messaging/http_transfer_encoding.vtc | 4 ++--
 reg-tests/http-messaging/srv_ws.vtc | 5 ++---
 reg-tests/http-rules/default_rules.vtc  | 3 +--
 reg-tests/startup/default_rules.vtc | 3 +--
 reg-tests/tcp-rules/default_rules.vtc   | 3 +--
 6 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/reg-tests/connection/cli_src_dst.vtc 
b/reg-tests/connection/cli_src_dst.vtc
index cc0c94545..fa12bc805 100644
--- a/reg-tests/connection/cli_src_dst.vtc
+++ b/reg-tests/connection/cli_src_dst.vtc
@@ -1,7 +1,6 @@
 varnishtest "Test multi-level client source and destination addresses"
 
-#REQUIRE_VERSION=2.5
-
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature ignore_unknown_macro
 
 haproxy h1 -conf {
diff --git a/reg-tests/http-messaging/http_transfer_encoding.vtc 
b/reg-tests/http-messaging/http_transfer_encoding.vtc
index 543e965fa..258b8a9e8 100644
--- a/reg-tests/http-messaging/http_transfer_encoding.vtc
+++ b/reg-tests/http-messaging/http_transfer_encoding.vtc
@@ -1,7 +1,7 @@
 varnishtest "A test to validate Transfer-Encoding header conformance to the 
spec"
-feature ignore_unknown_macro
 
-#REQUIRE_VERSION=2.5
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
+feature ignore_unknown_macro
 
 server s1 {
 rxreq
diff --git a/reg-tests/http-messaging/srv_ws.vtc 
b/reg-tests/http-messaging/srv_ws.vtc
index bce12f6b1..32369a1a3 100644
--- a/reg-tests/http-messaging/srv_ws.vtc
+++ b/reg-tests/http-messaging/srv_ws.vtc
@@ -3,11 +3,10 @@
 
 varnishtest "h2 backend websocket management via server keyword"
 
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
 feature ignore_unknown_macro
 
-#REQUIRE_VERSION=2.5
-#REQUIRE_OPTION=OPENSSL
-
 # haproxy server
 haproxy hapsrv -conf {
defaults
diff --git a/reg-tests/http-rules/default_rules.vtc 
b/reg-tests/http-rules/default_rules.vtc
index a72776c07..3baa33a92 100644
--- a/reg-tests/http-rules/default_rules.vtc
+++ b/reg-tests/http-rules/default_rules.vtc
@@ -1,7 +1,6 @@
 varnishtest "Test declaration of HTTP rules in default sections"
 
-#REQUIRE_VERSION=2.5
-
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature ignore_unknown_macro
 
 server s1 {
diff --git a/reg-tests/startup/default_rules.vtc 
b/reg-tests/startup/default_rules.vtc
index 4c8051312..cd86f7414 100644
--- a/reg-tests/startup/default_rules.vtc
+++ b/reg-tests/startup/default_rules.vtc
@@ -1,7 +1,6 @@
 varnishtest "Misuses of defaults section defining TCP/HTTP rules"
 
-#REQUIRE_VERSION=2.5
-
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature ignore_unknown_macro
 
 #
diff --git a/reg-tests/tcp-rules/default_rules.vtc 
b/reg-tests/tcp-rules/default_rules.vtc
index 826a336cb..a2e8ce9ef 100644
--- a/reg-tests/tcp-rules/default_rules.vtc
+++ b/reg-tests/tcp-rules/default_rules.vtc
@@ -1,7 +1,6 @@
 varnishtest "Test declaration of TCP rules in default sections"
 
-#REQUIRE_VERSION=2.5
-
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature ignore_unknown_macro
 
 server s1 {
-- 
2.33.1




[PATCH] CLEANUP: halog: Remove dead stores

2021-11-04 Thread Tim Duesterhus
Found using clang's scan-build.
---
 admin/halog/halog.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/admin/halog/halog.c b/admin/halog/halog.c
index 900cf5d46..f368c1c6f 100644
--- a/admin/halog/halog.c
+++ b/admin/halog/halog.c
@@ -551,7 +551,8 @@ int convert_date_to_timestamp(const char *field)
d = mo = y = h = m = s = 0;
e = field;
 
-   c = *(e++); // remove '['
+   e++; // remove '['
+
/* day + '/' */
while (1) {
c = *(e++) - '0';
@@ -1148,13 +1149,12 @@ int main(int argc, char **argv)
/* sort all timers */
for (f = 0; f < 5; f++) {
struct eb32_node *n;
-   int val;
 
-   val = 0;
n = eb32_first([f]);
while (n) {
int i;
double d;
+   int val;
 
t = container_of(n, struct timer, node);
last = n->key;
-- 
2.33.1




[PATCH 2/2] CLEANUP: Apply ha_free.cocci

2021-11-04 Thread Tim Duesterhus
Use `ha_free()` where possible.
---
 src/action.c   | 3 +--
 src/server.c   | 3 +--
 src/ssl_ckch.c | 6 ++
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/action.c b/src/action.c
index ba465a253..1de97692e 100644
--- a/src/action.c
+++ b/src/action.c
@@ -39,8 +39,7 @@ int check_action_rules(struct list *rules, struct proxy *px, 
int *err_code)
err++;
}
*err_code |= warnif_tcp_http_cond(px, rule->cond);
-   free(errmsg);
-   errmsg = NULL;
+   ha_free();
}
 
return err;
diff --git a/src/server.c b/src/server.c
index a0206021d..a8e85a982 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2380,8 +2380,7 @@ struct server *srv_drop(struct server *srv)
 
EXTRA_COUNTERS_FREE(srv->extra_counters);
 
-   free(srv);
-   srv = NULL;
+   ha_free();
 
  end:
return next;
diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 2378ee349..eeb031b27 100644
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -2506,8 +2506,7 @@ static int cli_parse_set_cafile(char **args, char 
*payload, struct appctx *appct
appctx->ctx.ssl.new_cafile_entry = NULL;
appctx->ctx.ssl.old_cafile_entry = NULL;
 
-   free(appctx->ctx.ssl.path);
-   appctx->ctx.ssl.path = NULL;
+   ha_free(>ctx.ssl.path);
 
HA_SPIN_UNLOCK(CKCH_LOCK, _lock);
return cli_dynerr(appctx, memprintf(, "%sCan't update 
%s!\n", err ? err : "", args[3]));
@@ -3225,8 +3224,7 @@ static int cli_parse_set_crlfile(char **args, char 
*payload, struct appctx *appc
appctx->ctx.ssl.new_crlfile_entry = NULL;
appctx->ctx.ssl.old_crlfile_entry = NULL;
 
-   free(appctx->ctx.ssl.path);
-   appctx->ctx.ssl.path = NULL;
+   ha_free(>ctx.ssl.path);
 
HA_SPIN_UNLOCK(CKCH_LOCK, _lock);
return cli_dynerr(appctx, memprintf(, "%sCan't update 
%s!\n", err ? err : "", args[3]));
-- 
2.33.1




[PATCH 1/2] DEV: coccinelle: Add ha_free.cocci

2021-11-04 Thread Tim Duesterhus
Taken from 61cfdf4fd8a93dc6fd9922d5b309a71bdc7d2853.
---
 dev/coccinelle/ha_free.cocci | 6 ++
 1 file changed, 6 insertions(+)
 create mode 100644 dev/coccinelle/ha_free.cocci

diff --git a/dev/coccinelle/ha_free.cocci b/dev/coccinelle/ha_free.cocci
new file mode 100644
index 0..00190393b
--- /dev/null
+++ b/dev/coccinelle/ha_free.cocci
@@ -0,0 +1,6 @@
+@ rule @
+expression E;
+@@
+- free(E);
+- E = NULL;
++ ha_free();
-- 
2.33.1




[PATCH 1/2] MINOR: jwt: Make invalid static JWT algorithms an error in `jwt_verify` converter

2021-10-29 Thread Tim Duesterhus
It is not useful to start a configuration where an invalid static string is
provided as the JWT algorithm. Better make the administrator aware of the
suspected typo by failing to start.
---
 src/sample.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/sample.c b/src/sample.c
index 9200ca303..5abf4712a 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -3522,14 +3522,14 @@ static int sample_conv_jwt_verify_check(struct arg 
*args, struct sample_conv *co
 
switch(alg) {
case JWT_ALG_DEFAULT:
-   memprintf(err, "unknown JWT algorithm : %s", *err);
-   break;
+   memprintf(err, "unknown JWT algorithm: %s", 
args[0].data.str.area);
+   return 0;
 
case JWS_ALG_PS256:
case JWS_ALG_PS384:
case JWS_ALG_PS512:
memprintf(err, "RSASSA-PSS JWS signing not managed 
yet");
-   break;
+   return 0;
 
default:
break;
-- 
2.33.1




[PATCH 2/2] BUG/MINOR: jwt: Fix jwt_parse_alg incorrectly returning JWS_ALG_NONE

2021-10-29 Thread Tim Duesterhus
Hi Remi, Willy,

Is the length check at the start of `jwt_parse_alg()` actually useful? I would
expect that the vast majority of strings passed are valid algorithms that are
*not* `none`. Thus I expect this `if()` to almost never be `true`.

Should the `if()` be removed and a new `case 'n'` be added to the switch? Or
should an `unlikely()` be added around the condition?

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
`jwt_parse_alg()` previously incorrectly returned `JWS_ALG_NONE` for inputs
`""`, `"n"`, `"no"`, and `"non"` due to an incorrect check with `strncmp` that
also matches prefixes.

This bug did not affect the matching of the other known variants, because of
the special cased length check at the start of the function. Nonetheless these
variants are also affected and this bug might've been exposed during 
refactoring.

I did not see an small fix for `strncmp`, so I used the opportunity to migrate
this function to the ist API, which avoids the issue altogether. The overall
structure of this function was preserved.

A config like:

http-response set-header bearer %[str(),jwt_verify(,)]

Now correctly returns:

> [ALERT](109770) : config : parsing [./haproxy.cfg:6] : error detected in
> proxy 'test' while parsing 'http-response set-header' rule : failed to parse
> sample expression  : invalid args in converter
> 'jwt_verify' : unknown JWT algorithm: .

JWT support is new in 2.5, no backport needed.
---
 include/haproxy/jwt.h |  2 +-
 src/jwt.c | 34 +-
 src/sample.c  |  2 +-
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/include/haproxy/jwt.h b/include/haproxy/jwt.h
index a343ffaf7..84421530d 100644
--- a/include/haproxy/jwt.h
+++ b/include/haproxy/jwt.h
@@ -26,7 +26,7 @@
 #include 
 
 #ifdef USE_OPENSSL
-enum jwt_alg jwt_parse_alg(const char *alg_str, unsigned int alg_len);
+enum jwt_alg jwt_parse_alg(struct ist);
 int jwt_tokenize(const struct buffer *jwt, struct jwt_item *items, unsigned 
int *item_num);
 int jwt_tree_load_cert(char *path, int pathlen, char **err);
 
diff --git a/src/jwt.c b/src/jwt.c
index 94bfa5adb..590b18c3b 100644
--- a/src/jwt.c
+++ b/src/jwt.c
@@ -28,49 +28,49 @@ static struct eb_root jwt_cert_tree = EB_ROOT_UNIQUE;
  * The possible algorithm strings that can be found in a JWS's JOSE header are
  * defined in section 3.1 of RFC7518.
  */
-enum jwt_alg jwt_parse_alg(const char *alg_str, unsigned int alg_len)
+enum jwt_alg jwt_parse_alg(struct ist str)
 {
enum jwt_alg alg = JWT_ALG_DEFAULT;
 
/* Algorithms are all 5 characters long apart from "none". */
-   if (alg_len < sizeof("HS256")-1) {
-   if (strncmp("none", alg_str, alg_len) == 0)
+   if (istlen(str) < sizeof("HS256")-1) {
+   if (isteq(str, ist("none")))
alg = JWS_ALG_NONE;
return alg;
}
 
if (alg == JWT_ALG_DEFAULT) {
-   switch(*alg_str++) {
+   switch(istshift()) {
case 'H':
-   if (strncmp(alg_str, "S256", alg_len-1) == 0)
+   if (isteq(str, ist("S256")))
alg = JWS_ALG_HS256;
-   else if (strncmp(alg_str, "S384", alg_len-1) == 0)
+   else if (isteq(str, ist("S384")))
alg = JWS_ALG_HS384;
-   else if (strncmp(alg_str, "S512", alg_len-1) == 0)
+   else if (isteq(str, ist("S512")))
alg = JWS_ALG_HS512;
break;
case 'R':
-   if (strncmp(alg_str, "S256", alg_len-1) == 0)
+   if (isteq(str, ist("S256")))
alg = JWS_ALG_RS256;
-   else if (strncmp(alg_str, "S384", alg_len-1) == 0)
+   else if (isteq(str, ist("S384")))
alg = JWS_ALG_RS384;
-   else if (strncmp(alg_str, "S512", alg_len-1) == 0)
+   else if (isteq(str, ist("S512")))
alg = JWS_ALG_RS512;
break;
case 'E':
-   if (strncmp(alg_str, "S256", alg_len-1) == 0)
+   if (isteq(str, ist("S256")))
alg = JWS_ALG_ES256;
-   else if (strncmp(alg_str, "S384", alg_len-1) == 0)
+   else if (isteq(str, ist("S384")))
alg = JWS_ALG_ES384;
-   else if (strncmp(alg_str, "S512", alg_len-1) == 0)
+   else if (isteq(str, ist("S512")))
alg = JWS_ALG_ES512;
break;
case 'P':
-   if (strncmp(alg_str, "S256", alg_len-1) == 0)
+   

[PATCH] CLEANUP: hlua: Remove obsolete branch in `hlua_alloc()`

2021-10-23 Thread Tim Duesterhus
This branch is no longer required, because the `!nsize` case is handled for any
value of `ptr` now.

see 22586524e32f14c44239063088a38ccea8abc9b7
see a5efdff93c36f75345a2a18f18bffee9b602bc7b
---
 src/hlua.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/hlua.c b/src/hlua.c
index ac61a3171..0e12614af 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -11463,9 +11463,6 @@ static void *hlua_alloc(void *ud, void *ptr, size_t 
osize, size_t nsize)
struct hlua_mem_allocator *zone = ud;
size_t limit, old, new;
 
-   if (unlikely(!ptr && !nsize))
-   return NULL;
-
/* a limit of ~0 means unlimited and boot complete, so there's no need
 * for accounting anymore.
 */
-- 
2.33.1




[PATCH] DEV: coccinelle: Add realloc_leak.cocci

2021-10-23 Thread Tim Duesterhus
This coccinelle patch finds locations where the return value of `realloc()` is
assigned to the pointer passed to `realloc()`. This calls will leak memory if
`realloc()` returns `NULL`.
---
 dev/coccinelle/realloc_leak.cocci | 6 ++
 1 file changed, 6 insertions(+)
 create mode 100644 dev/coccinelle/realloc_leak.cocci

diff --git a/dev/coccinelle/realloc_leak.cocci 
b/dev/coccinelle/realloc_leak.cocci
new file mode 100644
index 0..c201b808c
--- /dev/null
+++ b/dev/coccinelle/realloc_leak.cocci
@@ -0,0 +1,6 @@
+@@
+expression E;
+expression F;
+@@
+
+* E = realloc(E, F);
-- 
2.33.1




[PATCH 2/2] CLEANUP: jwt: Remove the use of a trash buffer in jwt_jwsverify_rsa_ecdsa()

2021-10-18 Thread Tim Duesterhus
`trash` was completely unused within this function.
---
 src/jwt.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/src/jwt.c b/src/jwt.c
index d075bcfd4..94bfa5adb 100644
--- a/src/jwt.c
+++ b/src/jwt.c
@@ -214,14 +214,9 @@ jwt_jwsverify_rsa_ecdsa(const struct jwt_ctx *ctx, const 
struct buffer *decoded_
const EVP_MD *evp = NULL;
EVP_MD_CTX *evp_md_ctx;
enum jwt_vrfy_status retval = JWT_VRFY_KO;
-   struct buffer *trash = NULL;
struct ebmb_node *eb;
struct jwt_cert_tree_entry *entry = NULL;
 
-   trash = alloc_trash_chunk();
-   if (!trash)
-   return JWT_VRFY_OUT_OF_MEMORY;
-
switch(ctx->alg) {
case JWS_ALG_RS256:
case JWS_ALG_ES256:
@@ -239,10 +234,8 @@ jwt_jwsverify_rsa_ecdsa(const struct jwt_ctx *ctx, const 
struct buffer *decoded_
}
 
evp_md_ctx = EVP_MD_CTX_new();
-   if (!evp_md_ctx) {
-   free_trash_chunk(trash);
+   if (!evp_md_ctx)
return JWT_VRFY_OUT_OF_MEMORY;
-   }
 
eb = ebst_lookup(_cert_tree, ctx->key);
 
@@ -267,7 +260,6 @@ jwt_jwsverify_rsa_ecdsa(const struct jwt_ctx *ctx, const 
struct buffer *decoded_
 
 end:
EVP_MD_CTX_free(evp_md_ctx);
-   free_trash_chunk(trash);
return retval;
 }
 
-- 
2.33.0




[PATCH 1/2] CLEANUP: jwt: Remove the use of a trash buffer in jwt_jwsverify_hmac()

2021-10-18 Thread Tim Duesterhus
The OpenSSL documentation (https://www.openssl.org/docs/man1.1.0/man3/HMAC.html)
specifies:

> It places the result in md (which must have space for the output of the hash
> function, which is no more than EVP_MAX_MD_SIZE bytes). If md is NULL, the
> digest is placed in a static array. The size of the output is placed in
> md_len, unless it is NULL. Note: passing a NULL value for md to use the
> static array is not thread safe.

`EVP_MAX_MD_SIZE` appears to be defined as `64`, so let's simply use a stack
buffer to avoid the whole memory management.
---
 src/jwt.c | 12 +---
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/src/jwt.c b/src/jwt.c
index e29a1c797..d075bcfd4 100644
--- a/src/jwt.c
+++ b/src/jwt.c
@@ -175,19 +175,11 @@ static enum jwt_vrfy_status
 jwt_jwsverify_hmac(const struct jwt_ctx *ctx, const struct buffer 
*decoded_signature)
 {
const EVP_MD *evp = NULL;
-   unsigned char *signature = NULL;
+   unsigned char signature[EVP_MAX_MD_SIZE];
unsigned int signature_length = 0;
-   struct buffer *trash = NULL;
unsigned char *hmac_res = NULL;
enum jwt_vrfy_status retval = JWT_VRFY_KO;
 
-   trash = alloc_trash_chunk();
-   if (!trash)
-   return JWT_VRFY_OUT_OF_MEMORY;
-
-   signature = (unsigned char*)trash->area;
-   signature_length = trash->size;
-
switch(ctx->alg) {
case JWS_ALG_HS256:
evp = EVP_sha256();
@@ -208,8 +200,6 @@ jwt_jwsverify_hmac(const struct jwt_ctx *ctx, const struct 
buffer *decoded_signa
  (CRYPTO_memcmp(decoded_signature->area, signature, 
signature_length) == 0))
retval = JWT_VRFY_OK;
 
-   free_trash_chunk(trash);
-
return retval;
 }
 
-- 
2.33.0




[PATCH] CLEANUP: Consistently `unsigned int` for bitfields

2021-10-16 Thread Tim Duesterhus
see 6a0dd733906611dea958cf74b9f51bb16028ae20

Found using GitHub's CodeQL scan.
---
 include/haproxy/stick_table-t.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/haproxy/stick_table-t.h b/include/haproxy/stick_table-t.h
index 3b1f2b3ef..133f992b5 100644
--- a/include/haproxy/stick_table-t.h
+++ b/include/haproxy/stick_table-t.h
@@ -125,8 +125,8 @@ struct stktable_data_type {
const char *name; /* name of the data type */
int std_type; /* standard type we can use for this data, STD_T_* */
int arg_type; /* type of optional argument, ARG_T_* */
-   int is_array:1;   /* this is an array of gpc/gpt */
-   int is_local:1;   /* this is local only and never learned */
+   unsigned int is_array:1;   /* this is an array of gpc/gpt */
+   unsigned int is_local:1;   /* this is local only and never learned */
 };
 
 /* stick table keyword type */
-- 
2.33.0




[PATCH 1/2] CI: Add `permissions` to GitHub Actions

2021-10-16 Thread Tim Duesterhus
This change locks down the permissions of the access token in GitHub Actions to
only allow reading the repository contents and nothing else.

see 
https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token
---
 .github/workflows/codespell.yml| 3 +++
 .github/workflows/compliance.yml   | 3 +++
 .github/workflows/contrib.yml  | 3 +++
 .github/workflows/coverity.yml | 3 +++
 .github/workflows/musl.yml | 3 +++
 .github/workflows/openssl-nodeprecated.yml | 3 +++
 .github/workflows/vtest.yml| 3 +++
 .github/workflows/windows.yml  | 3 +++
 8 files changed, 24 insertions(+)

diff --git a/.github/workflows/codespell.yml b/.github/workflows/codespell.yml
index de49f4343..61edaeb9e 100644
--- a/.github/workflows/codespell.yml
+++ b/.github/workflows/codespell.yml
@@ -4,6 +4,9 @@ on:
   schedule:
 - cron: "0 0 * * 2"
 
+permissions:
+  contents: read
+
 jobs:
   codespell:
 
diff --git a/.github/workflows/compliance.yml b/.github/workflows/compliance.yml
index 9f2bec289..fe6c2711e 100644
--- a/.github/workflows/compliance.yml
+++ b/.github/workflows/compliance.yml
@@ -5,6 +5,9 @@ on:
   schedule:
 - cron: "0 0 * * 3"
 
+permissions:
+  contents: read
+
 jobs:
   h2spec:
 name: h2spec
diff --git a/.github/workflows/contrib.yml b/.github/workflows/contrib.yml
index 53f6025ca..93387a458 100644
--- a/.github/workflows/contrib.yml
+++ b/.github/workflows/contrib.yml
@@ -3,6 +3,9 @@ name: Contrib
 on:
   push:
 
+permissions:
+  contents: read
+
 jobs:
   build:
 
diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index fd5a0e2d2..b3dd5ec52 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -9,6 +9,9 @@ on:
   schedule:
   - cron: "0 0 * * *"
 
+permissions:
+  contents: read
+
 jobs:
   scan:
 runs-on: ubuntu-latest
diff --git a/.github/workflows/musl.yml b/.github/workflows/musl.yml
index 8f6922486..19d82af7c 100644
--- a/.github/workflows/musl.yml
+++ b/.github/workflows/musl.yml
@@ -2,6 +2,9 @@ name: alpine/musl
 
 on: [push]
 
+permissions:
+  contents: read
+
 jobs:
   musl:
   name: gcc
diff --git a/.github/workflows/openssl-nodeprecated.yml 
b/.github/workflows/openssl-nodeprecated.yml
index 6833911e4..f6da38234 100644
--- a/.github/workflows/openssl-nodeprecated.yml
+++ b/.github/workflows/openssl-nodeprecated.yml
@@ -14,6 +14,9 @@ on:
   schedule:
 - cron: "0 0 * * 4"
 
+permissions:
+  contents: read
+
 jobs:
   test:
 
diff --git a/.github/workflows/vtest.yml b/.github/workflows/vtest.yml
index 1dc216eeb..4cdbdce5b 100644
--- a/.github/workflows/vtest.yml
+++ b/.github/workflows/vtest.yml
@@ -11,6 +11,9 @@ name: VTest
 on:
   push:
 
+permissions:
+  contents: read
+
 jobs:
   # The generate-matrix job generates the build matrix using JSON output
   # generated by .github/matrix.py.
diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml
index b5a198aff..42bb4e8c9 100644
--- a/.github/workflows/windows.yml
+++ b/.github/workflows/windows.yml
@@ -11,6 +11,9 @@ name: Windows
 on:
   push:
 
+permissions:
+  contents: read
+
 jobs:
   msys2:
 name: ${{ matrix.name }}
-- 
2.33.0




[PATCH 2/2] CI: Clean up formatting in GitHub Action definitions

2021-10-16 Thread Tim Duesterhus
This patch cleans up the formatting within the .yml definition files for GitHub
Actions to ensure a consistent look across all actions.
---
 .github/workflows/codespell.yml| 15 +++---
 .github/workflows/compliance.yml   |  2 +-
 .github/workflows/contrib.yml  |  2 -
 .github/workflows/musl.yml | 57 +++---
 .github/workflows/openssl-nodeprecated.yml | 10 ++--
 5 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/.github/workflows/codespell.yml b/.github/workflows/codespell.yml
index 61edaeb9e..955560a0a 100644
--- a/.github/workflows/codespell.yml
+++ b/.github/workflows/codespell.yml
@@ -1,4 +1,4 @@
-name: codespell
+name: Spelling Check
 
 on:
   schedule:
@@ -9,12 +9,15 @@ permissions:
 
 jobs:
   codespell:
-
 runs-on: ubuntu-latest
-
 steps:
 - uses: actions/checkout@v2
-- name: install prerequisites
+- name: Install codespell
   run: sudo pip install codespell
-- name: check
-  run: codespell -c -q 2 --ignore-words-list 
ist,ists,hist,wan,ca,cas,que,ans,te,nd,referer,ot,uint,iif,fo,keep-alives,dosen 
--skip="CHANGELOG,Makefile,*.fig,*.pem"
+- name: Run codespell
+  run: |
+codespell \
+  -c \
+  -q 2 \
+  --ignore-words-list 
ist,ists,hist,wan,ca,cas,que,ans,te,nd,referer,ot,uint,iif,fo,keep-alives,dosen 
\
+  --skip="CHANGELOG,Makefile,*.fig,*.pem"
diff --git a/.github/workflows/compliance.yml b/.github/workflows/compliance.yml
index fe6c2711e..3ce717805 100644
--- a/.github/workflows/compliance.yml
+++ b/.github/workflows/compliance.yml
@@ -3,7 +3,7 @@ name: Spec Compliance
 
 on:
   schedule:
-- cron: "0 0 * * 3"
+  - cron: "0 0 * * 3"
 
 permissions:
   contents: read
diff --git a/.github/workflows/contrib.yml b/.github/workflows/contrib.yml
index 93387a458..480f61be8 100644
--- a/.github/workflows/contrib.yml
+++ b/.github/workflows/contrib.yml
@@ -8,9 +8,7 @@ permissions:
 
 jobs:
   build:
-
 runs-on: ubuntu-latest
-
 steps:
 - uses: actions/checkout@v2
 - name: Compile admin/halog/halog
diff --git a/.github/workflows/musl.yml b/.github/workflows/musl.yml
index 19d82af7c..c106b1d05 100644
--- a/.github/workflows/musl.yml
+++ b/.github/workflows/musl.yml
@@ -1,6 +1,7 @@
 name: alpine/musl
 
-on: [push]
+on:
+  push:
 
 permissions:
   contents: read
@@ -12,30 +13,30 @@ jobs:
   container:
 image: alpine:latest
   steps:
-- uses: actions/checkout@master
-- name: Install dependencies
-  run: apk add gcc make tar git python3 libc-dev linux-headers 
pcre-dev pcre2-dev openssl-dev lua5.3-dev grep socat curl
-- name: Install VTest
-  run: scripts/build-vtest.sh
-- name: Build
-  run: make -j$(nproc) CC=cc V=1 TARGET=linux-musl USE_LUA=1 
LUA_INC=/usr/include/lua5.3 LUA_LIB=/usr/lib/lua5.3 USE_OPENSSL=1 USE_PCRE2=1 
USE_PCRE2_JIT=1 USE_PROMEX=1
-- name: Show version
-  run: ./haproxy -vv
-- name: Show linked libraries
-  run: ldd haproxy
-- name: Install problem matcher for VTest
-  # This allows one to more easily see which tests fail.
-  run: echo "::add-matcher::.github/vtest.json"
-- name: Run VTest
-  id: vtest
-  run: make reg-tests VTEST_PROGRAM=../vtest/vtest 
REGTESTS_TYPES=default,bug,devel
-- name: Show results
-  if: ${{ failure() }}
-  run: |
-for folder in /tmp/haregtests-*/vtc.*; do
-  printf "::group::"
-  cat $folder/INFO
-  cat $folder/LOG
-  echo "::endgroup::"
-done
-shopt -s nullglob
+  - uses: actions/checkout@master
+  - name: Install dependencies
+run: apk add gcc make tar git python3 libc-dev linux-headers pcre-dev 
pcre2-dev openssl-dev lua5.3-dev grep socat curl
+  - name: Install VTest
+run: scripts/build-vtest.sh
+  - name: Build
+run: make -j$(nproc) CC=cc V=1 TARGET=linux-musl USE_LUA=1 
LUA_INC=/usr/include/lua5.3 LUA_LIB=/usr/lib/lua5.3 USE_OPENSSL=1 USE_PCRE2=1 
USE_PCRE2_JIT=1 USE_PROMEX=1
+  - name: Show version
+run: ./haproxy -vv
+  - name: Show linked libraries
+run: ldd haproxy
+  - name: Install problem matcher for VTest
+# This allows one to more easily see which tests fail.
+run: echo "::add-matcher::.github/vtest.json"
+  - name: Run VTest
+id: vtest
+run: make reg-tests VTEST_PROGRAM=../vtest/vtest 
REGTESTS_TYPES=default,bug,devel
+  - name: Show results
+if: ${{ failure() }}
+run: |
+  for folder in /tmp/haregtests-*/vtc.*; do
+printf "::group::"
+cat $folder/INFO
+cat $folder/LOG
+echo "::endgroup::"
+  done
+  shopt -s nullglob
diff --git a/.github/workflows/openssl-nodeprecated.yml 
b/.github/workflows/openssl-nodeprecated.yml
index 

[PATCH 1/2] DEV: coccinelle: Add strcmp.cocci

2021-10-16 Thread Tim Duesterhus
see e5ff14100aceba70714a2d8549ee7621ffc2701e
---
 dev/coccinelle/strcmp.cocci | 309 
 1 file changed, 309 insertions(+)
 create mode 100644 dev/coccinelle/strcmp.cocci

diff --git a/dev/coccinelle/strcmp.cocci b/dev/coccinelle/strcmp.cocci
new file mode 100644
index 0..f6064bffb
--- /dev/null
+++ b/dev/coccinelle/strcmp.cocci
@@ -0,0 +1,309 @@
+@@
+statement S;
+expression E;
+expression F;
+@@
+
+  if (
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) != 0
+  )
+(
+  S
+|
+  { ... }
+)
+
+@@
+statement S;
+expression E;
+expression F;
+@@
+
+  if (
+- !
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) == 0
+  )
+(
+  S
+|
+  { ... }
+)
+
+@@
+expression E;
+expression F;
+expression G;
+@@
+
+(
+G &&
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) != 0
+)
+
+@@
+expression E;
+expression F;
+expression G;
+@@
+
+(
+G ||
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) != 0
+)
+
+@@
+expression E;
+expression F;
+expression G;
+@@
+
+(
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) != 0
+&& G
+)
+
+@@
+expression E;
+expression F;
+expression G;
+@@
+
+(
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) != 0
+|| G
+)
+
+@@
+expression E;
+expression F;
+expression G;
+@@
+
+(
+G &&
+- !
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) == 0
+)
+
+@@
+expression E;
+expression F;
+expression G;
+@@
+
+(
+G ||
+- !
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) == 0
+)
+
+@@
+expression E;
+expression F;
+expression G;
+@@
+
+(
+- !
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) == 0
+&& G
+)
+
+@@
+expression E;
+expression F;
+expression G;
+@@
+
+(
+- !
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) == 0
+|| G
+)
+
+@@
+expression E;
+expression F;
+expression G;
+@@
+
+(
+- !
+(
+dns_hostname_cmp
+|
+eb_memcmp
+|
+memcmp
+|
+strcasecmp
+|
+strcmp
+|
+strncasecmp
+|
+strncmp
+)
+-  (E, F)
++  (E, F) == 0
+)
-- 
2.33.0




[PATCH 2/2] CLEANUP: Apply strcmp.cocci

2021-10-16 Thread Tim Duesterhus
This fixes the use of the various *cmp functions to use != 0 or == 0.
---
 src/cfgparse.c | 2 +-
 src/cli.c  | 4 ++--
 src/server.c   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index f013928f6..892170284 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -1536,7 +1536,7 @@ static void check_section_position(char *section_name,
const char *file, int linenum,
int *non_global_parsed)
 {
-   if (!strcmp(section_name, "global")) {
+   if (strcmp(section_name, "global") == 0) {
if (*non_global_parsed == 1)
_ha_diag_warning("parsing [%s:%d] : global section 
detected after a non-global one, the prevalence of their statements is 
unspecified\n", file, linenum);
}
diff --git a/src/cli.c b/src/cli.c
index a3ee4c442..6a46f138c 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -1714,11 +1714,11 @@ static int cli_parse_expert_experimental_mode(char 
**args, char *payload, struct
if (!cli_has_level(appctx, ACCESS_LVL_ADMIN))
return 1;
 
-   if (!strcmp(args[0], "expert-mode")) {
+   if (strcmp(args[0], "expert-mode") == 0) {
level = ACCESS_EXPERT;
level_str = "expert-mode";
}
-   else if (!strcmp(args[0], "experimental-mode")) {
+   else if (strcmp(args[0], "experimental-mode") == 0) {
level = ACCESS_EXPERIMENTAL;
level_str = "experimental-mode";
}
diff --git a/src/server.c b/src/server.c
index b8908cefa..15c4d0536 100644
--- a/src/server.c
+++ b/src/server.c
@@ -4681,7 +4681,7 @@ static int cli_parse_add_server(char **args, char 
*payload, struct appctx *appct
 
while (1) {
/* check for duplicate server */
-   if (!strcmp(srv->id, next->id)) {
+   if (strcmp(srv->id, next->id) == 0) {
ha_alert("Already exists a server with the same 
name in backend.\n");
goto out;
}
-- 
2.33.0




[PATCH] CLEANUP: http_fetch: Use ist helpers in smp_fetch_http_auth_bearer()

2021-10-14 Thread Tim Duesterhus
Remi,

please find a suggested cleanup for your JWT patch series. I think that
using the ist functions results in easier to understand code, because you
don't need to manually calculate lengths and offsets.

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
Using the ist helper functions is arguably cleaner than using C's regular
string functions on an ist.
---
 src/http_fetch.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/http_fetch.c b/src/http_fetch.c
index 7b4e41d85..61545d66f 100644
--- a/src/http_fetch.c
+++ b/src/http_fetch.c
@@ -1369,10 +1369,10 @@ static int smp_fetch_http_auth_bearer(const struct arg 
*args, struct sample *smp
 
ctx.blk = NULL;
if (http_find_header(htx, hdr_name, , 0)) {
-   char *space = NULL;
-   space = memchr(ctx.value.ptr, ' ', ctx.value.len);
-   if (space && strncasecmp("Bearer", ctx.value.ptr, 
ctx.value.len) == 0) {
-   chunk_initlen(_val, space+1, 0, 
ctx.value.len - (space - ctx.value.ptr) - 1);
+   struct ist type = istsplit(, ' ');
+
+   if (isteqi(type, ist("Bearer")) && istend(type) != 
istend(ctx.value)) {
+   chunk_initlen(_val, istptr(ctx.value), 
0, istlen(ctx.value));
}
}
}
-- 
2.33.0




[PATCH] BUG/MINOR: lua: Fix lua error handling in `hlua_config_prepend_path()`

2021-10-11 Thread Tim Duesterhus
Set an `lua_atpanic()` handler before calling `hlua_prepend_path()` in
`hlua_config_prepend_path()`.

This prevents the process from abort()ing when `hlua_prepend_path()` fails
for some reason.

see GitHub Issue #1409

This is a very minor issue that can't happen in practice. No backport needed.
---
 src/hlua.c | 27 +--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/src/hlua.c b/src/hlua.c
index b03d8bb46..1dbaaa3e0 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -11167,8 +11167,31 @@ static int hlua_config_prepend_path(char **args, int 
section_type, struct proxy
}
LIST_APPEND(_path_list, >l);
 
-   hlua_prepend_path(hlua_states[0], type, path);
-   hlua_prepend_path(hlua_states[1], type, path);
+   /* Handle the global state and the per-thread state for the first
+* thread. The remaining threads will be initialized based on
+* prepend_path_list.
+*/
+   for (size_t i = 0; i < 2; i++) {
+   lua_State *L = hlua_states[i];
+   const char *error;
+
+   if (setjmp(safe_ljmp_env) != 0) {
+   lua_atpanic(L, hlua_panic_safe);
+   if (lua_type(L, -1) == LUA_TSTRING)
+   error = lua_tostring(L, -1);
+   else
+   error = "critical error";
+   fprintf(stderr, "lua-prepend-path: %s.\n", error);
+   exit(1);
+   } else {
+   lua_atpanic(L, hlua_panic_ljmp);
+   }
+
+   hlua_prepend_path(L, type, path);
+
+   lua_atpanic(L, hlua_panic_safe);
+   }
+
return 0;
 
 err2:
-- 
2.33.0




[PATCH] CLEANUP: slz: Mark `reset_refs` as static

2021-09-20 Thread Tim Duesterhus
Willy,

please also apply to https://github.com/wtarreau/libslz/.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This function has no prototype and is not used outside of slz.c.
---
 src/slz.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/slz.c b/src/slz.c
index 9aacb9b7a..23912da6a 100644
--- a/src/slz.c
+++ b/src/slz.c
@@ -466,7 +466,7 @@ static inline long memmatch(const unsigned char *a, const 
unsigned char *b, long
  * be applied to 64-bit aligned data exclusively, which makes it slightly
  * faster than the regular memset() since no alignment check is performed.
  */
-void reset_refs(union ref *refs, long count)
+static void reset_refs(union ref *refs, long count)
 {
/* avoid a shift/mask by casting to void* */
union ref *end = (void *)refs + count;
-- 
2.33.0




[PATCH 2/2] CLEANUP: Remove unreachable `break` from parse_time_err()

2021-09-16 Thread Tim Duesterhus
The `return` already leaves the function.
---
 src/tools.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/tools.c b/src/tools.c
index 76a0cd7f1..91b2acad8 100644
--- a/src/tools.c
+++ b/src/tools.c
@@ -2432,7 +2432,6 @@ const char *parse_time_err(const char *text, unsigned 
*ret, unsigned unit_flags)
break;
default:
return text;
-   break;
}
if (*(++text) != '\0') {
ha_warning("unexpected character '%c' after the timer value 
'%s', only "
-- 
2.33.0




[PATCH 1/2] CLEANUP: Include check.h in flt_spoe.c

2021-09-16 Thread Tim Duesterhus
This is required for the prototype of spoe_prepare_healthcheck_request().
---
 src/flt_spoe.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/flt_spoe.c b/src/flt_spoe.c
index 28a5a24f8..aeb68c889 100644
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.33.0




[PATCH 7/7] CLEANUP: Apply bug_on.cocci

2021-09-15 Thread Tim Duesterhus
The changes look safe to me, even if `DEBUG_STRICT` is not enabled.
---
 src/log.c   | 3 +--
 src/quic_sock.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/log.c b/src/log.c
index 5cae18293..875905102 100644
--- a/src/log.c
+++ b/src/log.c
@@ -3497,8 +3497,7 @@ void syslog_fd_handler(int fd)
struct listener *l = objt_listener(fdtab[fd].owner);
int max_accept;
 
-   if(!l)
-   ABORT_NOW();
+   BUG_ON(!l);
 
if (fdtab[fd].state & FD_POLL_IN) {
 
diff --git a/src/quic_sock.c b/src/quic_sock.c
index 82d627067..6c1f2d04c 100644
--- a/src/quic_sock.c
+++ b/src/quic_sock.c
@@ -201,8 +201,7 @@ void quic_sock_fd_iocb(int fd)
struct sockaddr_storage saddr = {0};
socklen_t saddrlen;
 
-   if (!l)
-   ABORT_NOW();
+   BUG_ON(!l);
 
if (!(fdtab[fd].state & FD_POLL_IN) || !fd_recv_ready(fd))
return;
-- 
2.33.0




[PATCH 6/7] DOC: Add bug_on.cocci

2021-09-15 Thread Tim Duesterhus
This replaces an if + ABORT_NOW() by BUG_ON(). It might change behavior,
because BUG_ON will result in a no-op if not enabled.
---
 scripts/coccinelle/bug_on.cocci | 7 +++
 1 file changed, 7 insertions(+)
 create mode 100644 scripts/coccinelle/bug_on.cocci

diff --git a/scripts/coccinelle/bug_on.cocci b/scripts/coccinelle/bug_on.cocci
new file mode 100644
index 0..38378794d
--- /dev/null
+++ b/scripts/coccinelle/bug_on.cocci
@@ -0,0 +1,7 @@
+@@
+expression E;
+@@
+
+- if (E)
+- ABORT_NOW();
++ BUG_ON(E);
-- 
2.33.0




[PATCH 5/7] DOC: Add xalloc_cast.cocci

2021-09-15 Thread Tim Duesterhus
This remove's C++ style casts from the return value of malloc/calloc.

see 403fd722ace1d98d3cfe17bbee1382bf58040466
---
 scripts/coccinelle/xalloc_cast.cocci | 11 +++
 1 file changed, 11 insertions(+)
 create mode 100644 scripts/coccinelle/xalloc_cast.cocci

diff --git a/scripts/coccinelle/xalloc_cast.cocci 
b/scripts/coccinelle/xalloc_cast.cocci
new file mode 100644
index 0..75baa0022
--- /dev/null
+++ b/scripts/coccinelle/xalloc_cast.cocci
@@ -0,0 +1,11 @@
+@@
+type T;
+@@
+
+- (T*)
+(
+malloc
+|
+calloc
+)
+  (...)
-- 
2.33.0




[PATCH 2/7] CLEANUP: Apply ist.cocci

2021-09-15 Thread Tim Duesterhus
This cleans up ist handling.
---
 addons/ot/src/http.c   |  9 +++--
 addons/promex/service-prometheus.c |  2 +-
 include/haproxy/htx.h  | 12 ++--
 src/hlua.c |  5 ++---
 src/stream.c   |  2 +-
 5 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/addons/ot/src/http.c b/addons/ot/src/http.c
index 0d824ddb5..3376a3b00 100644
--- a/addons/ot/src/http.c
+++ b/addons/ot/src/http.c
@@ -220,12 +220,10 @@ int flt_ot_http_header_set(struct channel *chn, const 
char *prefix, const char *
}
 
if (!FLT_OT_STR_ISVALID(prefix)) {
-   ist_name.ptr = (char *)name;
-   ist_name.len = strlen(name);
+   ist_name = ist2((char *)name, strlen(name));
}
else if (!FLT_OT_STR_ISVALID(name)) {
-   ist_name.ptr = (char *)prefix;
-   ist_name.len = strlen(prefix);
+   ist_name = ist2((char *)prefix, strlen(prefix));
}
else {
buffer = flt_ot_trash_alloc(0, err);
@@ -234,8 +232,7 @@ int flt_ot_http_header_set(struct channel *chn, const char 
*prefix, const char *
 
(void)chunk_printf(buffer, "%s-%s", prefix, name);
 
-   ist_name.ptr = buffer->area;
-   ist_name.len = buffer->data;
+   ist_name = ist2(buffer->area, buffer->data);
}
 
/* Remove all occurrences of the header. */
diff --git a/addons/promex/service-prometheus.c 
b/addons/promex/service-prometheus.c
index 6a42a47c3..2070b9c1f 100644
--- a/addons/promex/service-prometheus.c
+++ b/addons/promex/service-prometheus.c
@@ -1006,7 +1006,7 @@ static int promex_dump_srv_metrics(struct appctx *appctx, 
struct htx *htx)
val = 
mkf_u32(FO_STATUS, sv->check.status == appctx->ctx.stats.st_code);
check_state = 
get_check_status_info(appctx->ctx.stats.st_code);
labels[2].name = 
ist("state");
-   labels[2].value = 
ist2(check_state, strlen(check_state));
+   labels[2].value = 
ist(check_state);
if 
(!promex_dump_metric(appctx, htx, prefix, _st_metrics[appctx->st2],

, labels, , max))
goto full;
diff --git a/include/haproxy/htx.h b/include/haproxy/htx.h
index 93b3206a8..0faf29646 100644
--- a/include/haproxy/htx.h
+++ b/include/haproxy/htx.h
@@ -385,8 +385,8 @@ static inline struct ist htx_get_blk_name(const struct htx 
*htx, const struct ht
switch (type) {
case HTX_BLK_HDR:
case HTX_BLK_TLR:
-   ret.ptr = htx_get_blk_ptr(htx, blk);
-   ret.len = blk->info & 0xff;
+   ret = ist2(htx_get_blk_ptr(htx, blk),
+  blk->info & 0xff);
break;
 
default:
@@ -407,15 +407,15 @@ static inline struct ist htx_get_blk_value(const struct 
htx *htx, const struct h
switch (type) {
case HTX_BLK_HDR:
case HTX_BLK_TLR:
-   ret.ptr = htx_get_blk_ptr(htx, blk) + (blk->info & 
0xff);
-   ret.len = (blk->info >> 8) & 0xf;
+   ret = ist2(htx_get_blk_ptr(htx, blk) + (blk->info & 
0xff),
+  (blk->info >> 8) & 0xf);
break;
 
case HTX_BLK_REQ_SL:
case HTX_BLK_RES_SL:
case HTX_BLK_DATA:
-   ret.ptr = htx_get_blk_ptr(htx, blk);
-   ret.len = blk->info & 0xfff;
+   ret = ist2(htx_get_blk_ptr(htx, blk),
+  blk->info & 0xfff);
break;
 
default:
diff --git a/src/hlua.c b/src/hlua.c
index 915356c09..17f0d4842 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -6315,8 +6315,7 @@ static int _hlua_http_msg_dup(struct http_msg *msg, 
lua_State *L, size_t offset,
 
case HTX_BLK_DATA:
v = htx_get_blk_value(htx, blk);
-   v.ptr += offset;
-   v.len -= offset;
+   v = istadv(v, offset);
if (v.len > len)
v.len = len;
 
@@ -6417,7 +6416,7 @@ static void _hlua_http_msg_delete(struct http_msg *msg, 
struct filter *filter, s
v.ptr += htxret.ret;
if (v.len > len)
v.len  = len;
-   blk = 

[PATCH 4/7] CLEANUP: Apply xalloc_size.cocci

2021-09-15 Thread Tim Duesterhus
This fixes a few locations with a hardcoded type within `sizeof()`.
---
 include/haproxy/listener.h | 2 +-
 src/errors.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/haproxy/listener.h b/include/haproxy/listener.h
index 1c37d34da..2212ca927 100644
--- a/include/haproxy/listener.h
+++ b/include/haproxy/listener.h
@@ -179,7 +179,7 @@ unsigned int bind_map_thread_id(const struct bind_conf 
*conf, unsigned int r);
 static inline struct bind_conf *bind_conf_alloc(struct proxy *fe, const char 
*file,
  int line, const char *arg, struct xprt_ops 
*xprt)
 {
-   struct bind_conf *bind_conf = calloc(1, sizeof(struct bind_conf));
+   struct bind_conf *bind_conf = calloc(1, sizeof(*bind_conf));
 
if (!bind_conf)
goto err;
diff --git a/src/errors.c b/src/errors.c
index d2eafcea4..993b6ae58 100644
--- a/src/errors.c
+++ b/src/errors.c
@@ -126,7 +126,7 @@ static void generate_usermsgs_ctx_str(void)
int ret;
 
if (unlikely(b_is_null(>str))) {
-   area = calloc(USERMSGS_CTX_BUFSIZE, sizeof(char));
+   area = calloc(USERMSGS_CTX_BUFSIZE, sizeof(*area));
if (area)
ctx->str = b_make(area, USERMSGS_CTX_BUFSIZE, 0, 0);
}
-- 
2.33.0




[PATCH 0/7] Coccinelle

2021-09-15 Thread Tim Duesterhus
Willy,

not sure about the "DOC" tag for the coccinelle patches and the placement
within the directory structure. Feel free to adjust.

Best regards

Tim Duesterhus (7):
  DOC: Add ist.cocci
  CLEANUP: Apply ist.cocci
  DOC: Add xalloc_size.cocci
  CLEANUP: Apply xalloc_size.cocci
  DOC: Add xalloc_cast.cocci
  DOC: Add bug_on.cocci
  CLEANUP: Apply bug_on.cocci

 addons/ot/src/http.c |  9 ++
 addons/promex/service-prometheus.c   |  2 +-
 include/haproxy/htx.h| 12 
 include/haproxy/listener.h   |  2 +-
 scripts/coccinelle/bug_on.cocci  |  7 +
 scripts/coccinelle/ist.cocci | 42 
 scripts/coccinelle/xalloc_cast.cocci | 11 
 scripts/coccinelle/xalloc_size.cocci | 41 +++
 src/errors.c |  2 +-
 src/hlua.c   |  5 ++--
 src/log.c|  3 +-
 src/quic_sock.c  |  3 +-
 src/stream.c |  2 +-
 13 files changed, 118 insertions(+), 23 deletions(-)
 create mode 100644 scripts/coccinelle/bug_on.cocci
 create mode 100644 scripts/coccinelle/ist.cocci
 create mode 100644 scripts/coccinelle/xalloc_cast.cocci
 create mode 100644 scripts/coccinelle/xalloc_size.cocci

-- 
2.33.0




[PATCH 3/7] DOC: Add xalloc_size.cocci

2021-09-15 Thread Tim Duesterhus
This commits the Coccinelle patch to clean up sizeof handling for malloc/calloc.
---
 scripts/coccinelle/xalloc_size.cocci | 41 
 1 file changed, 41 insertions(+)
 create mode 100644 scripts/coccinelle/xalloc_size.cocci

diff --git a/scripts/coccinelle/xalloc_size.cocci 
b/scripts/coccinelle/xalloc_size.cocci
new file mode 100644
index 0..80808e3d7
--- /dev/null
+++ b/scripts/coccinelle/xalloc_size.cocci
@@ -0,0 +1,41 @@
+@@
+type T;
+expression E;
+expression t;
+@@
+
+(
+  t = calloc(E, sizeof(*t))
+|
+- t = calloc(E, sizeof(T))
++ t = calloc(E, sizeof(*t))
+)
+
+@@
+type T;
+T *x;
+@@
+
+  x = malloc(
+- sizeof(T)
++ sizeof(*x)
+  )
+
+@@
+type T;
+T *x;
+@@
+
+  x = calloc(1,
+- sizeof(T)
++ sizeof(*x)
+  )
+
+@@
+@@
+
+  calloc(
++ 1,
+  ...
+- ,1
+  )
-- 
2.33.0




[PATCH 1/7] DOC: Add ist.cocci

2021-09-15 Thread Tim Duesterhus
This commits the Coccinelle patch to clean up ist handling.
---
 scripts/coccinelle/ist.cocci | 42 
 1 file changed, 42 insertions(+)
 create mode 100644 scripts/coccinelle/ist.cocci

diff --git a/scripts/coccinelle/ist.cocci b/scripts/coccinelle/ist.cocci
new file mode 100644
index 0..c3243302f
--- /dev/null
+++ b/scripts/coccinelle/ist.cocci
@@ -0,0 +1,42 @@
+@@
+struct ist i;
+expression p, l;
+@@
+
+- i.ptr = p;
+- i.len = l;
++ i = ist2(p, l);
+
+@@
+@@
+
+- ist2(NULL, 0)
++ IST_NULL
+
+@@
+struct ist i;
+expression e;
+@@
+
+- i.ptr += e;
+- i.len -= e;
++ i = istadv(i, e);
+
+@@
+struct ist i;
+@@
+
+- i.ptr != NULL
++ isttest(i)
+
+@@
+char *s;
+@@
+
+(
+- ist2(s, strlen(s))
++ ist(s)
+|
+- ist2(strdup(s), strlen(s))
++ ist(strdup(s))
+)
-- 
2.33.0




[PATCH] BUG/???: lua: Add missing call to RESET_SAFE_LJMP in hlua_filter_new()

2021-09-11 Thread Tim Duesterhus
In one case before exiting leaving the function the panic handler was not
reset.

Introduced in 69c581a09271e91d306e7b9080502a36abdc415e, which is 2.5+.
No backport required.
---
 src/hlua.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/hlua.c b/src/hlua.c
index 72d232491..915356c09 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -10005,6 +10005,7 @@ static int hlua_filter_new(struct stream *s, struct 
filter *filter)
/* Check stack size. */
if (!lua_checkstack(s->hlua->T, 1)) {
SEND_ERR(s->be, "Lua filter '%s': full stack.\n", 
conf->reg->name);
+   RESET_SAFE_LJMP(s->hlua);
ret = 0;
goto end;
}
-- 
2.33.0




[PATCH] CLEANUP: Move XXH3 macro from haproxy/compat.h to haproxy/xxhash.h

2021-09-11 Thread Tim Duesterhus
This moves all the xxhash functionality into a single location.

see d5fc8fcb86eb99831626051b3055bea7ca93a074
---
 include/haproxy/compat.h | 7 ---
 include/haproxy/xxhash.h | 7 +++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/haproxy/compat.h b/include/haproxy/compat.h
index 72dc3dc4f..886b7a365 100644
--- a/include/haproxy/compat.h
+++ b/include/haproxy/compat.h
@@ -288,13 +288,6 @@ typedef struct { } empty_t;
  */
 #define MAX_SEND_FD 253
 
-/* Make the new complex name for the xxhash function easier to remember
- * and use.
- */
-#ifndef XXH3
-#define XXH3(data, len, seed) XXH3_64bits_withSeed(data, len, seed)
-#endif
-
 #endif /* _HAPROXY_COMPAT_H */
 
 /*
diff --git a/include/haproxy/xxhash.h b/include/haproxy/xxhash.h
index 83a2fb71c..cd333e645 100644
--- a/include/haproxy/xxhash.h
+++ b/include/haproxy/xxhash.h
@@ -42,4 +42,11 @@
 
 #include 
 
+/* Make the new complex name for the xxhash function easier to remember
+ * and use.
+ */
+#ifndef XXH3
+#define XXH3(data, len, seed) XXH3_64bits_withSeed(data, len, seed)
+#endif
+
 #endif
-- 
2.33.0




[PATCH] CLEANUP: Add haproxy/xxhash.h to avoid modifying import/xxhash.h

2021-09-11 Thread Tim Duesterhus
This solves setting XXH_INLINE_ALL in a cleaner way, because the imported
header is not modified, easing future updates.

see 6f7cc11e6dd0f01b437fba893da2edd2362660a2
---
 addons/51degrees/51d.c |  2 +-
 admin/dyncookie/dyncookie.c|  3 ++-
 include/haproxy/activity.h |  2 +-
 include/haproxy/connection-t.h |  2 +-
 include/haproxy/defaults.h |  7 --
 include/haproxy/xxhash.h   | 45 ++
 include/import/xxhash.h|  2 --
 src/pattern.c  |  2 +-
 src/sample.c   |  2 +-
 src/server.c   |  3 +--
 src/server_state.c |  2 +-
 src/ssl_sock.c |  2 +-
 12 files changed, 55 insertions(+), 19 deletions(-)
 create mode 100644 include/haproxy/xxhash.h

diff --git a/addons/51degrees/51d.c b/addons/51degrees/51d.c
index 6dfa578e2..5d686953c 100644
--- a/addons/51degrees/51d.c
+++ b/addons/51degrees/51d.c
@@ -1,7 +1,6 @@
 #include 
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -15,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include <51Degrees.h>
 
 struct _51d_property_names {
diff --git a/admin/dyncookie/dyncookie.c b/admin/dyncookie/dyncookie.c
index 0c778eb7a..ddb71a748 100644
--- a/admin/dyncookie/dyncookie.c
+++ b/admin/dyncookie/dyncookie.c
@@ -14,7 +14,8 @@
 #include 
 #include 
 #include 
-#include 
+
+#include 
 
 __attribute__((noreturn)) void die(int code, const char *format, ...)
 {
diff --git a/include/haproxy/activity.h b/include/haproxy/activity.h
index 42569f203..66a5f3be7 100644
--- a/include/haproxy/activity.h
+++ b/include/haproxy/activity.h
@@ -22,11 +22,11 @@
 #ifndef _HAPROXY_ACTIVITY_H
 #define _HAPROXY_ACTIVITY_H
 
-#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 extern unsigned int profiling;
 extern unsigned long task_profiling_mask;
diff --git a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h
index ff8927dba..1afae9624 100644
--- a/include/haproxy/connection-t.h
+++ b/include/haproxy/connection-t.h
@@ -30,7 +30,6 @@
 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -38,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* referenced below */
 struct connection;
diff --git a/include/haproxy/defaults.h b/include/haproxy/defaults.h
index 16058817e..19a9cb623 100644
--- a/include/haproxy/defaults.h
+++ b/include/haproxy/defaults.h
@@ -404,13 +404,6 @@
 #define MAX_POLLERS10
 #endif
 
-/* Make all xxhash functions inline, with implementations being directly
- * included within xxhash.h.
- */
-#ifndef XXH_INLINE_ALL
-#define XXH_INLINE_ALL
-#endif
-
 /* system sysfs directory */
 #define NUMA_DETECT_SYSTEM_SYSFS_PATH "/sys/devices/system"
 
diff --git a/include/haproxy/xxhash.h b/include/haproxy/xxhash.h
new file mode 100644
index 0..83a2fb71c
--- /dev/null
+++ b/include/haproxy/xxhash.h
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2020 Dragan Dosen 
+ * Copyright (C) 2021 Tim Duesterhus 
+ *
+ * BSD 2-Clause License (https://www.opensource.org/licenses/bsd-license.php)
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ ** Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ ** Redistributions in binary form must reproduce the above
+ *  copyright notice, this list of conditions and the following disclaimer
+ *  in the documentation and/or other materials provided with the
+ *  distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _HAPROXY_XXHASH_H
+#define _HAPROXY_XXHASH_H
+
+/* Make all xxhash functions inline, with implementations being directly
+ * included within xxhash.h.
+ */
+#ifndef XXH_INLINE_ALL
+#define XXH_INLINE_ALL
+#else
+#error "XXH_INLINE_ALL is already defined."
+#endif
+
+#include 
+
+#endif
diff --git a/include/import/xxhash.h b/include/import/xxhash.h
index 24326bc33..d0e3e241e 100644
--- a/include/import/xxhash.h
+++ b/include/import/xxhash.h
@@ -75,8 +75,6 @@ XXH326.8 GB/s6.0 GB

[PATCH] CLEANUP: Add missing include guard to signal.h

2021-09-01 Thread Tim Duesterhus
Willy,

this also affects include/import/lru.h, include/import/xxhash.h, and
include/import/sha1.h. But I did not touch these, as they are within import/*

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
Found using GitHub's CodeQL scan.
---
 include/haproxy/signal.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/haproxy/signal.h b/include/haproxy/signal.h
index 763cfb0be..331b7fdc6 100644
--- a/include/haproxy/signal.h
+++ b/include/haproxy/signal.h
@@ -11,6 +11,9 @@
  *
  */
 
+#ifndef _HAPROXY_SIGNAL_H
+#define _HAPROXY_SIGNAL_H
+
 #include 
 
 #include 
@@ -39,6 +42,8 @@ static inline void signal_process_queue()
__signal_process_queue();
 }
 
+#endif /* _HAPROXY_SIGNAL_H */
+
 /*
  * Local variables:
  *  c-indent-level: 8
-- 
2.33.0




[PATCH] BUG/MINOR: tools: Fix loop condition in dump_text()

2021-08-28 Thread Tim Duesterhus
The condition should first check whether `bsize` is reached, before
dereferencing the offset. Even if this always works fine, due to the
string being null-terminated, this certainly looks odd.

Found using GitHub's CodeQL scan.

This bug traces back to at least 97c2ae13bc0d7961a348102d6719fbcaf34d46d5
(1.7.0+) and this patch should be backported accordingly.
---
 src/tools.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/tools.c b/src/tools.c
index 4f536efc3..cc71d4535 100644
--- a/src/tools.c
+++ b/src/tools.c
@@ -4522,9 +4522,9 @@ int may_access(const void *ptr)
 int dump_text(struct buffer *out, const char *buf, int bsize)
 {
unsigned char c;
-   int ptr = 0;
+   size_t ptr = 0;
 
-   while (buf[ptr] && ptr < bsize) {
+   while (ptr < bsize && buf[ptr]) {
c = buf[ptr];
if (isprint((unsigned char)c) && isascii((unsigned char)c) && c 
!= '\\' && c != ' ' && c != '=') {
if (out->data > out->size - 1)
-- 
2.33.0




[PATCH] BUG/???: threads: Use get_(local|gm)time instead of (local|gm)time

2021-08-28 Thread Tim Duesterhus
Willy,

please fill in the severity yourself.

Best regards
Tim Düsterhus

PS: sample_conv_http_date has a pretty messed up indentation, mixing tabs
and spaces. It probably should be cleaned up.

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
Using localtime / gmtime is not thread-safe, whereas the `get_*` wrappers are.

Found using GitHub's CodeQL scan.

The use in sample_conv_ltime() can be traced back to at least
fac9ccfb705702f211f99e67d5f5d5129002086a (first appearing in 1.6-dev3), so all
supported branches with thread support are affected.
---
 src/http_conv.c | 18 --
 src/sample.c| 20 
 2 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/src/http_conv.c b/src/http_conv.c
index dd1be1715..121a4989b 100644
--- a/src/http_conv.c
+++ b/src/http_conv.c
@@ -44,7 +44,7 @@ static int sample_conv_http_date(const struct arg *args, 
struct sample *smp, voi
const char day[7][4] = { "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", 
"Sat" };
const char mon[12][4] = { "Jan", "Feb", "Mar", "Apr", "May", "Jun", 
"Jul", "Aug", "Sep", "Oct", "Nov", "Dec" };
struct buffer *temp;
-   struct tm *tm;
+   struct tm tm;
int sec_frac = 0;
time_t curr_date;
 
@@ -66,23 +66,21 @@ static int sample_conv_http_date(const struct arg *args, 
struct sample *smp, voi
/* With high numbers, the date returned can be negative, the 55 bits 
mask prevent this. */
curr_date = smp->data.u.sint & 0x007fLL;
 
-   tm = gmtime(_date);
-   if (!tm)
-   return 0;
+   get_gmtime(curr_date, );
 
temp = get_trash_chunk();
if (args[1].type == ARGT_SINT && args[1].data.sint != TIME_UNIT_S) {
temp->data = snprintf(temp->area, temp->size - temp->data,
  "%s, %02d %s %04d %02d:%02d:%02d.%d GMT",
- day[tm->tm_wday], tm->tm_mday, 
mon[tm->tm_mon],
- 1900+tm->tm_year,
- tm->tm_hour, tm->tm_min, tm->tm_sec, 
sec_frac);
+ day[tm.tm_wday], tm.tm_mday, mon[tm.tm_mon],
+ 1900+tm.tm_year,
+ tm.tm_hour, tm.tm_min, tm.tm_sec, sec_frac);
} else {
temp->data = snprintf(temp->area, temp->size - temp->data,
  "%s, %02d %s %04d %02d:%02d:%02d GMT",
- day[tm->tm_wday], tm->tm_mday, 
mon[tm->tm_mon],
- 1900+tm->tm_year,
- tm->tm_hour, tm->tm_min, tm->tm_sec);
+ day[tm.tm_wday], tm.tm_mday, mon[tm.tm_mon],
+ 1900+tm.tm_year,
+ tm.tm_hour, tm.tm_min, tm.tm_sec);
 }
 
smp->data.u.str = *temp;
diff --git a/src/sample.c b/src/sample.c
index 6f0ddee20..b9d90ea0e 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -2300,18 +2300,16 @@ static int sample_conv_ltime(const struct arg *args, 
struct sample *smp, void *p
struct buffer *temp;
/* With high numbers, the date returned can be negative, the 55 bits 
mask prevent this. */
time_t curr_date = smp->data.u.sint & 0x007fLL;
-   struct tm *tm;
+   struct tm tm;
 
/* add offset */
if (args[1].type == ARGT_SINT)
curr_date += args[1].data.sint;
 
-   tm = localtime(_date);
-   if (!tm)
-   return 0;
+   get_localtime(curr_date, );
+
temp = get_trash_chunk();
-   temp->data = strftime(temp->area, temp->size, args[0].data.str.area,
- tm);
+   temp->data = strftime(temp->area, temp->size, args[0].data.str.area, 
);
smp->data.u.str = *temp;
smp->data.type = SMP_T_STR;
return 1;
@@ -2337,18 +2335,16 @@ static int sample_conv_utime(const struct arg *args, 
struct sample *smp, void *p
struct buffer *temp;
/* With high numbers, the date returned can be negative, the 55 bits 
mask prevent this. */
time_t curr_date = smp->data.u.sint & 0x007fLL;
-   struct tm *tm;
+   struct tm tm;
 
/* add offset */
if (args[1].type == ARGT_SINT)
curr_date += args[1].data.sint;
 
-   tm = gmtime(_date);
-   if (!tm)
-   return 0;
+   get_gmtime(curr_date, );
+
temp = get_trash_chunk();
-   temp->data = strftime(temp->area, temp->size, args[0].data.str.area,
- tm);
+   temp->data = strftime(temp->area, temp->size, args[0].data.str.area, 
);
smp->data.u.str = *temp;
smp->data.type = SMP_T_STR;
return 1;
-- 
2.33.0




[PATCH] REGTESTS: Remove REQUIRE_VERSION=1.5 from all tests

2021-08-25 Thread Tim Duesterhus
HAProxy 1.5 is EOL, thus this always matches.

1.6 / 1.7 were already removed in:
d8be0018fe85b5f815d59cdf1e0400274a99a9b1 (1.6)
1b095cac9468d0c3eeb157e9b1a2947487bd3c83 (1.7)
---
 reg-tests/ssl/ssl_frontend_samples.vtc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/reg-tests/ssl/ssl_frontend_samples.vtc 
b/reg-tests/ssl/ssl_frontend_samples.vtc
index 86851cf75..bca085674 100644
--- a/reg-tests/ssl/ssl_frontend_samples.vtc
+++ b/reg-tests/ssl/ssl_frontend_samples.vtc
@@ -1,7 +1,6 @@
 #REGTEST_TYPE=devel
 
 varnishtest "Test the ssl_f_* sample fetches"
-#REQUIRE_VERSION=1.5
 #REQUIRE_OPTIONS=OPENSSL
 feature ignore_unknown_macro
 
-- 
2.32.0




[PATCH] REGTESTS: Use `feature cmd` for 2.5+ tests

2021-08-25 Thread Tim Duesterhus
Using `REQUIRE_VERSION` is deprecated for tests targeting HAProxy with `-cc`
support.
---
 reg-tests/http-messaging/scheme_based_normalize.vtc | 2 +-
 reg-tests/server/cli_delete_server.vtc  | 3 +--
 reg-tests/server/cli_delete_server_lua.vtc  | 5 ++---
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/reg-tests/http-messaging/scheme_based_normalize.vtc 
b/reg-tests/http-messaging/scheme_based_normalize.vtc
index 568b5a8f8..4511a5fea 100644
--- a/reg-tests/http-messaging/scheme_based_normalize.vtc
+++ b/reg-tests/http-messaging/scheme_based_normalize.vtc
@@ -1,6 +1,6 @@
 varnishtest "scheme based normalization (rfc3982 6.3.2)"
-#REQUIRE_VERSION=2.5
 
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature ignore_unknown_macro
 
 syslog S1 -level info {
diff --git a/reg-tests/server/cli_delete_server.vtc 
b/reg-tests/server/cli_delete_server.vtc
index bfa9ff4a9..68f702bc9 100644
--- a/reg-tests/server/cli_delete_server.vtc
+++ b/reg-tests/server/cli_delete_server.vtc
@@ -3,10 +3,9 @@
 #
 varnishtest "Delete server via cli"
 
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature ignore_unknown_macro
 
-#REQUIRE_VERSION=2.5
-
 haproxy h1 -conf {
defaults
mode http
diff --git a/reg-tests/server/cli_delete_server_lua.vtc 
b/reg-tests/server/cli_delete_server_lua.vtc
index ab4200f0c..1b2473d76 100644
--- a/reg-tests/server/cli_delete_server_lua.vtc
+++ b/reg-tests/server/cli_delete_server_lua.vtc
@@ -2,11 +2,10 @@
 # cannot be removed at runtime.
 varnishtest "Delete lua server via cli"
 
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(LUA)'"
 feature ignore_unknown_macro
 
-#REQUIRE_VERSION=2.5
-#REQUIRE_OPTIONS=LUA
-
 server s1 {
rxreq
txresp
-- 
2.32.0




[PATCH] CI: Remove obsolete USE_SLZ=1 CI job

2021-08-14 Thread Tim Duesterhus
Using SLZ is a default, thus this build is equivalent to the "no features"
build.
---
 .github/matrix.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/matrix.py b/.github/matrix.py
index cfef53c9e..fdd76958c 100755
--- a/.github/matrix.py
+++ b/.github/matrix.py
@@ -94,7 +94,7 @@ for CC in ["gcc", "clang"]:
 }
 )
 
-for compression in ["USE_SLZ=1", "USE_ZLIB=1"]:
+for compression in ["USE_ZLIB=1"]:
 matrix.append(
 {
 "name": "{}, {}, gz={}".format(
-- 
2.32.0




[PATCH 1/1] DOC: Replace issue templates by issue forms

2021-06-23 Thread Tim Duesterhus
From: Tim Düsterhus 

GitHub's issue forms are the next evolution of issue templates and allow for
showing an actual form with separate inputs when creating an issue. They ensure
that all the required fields are filled in and automatically format code parts
(e.g. haproxy -vv or the configuration) as actual code blocks, possibly with
syntax highlighting.

Co-authored-by: Maximilian Mader 
---
 .github/ISSUE_TEMPLATE/Bug.md  | 103 ---
 .github/ISSUE_TEMPLATE/Bug.yml | 108 +
 .github/ISSUE_TEMPLATE/Code-Report.md  |  55 -
 .github/ISSUE_TEMPLATE/Code-Report.yml |  43 ++
 .github/ISSUE_TEMPLATE/Feature.md  |  55 -
 .github/ISSUE_TEMPLATE/Feature.yml |  50 
 .github/ISSUE_TEMPLATE/Question.md |  19 -
 7 files changed, 201 insertions(+), 232 deletions(-)
 delete mode 100644 .github/ISSUE_TEMPLATE/Bug.md
 create mode 100644 .github/ISSUE_TEMPLATE/Bug.yml
 delete mode 100644 .github/ISSUE_TEMPLATE/Code-Report.md
 create mode 100644 .github/ISSUE_TEMPLATE/Code-Report.yml
 delete mode 100644 .github/ISSUE_TEMPLATE/Feature.md
 create mode 100644 .github/ISSUE_TEMPLATE/Feature.yml
 delete mode 100644 .github/ISSUE_TEMPLATE/Question.md

diff --git a/.github/ISSUE_TEMPLATE/Bug.md b/.github/ISSUE_TEMPLATE/Bug.md
deleted file mode 100644
index 038330a6c..0
--- a/.github/ISSUE_TEMPLATE/Bug.md
+++ /dev/null
@@ -1,103 +0,0 @@

-name: Bug Report
-about: Report a problem with HAProxy to help us resolve it.
-labels: 'type: bug, status: needs-triage'

-
-
-
-
-## Detailed description of the problem
-
-
-
-## Expected behavior
-
-
-
-## Steps to reproduce the behavior
-
-
-
-1. 
-2. 
-3. 
-
-## Do you have any idea what may have caused this?
-
-## Do you have an idea how to solve the issue?
-
-## What is your configuration?
-
-
-
-```
-(paste your output here)
-```
-
-## Output of `haproxy -vv` and `uname -a`
-
-
-
-```
-(paste your output here)
-```
-
-## If HAProxy crashed: Last outputs and backtraces
-
-
-
-```
-(paste your output here)
-```
-
-## Additional information (if helpful)
-
-
diff --git a/.github/ISSUE_TEMPLATE/Bug.yml b/.github/ISSUE_TEMPLATE/Bug.yml
new file mode 100644
index 0..b56ecb710
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/Bug.yml
@@ -0,0 +1,108 @@
+name: Bug Report
+description: Report a problem with HAProxy to help us resolve it.
+labels:
+- 'type: bug'
+- 'status: needs-triage'
+body:
+- type: markdown
+  attributes:
+value: |
+  ## Welcome!
+
+  You are about to *report a bug* you encountered in HAProxy. Please use 
the 'Feature Request' template if you want to propose a new feature instead.
+
+  This issue tracker is used to track actual bugs. Please use [the 
forum](https://discourse.haproxy.org/) or mailing list if you have a question, 
e.g. to get help with building a configuration to achieve your desired behavior.
+
+  The forum is at: https://discourse.haproxy.org/
+
+  The mailing list (no need to subscribe) is: haproxy@formilux.org
+  Subscribe to the list: haproxy+subscr...@formilux.org
+  Unsubscribe from the list: haproxy+unsubscr...@formilux.org
+
+  Forum and mailing list are correct places for questions about HAProxy or 
general suggestions and topics, e.g. usage or documentation questions! This 
issue tracker is for tracking bugs and feature requests directly relating to 
the development of the software itself.
+
+  Thanks for understanding, and for contributing to the project!
+- type: textarea
+  id: description
+  attributes:
+label: Detailed Description of the Problem
+description: |
+  In this section, please try to concentrate on observations. Only 
describe what you observed directly.
+  validations:
+required: true
+- type: textarea
+  id: expected-behavior
+  attributes:
+label: Expected Behavior
+description: |
+  Explain why you consider the described behavior (above) to be wrong. 
What did you expect instead?
+
+  Most likely this is a mismatch between HAProxy's documentation and 
HAProxy's behavior.
+  validations:
+required: true
+- type: textarea
+  id: steps
+  attributes:
+label: Steps to Reproduce the Behavior
+description: |
+  The more time you spend describing an easy way to reproduce the behavior 
(if this is possible), the easier it is for the project developers to fix it!
+placeholder: |
+  1.
+  2.
+  3.
+  validations:
+required: true
+- type: textarea
+  id: possible-cause
+  attributes:
+label: Do you have any idea what may have caused this?
+description: |
+  Simply leave this empty if you do not.
+- type: textarea
+  id: possible-solution
+  attributes:
+label: Do you have an idea how to solve the issue?
+description: |
+  Simply leave this empty if you do not.
+- type: textarea
+  id: configuration
+  attributes:
+label: What is your configuration?
+description: |
+  - 

[PATCH 0/1] Replace issue templates by issue forms

2021-06-23 Thread Tim Duesterhus
Hi Willy, Lukas, List!

GitHub finally launched their next evolution of issue templates, called issue
forms, as a public beta: 
https://github.blog/changelog/2021-06-23-issues-forms-beta-for-public-repositories/

Instead of prefilling the issue creation form with some markdown that can be
ignored or accidentally deleted issue forms will create different textareas,
automatically formatting the issue correctly. The end result will be regular
markdown that can be edited as usual.

Beta implies that they might still slightly change in the future, possibly
requiring some further adjustments. However as the final result no longer
depends on the form itself we are not locking ourselves into some feature
for eternity.

Max and I worked together to migrate the existing templates to issue forms,
cleaning up the old stuff that is no longer required.

You can find an example bug report here:

https://github.com/TimWolla/haproxy/issues/7

It looks just like before!

The new forms can be tested here: 
https://github.com/TimWolla/haproxy/issues/new/choose.

I have deleted the old 'Question' template, because it no longer is required,
as the user can't simply delete the template from the field when there's
separate fields :-)

Best regards

Tim Düsterhus (1):
  DOC: Replace issue templates by issue forms

 .github/ISSUE_TEMPLATE/Bug.md  | 103 ---
 .github/ISSUE_TEMPLATE/Bug.yml | 108 +
 .github/ISSUE_TEMPLATE/Code-Report.md  |  55 -
 .github/ISSUE_TEMPLATE/Code-Report.yml |  43 ++
 .github/ISSUE_TEMPLATE/Feature.md  |  55 -
 .github/ISSUE_TEMPLATE/Feature.yml |  50 
 .github/ISSUE_TEMPLATE/Question.md |  19 -
 7 files changed, 201 insertions(+), 232 deletions(-)
 delete mode 100644 .github/ISSUE_TEMPLATE/Bug.md
 create mode 100644 .github/ISSUE_TEMPLATE/Bug.yml
 delete mode 100644 .github/ISSUE_TEMPLATE/Code-Report.md
 create mode 100644 .github/ISSUE_TEMPLATE/Code-Report.yml
 delete mode 100644 .github/ISSUE_TEMPLATE/Feature.md
 create mode 100644 .github/ISSUE_TEMPLATE/Feature.yml
 delete mode 100644 .github/ISSUE_TEMPLATE/Question.md

-- 
2.32.0




[PATCH] CLEANUP: Prevent channel-t.h from being detected as C++ by GitHub

2021-06-19 Thread Tim Duesterhus
GitHub uses github/linguist to determine the programming language used for each
source file to show statistics and to power the search. In cases of unique file
extensions this is easy, but for `.h` files the situation is less clear as they
are used for C, C++, Objective C and more. In these cases linguist makes use of
heuristics to determine the language.

One of these heuristics for C++ is that the file contains a line beginning with
`try`, only preceded by whitespace indentation. This heuristic matches the long
comment at the bottom of `channel-t.h`, as one sentence includes the word `try`
after a linebreak.

Fix this misdetection by changing the comment to follow the convention that all
lines start with an asterisk.
---
 include/haproxy/channel-t.h | 184 ++--
 1 file changed, 92 insertions(+), 92 deletions(-)

diff --git a/include/haproxy/channel-t.h b/include/haproxy/channel-t.h
index 8bd06e147..8feb62dd2 100644
--- a/include/haproxy/channel-t.h
+++ b/include/haproxy/channel-t.h
@@ -207,98 +207,98 @@ struct channel {
 
 
 /* Note about the channel structure
-
-   A channel stores information needed to reliably transport data in a single
-   direction. It stores status flags, timeouts, counters, subscribed analysers,
-   pointers to a data producer and to a data consumer, and information about
-   the amount of data which is allowed to flow directly from the producer to
-   the consumer without waking up the analysers.
-
-   A channel may buffer data into two locations :
- - a visible buffer (->buf)
- - an invisible buffer which right now consists in a pipe making use of
-   kernel buffers that cannot be tampered with.
-
-   Data stored into the first location may be analysed and altered by analysers
-   while data stored in pipes is only aimed at being transported from one
-   network socket to another one without being subject to memory copies. This
-   buffer may only be used when both the socket layer and the data layer of the
-   producer and the consumer support it, which typically is the case with Linux
-   splicing over sockets, and when there are enough data to be transported
-   without being analyzed (transport of TCP/HTTP payload or tunnelled data,
-   which is indicated by ->to_forward).
-
-   In order not to mix data streams, the producer may only feed the invisible
-   data with data to forward, and only when the visible buffer is empty. The
-   producer may not always be able to feed the invisible buffer due to platform
-   limitations (lack of kernel support).
-
-   Conversely, the consumer must always take data from the invisible data first
-   before ever considering visible data. There is no limit to the size of data
-   to consume from the invisible buffer, as platform-specific implementations
-   will rarely leave enough control on this. So any byte fed into the invisible
-   buffer is expected to reach the destination file descriptor, by any means.
-   However, it's the consumer's responsibility to ensure that the invisible
-   data has been entirely consumed before consuming visible data. This must be
-   reflected by ->pipe->data. This is very important as this and only this can
-   ensure strict ordering of data between buffers.
-
-   The producer is responsible for decreasing ->to_forward. The ->to_forward
-   parameter indicates how many bytes may be fed into either data buffer
-   without waking the parent up. The special value CHN_INFINITE_FORWARD is
-   never decreased nor increased.
-
-   The buf->o parameter says how many bytes may be consumed from the visible
-   buffer. This parameter is updated by any buffer_write() as well as any data
-   forwarded through the visible buffer. Since the ->to_forward attribute
-   applies to data after buf->p, an analyser will not see a buffer which has a
-   non-null ->to_forward with buf->i > 0. A producer is responsible for raising
-   buf->o by min(to_forward, buf->i) when it injects data into the buffer.
-
-   The consumer is responsible for decreasing ->buf->o when it sends data
-   from the visible buffer, and ->pipe->data when it sends data from the
-   invisible buffer.
-
-   A real-world example consists in part in an HTTP response waiting in a
-   buffer to be forwarded. We know the header length (300) and the amount of
-   data to forward (content-length=9000). The buffer already contains 1000
-   bytes of data after the 300 bytes of headers. Thus the caller will set
-   buf->o to 300 indicating that it explicitly wants to send those data, and
-   set ->to_forward to 9000 (content-length). This value must be normalised
-   immediately after updating ->to_forward : since there are already 1300 bytes
-   in the buffer, 300 of which are already counted in buf->o, and that size
-   is smaller than ->to_forward, we must update buf->o to 1300 to flush the
-   whole buffer, and reduce ->to_forward to 8000. After that, the producer may
-   try to feed the additional data through the 

[PATCH] CI: Replace the requirement for 'sudo' with a call to 'ulimit -n'

2021-06-13 Thread Tim Duesterhus
Using 'sudo' required quite a few workarounds in various places. Setting an
explicit 'ulimit -n' removes the requirement for 'sudo', resulting in a cleaner
workflow configuration.
---
 .github/workflows/vtest.yml | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/vtest.yml b/.github/workflows/vtest.yml
index 786713374..1dc216eeb 100644
--- a/.github/workflows/vtest.yml
+++ b/.github/workflows/vtest.yml
@@ -99,13 +99,14 @@ jobs:
   run: echo "::add-matcher::.github/vtest.json"
 - name: Run VTest for HAProxy ${{ steps.show-version.outputs.version }}
   id: vtest
-  # sudo is required, because macOS fails due to an open files limit.
-  run: sudo make reg-tests VTEST_PROGRAM=../vtest/vtest 
REGTESTS_TYPES=default,bug,devel
+  run: |
+# This is required for macOS which does not actually allow to increase
+# the '-n' soft limit to the hard limit, thus failing to run.
+ulimit -n 5000
+make reg-tests VTEST_PROGRAM=../vtest/vtest 
REGTESTS_TYPES=default,bug,devel
 - name: Show results
   if: ${{ failure() }}
-  # The chmod / sudo is necessary due to the `sudo` while running the 
tests.
   run: |
-sudo chmod a+rX ${TMPDIR}/haregtests-*/
 for folder in ${TMPDIR}/haregtests-*/vtc.*; do
   printf "::group::"
   cat $folder/INFO
@@ -115,6 +116,6 @@ jobs:
 shopt -s nullglob
 for asan in asan.log*; do
   echo "::group::$asan"
-  sudo cat $asan
+  cat $asan
   echo "::endgroup::"
 done
-- 
2.32.0




[PATCH 1/4] REGTESTS: Replace REQUIRE_VERSION=2.5 with 'haproxy -cc'

2021-06-11 Thread Tim Duesterhus
This is safe, because running `haproxy -cc 'version_atleast(2.5-dev0)'` on
HAProxy 2.4 will also result in an exit code of 1.
---
 reg-tests/http-messaging/http_abortonclose.vtc | 2 +-
 reg-tests/ssl/new_del_ssl_cafile.vtc   | 2 +-
 reg-tests/ssl/new_del_ssl_crlfile.vtc  | 2 +-
 reg-tests/ssl/set_ssl_cafile.vtc   | 2 +-
 reg-tests/ssl/set_ssl_crlfile.vtc  | 2 +-
 reg-tests/ssl/show_ssl_ocspresponse.vtc| 2 +-
 reg-tests/startup/check_condition.vtc  | 1 -
 7 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/reg-tests/http-messaging/http_abortonclose.vtc 
b/reg-tests/http-messaging/http_abortonclose.vtc
index 49cdaeadd..c083f29f1 100644
--- a/reg-tests/http-messaging/http_abortonclose.vtc
+++ b/reg-tests/http-messaging/http_abortonclose.vtc
@@ -4,7 +4,7 @@ feature ignore_unknown_macro
 # NOTE : This test may fail if too many vtest are running in parallel because
 #the port reserved for closed s1 server may be reused by another vtest
 
-#REQUIRE_VERSION=2.5
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 #REGTEST_TYPE=slow
 
 # b1 : Don't send /c4 before /c3 was received by s2 server
diff --git a/reg-tests/ssl/new_del_ssl_cafile.vtc 
b/reg-tests/ssl/new_del_ssl_cafile.vtc
index 613e12866..f943a6632 100644
--- a/reg-tests/ssl/new_del_ssl_cafile.vtc
+++ b/reg-tests/ssl/new_del_ssl_cafile.vtc
@@ -9,7 +9,7 @@
 # - Check that you have socat
 
 varnishtest "Test the 'new ssl ca-file' and 'del ssl ca-file' commands of the 
CLI"
-#REQUIRE_VERSION=2.5
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 #REQUIRE_OPTIONS=OPENSSL
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
diff --git a/reg-tests/ssl/new_del_ssl_crlfile.vtc 
b/reg-tests/ssl/new_del_ssl_crlfile.vtc
index 0d82200e7..027f00096 100644
--- a/reg-tests/ssl/new_del_ssl_crlfile.vtc
+++ b/reg-tests/ssl/new_del_ssl_crlfile.vtc
@@ -9,7 +9,7 @@
 # - Check that you have socat
 
 varnishtest "Test the 'new ssl crl-file' and 'del ssl crl-file' commands of 
the CLI"
-#REQUIRE_VERSION=2.5
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 #REQUIRE_OPTIONS=OPENSSL
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
diff --git a/reg-tests/ssl/set_ssl_cafile.vtc b/reg-tests/ssl/set_ssl_cafile.vtc
index eb625639b..2f4ef2e8a 100644
--- a/reg-tests/ssl/set_ssl_cafile.vtc
+++ b/reg-tests/ssl/set_ssl_cafile.vtc
@@ -15,7 +15,7 @@
 # - Check that you have socat
 
 varnishtest "Test the 'set ssl ca-file' feature of the CLI"
-#REQUIRE_VERSION=2.5
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 #REQUIRE_OPTIONS=OPENSSL
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
diff --git a/reg-tests/ssl/set_ssl_crlfile.vtc 
b/reg-tests/ssl/set_ssl_crlfile.vtc
index 7060a1477..d3aad9065 100644
--- a/reg-tests/ssl/set_ssl_crlfile.vtc
+++ b/reg-tests/ssl/set_ssl_crlfile.vtc
@@ -18,7 +18,7 @@
 # - Check that you have socat
 
 varnishtest "Test the 'set ssl crl-file' feature of the CLI"
-#REQUIRE_VERSION=2.5
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 #REQUIRE_OPTIONS=OPENSSL
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
diff --git a/reg-tests/ssl/show_ssl_ocspresponse.vtc 
b/reg-tests/ssl/show_ssl_ocspresponse.vtc
index b7a2d5105..a942f2c47 100644
--- a/reg-tests/ssl/show_ssl_ocspresponse.vtc
+++ b/reg-tests/ssl/show_ssl_ocspresponse.vtc
@@ -19,7 +19,7 @@
 # - Check that you have socat
 
 varnishtest "Test the 'show ssl ocsp-response' and 'show ssl cert 
foo.pem.ocsp' features of the CLI"
-#REQUIRE_VERSION=2.5
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 #REQUIRE_OPTIONS=OPENSSL
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
diff --git a/reg-tests/startup/check_condition.vtc 
b/reg-tests/startup/check_condition.vtc
index 7d3324bfe..d56d73fde 100644
--- a/reg-tests/startup/check_condition.vtc
+++ b/reg-tests/startup/check_condition.vtc
@@ -1,6 +1,5 @@
 varnishtest "Tests the -cc argument"
 
-#REQUIRE_VERSION=2.5
 feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 
 shell {
-- 
2.32.0




[PATCH 0/4] Use 'feature cmd' in regtests

2021-06-11 Thread Tim Duesterhus
Hi!

I hope I added all the active developers that touch the reg-tests to the 'CC'
list :-)

This series updates the regtests to make use of VTest's 'feature cmd' syntax
to skip tests that are not supported in the current environment.

In the long run this will should result in much cleaner tests, because they
don't need to be parsed by run-regtests.sh to extract these marker comments. It
also allows to easily add test specific requirements without needing to invent
an entirely new syntax. An example might be the recently added tests that are
not supported on BoringSSL. These should be able to get a condition like:

feature cmd "$HAPROXY_PROGRAM -vv |grep -vq BoringSSL"

and then they won't run for BoringSSL (the example is untested).

After this series is applied the output of 'make reg-tests' will change. Tests
that are excluded using 'feature cmd' will still be listed as "Add test" in
the test gathering section. However the final line will show that a few tests
have been skipped:

0 tests failed, 4 tests skipped, 105 tests passed

I don't think this is going to be an issue. But if it is, please complain!

If all of you are happy with the series then it might be merged. Just remember
to use 'feature cmd' after it is applied :-)

Best regards

Tim Düsterhus (4):
  REGTESTS: Replace REQUIRE_VERSION=2.5 with 'haproxy -cc'
  REGTESTS: Replace REQUIRE_OPTIONS with 'haproxy -cc' for 2.5+ tests
  REGTESTS: Replace REQUIRE_BINARIES with 'command -v'
  REGTESTS: Remove support for REQUIRE_BINARIES

 reg-tests/http-messaging/http_abortonclose.vtc |  2 +-
 reg-tests/mcli/mcli_start_progs.vtc|  2 +-
 reg-tests/ssl/add_ssl_crt-list.vtc |  2 +-
 reg-tests/ssl/new_del_ssl_cafile.vtc   |  6 +++---
 reg-tests/ssl/new_del_ssl_crlfile.vtc  |  6 +++---
 reg-tests/ssl/set_ssl_cafile.vtc   |  6 +++---
 reg-tests/ssl/set_ssl_cert.vtc |  2 +-
 reg-tests/ssl/set_ssl_cert_bundle.vtc  |  2 +-
 reg-tests/ssl/set_ssl_cert_noext.vtc   |  2 +-
 reg-tests/ssl/set_ssl_crlfile.vtc  |  6 +++---
 reg-tests/ssl/set_ssl_server_cert.vtc  |  2 +-
 reg-tests/ssl/show_ssl_ocspresponse.vtc|  6 +++---
 reg-tests/startup/check_condition.vtc  |  1 -
 scripts/run-regtests.sh| 12 
 14 files changed, 22 insertions(+), 35 deletions(-)

-- 
2.32.0




[PATCH 3/4] REGTESTS: Replace REQUIRE_BINARIES with 'command -v'

2021-06-11 Thread Tim Duesterhus
This migrates the tests to the native `feature cmd` functionality of VTest.
---
 reg-tests/mcli/mcli_start_progs.vtc | 2 +-
 reg-tests/ssl/add_ssl_crt-list.vtc  | 2 +-
 reg-tests/ssl/new_del_ssl_cafile.vtc| 2 +-
 reg-tests/ssl/new_del_ssl_crlfile.vtc   | 2 +-
 reg-tests/ssl/set_ssl_cafile.vtc| 2 +-
 reg-tests/ssl/set_ssl_cert.vtc  | 2 +-
 reg-tests/ssl/set_ssl_cert_bundle.vtc   | 2 +-
 reg-tests/ssl/set_ssl_cert_noext.vtc| 2 +-
 reg-tests/ssl/set_ssl_crlfile.vtc   | 2 +-
 reg-tests/ssl/set_ssl_server_cert.vtc   | 2 +-
 reg-tests/ssl/show_ssl_ocspresponse.vtc | 2 +-
 11 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/reg-tests/mcli/mcli_start_progs.vtc 
b/reg-tests/mcli/mcli_start_progs.vtc
index 08de157f1..eb6f63502 100644
--- a/reg-tests/mcli/mcli_start_progs.vtc
+++ b/reg-tests/mcli/mcli_start_progs.vtc
@@ -1,7 +1,7 @@
 varnishtest "Try to start a master CLI with 2 programs"
 #REGTEST_TYPE=bug
 #REQUIRE_VERSION=2.0
-#REQUIRE_BINARIES=sleep
+feature cmd "command -v sleep"
 
 feature ignore_unknown_macro
 
diff --git a/reg-tests/ssl/add_ssl_crt-list.vtc 
b/reg-tests/ssl/add_ssl_crt-list.vtc
index ca5228501..7aae2338a 100644
--- a/reg-tests/ssl/add_ssl_crt-list.vtc
+++ b/reg-tests/ssl/add_ssl_crt-list.vtc
@@ -13,7 +13,7 @@
 varnishtest "Test the 'add ssl crt-list' feature of the CLI"
 #REQUIRE_VERSION=2.2
 #REQUIRE_OPTIONS=OPENSSL
-#REQUIRE_BINARIES=socat
+feature cmd "command -v socat"
 feature ignore_unknown_macro
 
 server s1 -repeat 2 {
diff --git a/reg-tests/ssl/new_del_ssl_cafile.vtc 
b/reg-tests/ssl/new_del_ssl_cafile.vtc
index 536db5007..1b5bef1a4 100644
--- a/reg-tests/ssl/new_del_ssl_cafile.vtc
+++ b/reg-tests/ssl/new_del_ssl_cafile.vtc
@@ -11,7 +11,7 @@
 varnishtest "Test the 'new ssl ca-file' and 'del ssl ca-file' commands of the 
CLI"
 feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
-#REQUIRE_BINARIES=socat
+feature cmd "command -v socat"
 feature ignore_unknown_macro
 
 server s1 -repeat 2 {
diff --git a/reg-tests/ssl/new_del_ssl_crlfile.vtc 
b/reg-tests/ssl/new_del_ssl_crlfile.vtc
index eeed09caf..54bbdc239 100644
--- a/reg-tests/ssl/new_del_ssl_crlfile.vtc
+++ b/reg-tests/ssl/new_del_ssl_crlfile.vtc
@@ -11,7 +11,7 @@
 varnishtest "Test the 'new ssl crl-file' and 'del ssl crl-file' commands of 
the CLI"
 feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
-#REQUIRE_BINARIES=socat
+feature cmd "command -v socat"
 feature ignore_unknown_macro
 
 server s1 -repeat 3 {
diff --git a/reg-tests/ssl/set_ssl_cafile.vtc b/reg-tests/ssl/set_ssl_cafile.vtc
index 5dcfaf989..72ce3e6dc 100644
--- a/reg-tests/ssl/set_ssl_cafile.vtc
+++ b/reg-tests/ssl/set_ssl_cafile.vtc
@@ -17,7 +17,7 @@
 varnishtest "Test the 'set ssl ca-file' feature of the CLI"
 feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
-#REQUIRE_BINARIES=socat
+feature cmd "command -v socat"
 feature ignore_unknown_macro
 
 server s1 -repeat 4 {
diff --git a/reg-tests/ssl/set_ssl_cert.vtc b/reg-tests/ssl/set_ssl_cert.vtc
index c4d088306..85684bc3e 100644
--- a/reg-tests/ssl/set_ssl_cert.vtc
+++ b/reg-tests/ssl/set_ssl_cert.vtc
@@ -22,7 +22,7 @@
 varnishtest "Test the 'set ssl cert' feature of the CLI"
 #REQUIRE_VERSION=2.2
 #REQUIRE_OPTIONS=OPENSSL
-#REQUIRE_BINARIES=socat
+feature cmd "command -v socat"
 feature ignore_unknown_macro
 
 server s1 -repeat 9 {
diff --git a/reg-tests/ssl/set_ssl_cert_bundle.vtc 
b/reg-tests/ssl/set_ssl_cert_bundle.vtc
index aaec89dda..218f7bfb4 100644
--- a/reg-tests/ssl/set_ssl_cert_bundle.vtc
+++ b/reg-tests/ssl/set_ssl_cert_bundle.vtc
@@ -17,7 +17,7 @@
 varnishtest "Test the 'set ssl cert' feature of the CLI with bundles"
 #REQUIRE_VERSION=2.3
 #REQUIRE_OPTIONS=OPENSSL
-#REQUIRE_BINARIES=socat
+feature cmd "command -v socat"
 feature ignore_unknown_macro
 
 server s1 -repeat 9 {
diff --git a/reg-tests/ssl/set_ssl_cert_noext.vtc 
b/reg-tests/ssl/set_ssl_cert_noext.vtc
index f1c42ff84..b7bafa8a3 100644
--- a/reg-tests/ssl/set_ssl_cert_noext.vtc
+++ b/reg-tests/ssl/set_ssl_cert_noext.vtc
@@ -14,7 +14,7 @@
 varnishtest "Test the 'set ssl cert' feature of the CLI with separate key and 
crt"
 #REQUIRE_VERSION=2.2
 #REQUIRE_OPTIONS=OPENSSL
-#REQUIRE_BINARIES=socat
+feature cmd "command -v socat"
 feature ignore_unknown_macro
 
 server s1 -repeat 3 {
diff --git a/reg-tests/ssl/set_ssl_crlfile.vtc 
b/reg-tests/ssl/set_ssl_crlfile.vtc
index 4ee7c1225..f6d97ce6b 100644
--- a/reg-tests/ssl/set_ssl_crlfile.vtc
+++ b/reg-tests/ssl/set_ssl_crlfile.vtc
@@ -20,7 +20,7 @@
 varnishtest "Test the 'set ssl crl-file' feature of the CLI"
 feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
-#REQUIRE_BINARIES=socat
+feature cmd "command -v socat"
 feature ignore_unknown_macro
 
 server s1 -repeat 4 {
diff --git 

[PATCH 2/4] REGTESTS: Replace REQUIRE_OPTIONS with 'haproxy -cc' for 2.5+ tests

2021-06-11 Thread Tim Duesterhus
This migrates the tests for HAProxy versions that support '-cc' to the native
VTest functionality.
---
 reg-tests/ssl/new_del_ssl_cafile.vtc| 2 +-
 reg-tests/ssl/new_del_ssl_crlfile.vtc   | 2 +-
 reg-tests/ssl/set_ssl_cafile.vtc| 2 +-
 reg-tests/ssl/set_ssl_crlfile.vtc   | 2 +-
 reg-tests/ssl/show_ssl_ocspresponse.vtc | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/reg-tests/ssl/new_del_ssl_cafile.vtc 
b/reg-tests/ssl/new_del_ssl_cafile.vtc
index f943a6632..536db5007 100644
--- a/reg-tests/ssl/new_del_ssl_cafile.vtc
+++ b/reg-tests/ssl/new_del_ssl_cafile.vtc
@@ -10,7 +10,7 @@
 
 varnishtest "Test the 'new ssl ca-file' and 'del ssl ca-file' commands of the 
CLI"
 feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
-#REQUIRE_OPTIONS=OPENSSL
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
 
diff --git a/reg-tests/ssl/new_del_ssl_crlfile.vtc 
b/reg-tests/ssl/new_del_ssl_crlfile.vtc
index 027f00096..eeed09caf 100644
--- a/reg-tests/ssl/new_del_ssl_crlfile.vtc
+++ b/reg-tests/ssl/new_del_ssl_crlfile.vtc
@@ -10,7 +10,7 @@
 
 varnishtest "Test the 'new ssl crl-file' and 'del ssl crl-file' commands of 
the CLI"
 feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
-#REQUIRE_OPTIONS=OPENSSL
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
 
diff --git a/reg-tests/ssl/set_ssl_cafile.vtc b/reg-tests/ssl/set_ssl_cafile.vtc
index 2f4ef2e8a..5dcfaf989 100644
--- a/reg-tests/ssl/set_ssl_cafile.vtc
+++ b/reg-tests/ssl/set_ssl_cafile.vtc
@@ -16,7 +16,7 @@
 
 varnishtest "Test the 'set ssl ca-file' feature of the CLI"
 feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
-#REQUIRE_OPTIONS=OPENSSL
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
 
diff --git a/reg-tests/ssl/set_ssl_crlfile.vtc 
b/reg-tests/ssl/set_ssl_crlfile.vtc
index d3aad9065..4ee7c1225 100644
--- a/reg-tests/ssl/set_ssl_crlfile.vtc
+++ b/reg-tests/ssl/set_ssl_crlfile.vtc
@@ -19,7 +19,7 @@
 
 varnishtest "Test the 'set ssl crl-file' feature of the CLI"
 feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
-#REQUIRE_OPTIONS=OPENSSL
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
 
diff --git a/reg-tests/ssl/show_ssl_ocspresponse.vtc 
b/reg-tests/ssl/show_ssl_ocspresponse.vtc
index a942f2c47..29673a525 100644
--- a/reg-tests/ssl/show_ssl_ocspresponse.vtc
+++ b/reg-tests/ssl/show_ssl_ocspresponse.vtc
@@ -20,7 +20,7 @@
 
 varnishtest "Test the 'show ssl ocsp-response' and 'show ssl cert 
foo.pem.ocsp' features of the CLI"
 feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
-#REQUIRE_OPTIONS=OPENSSL
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
 
-- 
2.32.0




[PATCH 4/4] REGTESTS: Remove support for REQUIRE_BINARIES

2021-06-11 Thread Tim Duesterhus
This is no longer used since the migration to the native `feature cmd`
functionality.
---
 scripts/run-regtests.sh | 12 
 1 file changed, 12 deletions(-)

diff --git a/scripts/run-regtests.sh b/scripts/run-regtests.sh
index b542f24f8..6eadc06c7 100755
--- a/scripts/run-regtests.sh
+++ b/scripts/run-regtests.sh
@@ -56,9 +56,6 @@ _help()
 #REQUIRE_VERSION=0.0
 #REQUIRE_VERSION_BELOW=99.9
 
-# To define required binaries for a test:
-#REQUIRE_BINARIES=socat,curl
-
   Configure environment variables to set the haproxy and vtest binaries to use
 setenv HAPROXY_PROGRAM /usr/local/sbin/haproxy
 setenv VTEST_PROGRAM /usr/local/bin/vtest
@@ -128,7 +125,6 @@ _findtests() {
 require_options="$(sed -ne 's/^#REQUIRE_OPTIONS=//p' "$i" | sed  -e 's/,/ 
/g')"
 require_services="$(sed -ne 's/^#REQUIRE_SERVICES=//p' "$i" | sed  -e 
's/,/ /g')"
 exclude_targets="$(sed -ne 's/^#EXCLUDE_TARGETS=//p' "$i" | sed  -e 's/,/ 
/g')"
-require_binaries="$(sed -ne 's/^#REQUIRE_BINARIES=//p' "$i" | sed  -e 
's/,/ /g')"
 if [ $any_test -ne 1 ] ; then
 regtest_type="$(sed -ne 's/^#REGTEST_TYPE=//p' "$i")"
 if [ -z $regtest_type ] ; then
@@ -205,14 +201,6 @@ _findtests() {
   fi
 done
 
-for requiredbin in $require_binaries; do
-  if ! command -v $requiredbin >/dev/null 2>&1
-  then
-echo "  Skip $i because '"$requiredbin"' is not installed"
-skiptest=1
-  fi
-done
-
 if [ -z $skiptest ]; then
   echo "  Add test: $i"
   testlist="$testlist $i"
-- 
2.32.0




[PATCH 1/2] REGTESTS: Remove REQUIRE_VERSION=1.6 from all tests

2021-06-11 Thread Tim Duesterhus
HAProxy 1.6 is EOL, thus this always matches.
---
 reg-tests/compression/lua_validation.vtc  | 1 -
 reg-tests/converter/json.vtc  | 1 -
 reg-tests/converter/url_dec.vtc   | 1 -
 reg-tests/http-messaging/h1_to_h1.vtc | 1 -
 reg-tests/http-messaging/http_request_buffer.vtc  | 1 -
 reg-tests/http-rules/h1_to_h1c.vtc| 1 -
 reg-tests/http-rules/map_redirect.vtc | 1 -
 reg-tests/lua/lua_socket.vtc  | 1 -
 reg-tests/mailers/healthcheckmail.vtc | 1 -
 reg-tests/ssl/ssl_client_auth.vtc | 1 -
 reg-tests/webstats/webstats-scope-and-post-change.vtc | 1 -
 11 files changed, 11 deletions(-)

diff --git a/reg-tests/compression/lua_validation.vtc 
b/reg-tests/compression/lua_validation.vtc
index d001c2d32..b10cbd93e 100644
--- a/reg-tests/compression/lua_validation.vtc
+++ b/reg-tests/compression/lua_validation.vtc
@@ -1,7 +1,6 @@
 # Checks that compression doesn't cause corruption..
 
 varnishtest "Compression validation"
-#REQUIRE_VERSION=1.6
 #REQUIRE_OPTIONS=ZLIB|SLZ,LUA,OPENSSL
 #REGTEST_TYPE=slow
 
diff --git a/reg-tests/converter/json.vtc b/reg-tests/converter/json.vtc
index efac8f622..b1c5f3800 100644
--- a/reg-tests/converter/json.vtc
+++ b/reg-tests/converter/json.vtc
@@ -1,6 +1,5 @@
 varnishtest "json converter test"
 
-#REQUIRE_VERSION=1.6
 
 feature ignore_unknown_macro
 
diff --git a/reg-tests/converter/url_dec.vtc b/reg-tests/converter/url_dec.vtc
index 464c35a27..9db3b64ef 100644
--- a/reg-tests/converter/url_dec.vtc
+++ b/reg-tests/converter/url_dec.vtc
@@ -1,6 +1,5 @@
 varnishtest "url_dec converter Test"
 
-#REQUIRE_VERSION=1.6
 
 feature ignore_unknown_macro
 
diff --git a/reg-tests/http-messaging/h1_to_h1.vtc 
b/reg-tests/http-messaging/h1_to_h1.vtc
index 4a442c79d..8d784d681 100644
--- a/reg-tests/http-messaging/h1_to_h1.vtc
+++ b/reg-tests/http-messaging/h1_to_h1.vtc
@@ -1,5 +1,4 @@
 varnishtest "HTTP request tests: H1 to H1 (HTX mode supported only for HAProxy 
>= 1.9)"
-#REQUIRE_VERSION=1.6
 
 # Run it with HAPROXY_PROGRAM=$PWD/haproxy varnishtest -l -k -t 1 "$1"
 
diff --git a/reg-tests/http-messaging/http_request_buffer.vtc 
b/reg-tests/http-messaging/http_request_buffer.vtc
index 1ac04c7ac..d9cc6aec8 100644
--- a/reg-tests/http-messaging/http_request_buffer.vtc
+++ b/reg-tests/http-messaging/http_request_buffer.vtc
@@ -1,7 +1,6 @@
 varnishtest "A test for http-request-buffer option"
 feature ignore_unknown_macro
 
-#REQUIRE_VERSION=1.6
 
 # This test checks HTTP request buffering feature.
 # We run one server s1 which can serve only one client (no -repeat argument 
here).
diff --git a/reg-tests/http-rules/h1_to_h1c.vtc 
b/reg-tests/http-rules/h1_to_h1c.vtc
index aedb41ff7..5ae1f9335 100644
--- a/reg-tests/http-rules/h1_to_h1c.vtc
+++ b/reg-tests/http-rules/h1_to_h1c.vtc
@@ -1,5 +1,4 @@
 varnishtest "Composite HTTP manipulation test (H1 clear to H1 clear)"
-#REQUIRE_VERSION=1.6
 #REQUIRE_VERSION_BELOW=1.9
 
 # This config tests several http-request features and their interactions.
diff --git a/reg-tests/http-rules/map_redirect.vtc 
b/reg-tests/http-rules/map_redirect.vtc
index f6a0eeb2d..77e9b0dc3 100644
--- a/reg-tests/http-rules/map_redirect.vtc
+++ b/reg-tests/http-rules/map_redirect.vtc
@@ -2,7 +2,6 @@ varnishtest "haproxy host header: map / redirect tests"
 #REQUIRE_OPTIONS=PCRE|PCRE2
 feature ignore_unknown_macro
 
-#REQUIRE_VERSION=1.6
 
 server s1 {
rxreq
diff --git a/reg-tests/lua/lua_socket.vtc b/reg-tests/lua/lua_socket.vtc
index 2e126f5c1..83e06a63d 100644
--- a/reg-tests/lua/lua_socket.vtc
+++ b/reg-tests/lua/lua_socket.vtc
@@ -2,7 +2,6 @@ varnishtest "Lua: check socket functionality from a lua-task"
 feature ignore_unknown_macro
 
 #REQUIRE_OPTIONS=LUA
-#REQUIRE_VERSION=1.6
 
 server s1 {
 rxreq
diff --git a/reg-tests/mailers/healthcheckmail.vtc 
b/reg-tests/mailers/healthcheckmail.vtc
index ce3335f9b..75325080b 100644
--- a/reg-tests/mailers/healthcheckmail.vtc
+++ b/reg-tests/mailers/healthcheckmail.vtc
@@ -1,6 +1,5 @@
 varnishtest "Lua: txn:get_priv() scope"
 #REQUIRE_OPTIONS=LUA
-#REQUIRE_VERSION=1.6
 #REGTEST_TYPE=broken
 
 feature ignore_unknown_macro
diff --git a/reg-tests/ssl/ssl_client_auth.vtc 
b/reg-tests/ssl/ssl_client_auth.vtc
index f82be7b16..885302e47 100644
--- a/reg-tests/ssl/ssl_client_auth.vtc
+++ b/reg-tests/ssl/ssl_client_auth.vtc
@@ -15,7 +15,6 @@
 # 
https://www.haproxy.com/blog/ssl-client-certificate-management-at-application-level/
 
 varnishtest "Test the client auth"
-#REQUIRE_VERSION=1.6
 #REQUIRE_OPTIONS=OPENSSL
 feature ignore_unknown_macro
 
diff --git a/reg-tests/webstats/webstats-scope-and-post-change.vtc 
b/reg-tests/webstats/webstats-scope-and-post-change.vtc
index b26356fef..b88287a7a 100644
--- a/reg-tests/webstats/webstats-scope-and-post-change.vtc
+++ b/reg-tests/webstats/webstats-scope-and-post-change.vtc
@@ -1,5 +1,4 @@
 varnishtest "Webgui stats page check 

[PATCH 2/2] REGTESTS: Remove REQUIRE_VERSION=1.7 from all tests

2021-06-11 Thread Tim Duesterhus
HAProxy 1.7 is the lowest supported version, thus this always matches.
---
 reg-tests/http-rules/map_regm_with_backref.vtc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/reg-tests/http-rules/map_regm_with_backref.vtc 
b/reg-tests/http-rules/map_regm_with_backref.vtc
index 7a5b879d2..78af44721 100644
--- a/reg-tests/http-rules/map_regm_with_backref.vtc
+++ b/reg-tests/http-rules/map_regm_with_backref.vtc
@@ -10,7 +10,6 @@
 varnishtest "map_regm get_trash_chunk test"
 feature ignore_unknown_macro
 
-#REQUIRE_VERSION=1.7
 #REGTEST_TYPE=bug
 
 syslog S1 -level notice {
-- 
2.32.0




[PATCH] CI: Make matrix.py executable and add shebang

2021-06-08 Thread Tim Duesterhus
It's a script, allow executing this as a script without needing to invoke
`python3` manually.
---
 .github/matrix.py | 2 ++
 1 file changed, 2 insertions(+)
 mode change 100644 => 100755 .github/matrix.py

diff --git a/.github/matrix.py b/.github/matrix.py
old mode 100644
new mode 100755
index 473524848..cfef53c9e
--- a/.github/matrix.py
+++ b/.github/matrix.py
@@ -1,3 +1,5 @@
+#!/usr/bin/python3
+
 # Copyright 2019 Ilya Shipitsin 
 # Copyright 2020 Tim Duesterhus 
 #
-- 
2.31.1




[PATCH] CLEANUP: reg-tests: Remove obsolete no-htx parameter for reg-tests

2021-05-31 Thread Tim Duesterhus
The legacy HTTP subsystem has been removed. HTX is always enabled.
---
 reg-tests/README  |  2 +-
 reg-tests/cache/basic.vtc |  1 -
 reg-tests/cache/caching_rules.vtc |  1 -
 reg-tests/cache/expires.vtc   |  1 -
 reg-tests/cache/if-modified-since.vtc |  1 -
 reg-tests/cache/if-none-match.vtc |  1 -
 reg-tests/cache/post_on_entry.vtc |  1 -
 reg-tests/cache/sample_fetches.vtc|  1 -
 reg-tests/cache/vary.vtc  |  1 -
 reg-tests/cache/vary_accept_encoding.vtc  |  1 -
 reg-tests/compression/basic.vtc   |  1 -
 reg-tests/compression/etags_conversion.vtc|  1 -
 reg-tests/compression/lua_validation.vtc  |  1 -
 reg-tests/compression/vary.vtc|  1 -
 reg-tests/connection/dispatch.vtc |  1 -
 .../connection/proxy_protocol_random_fail.vtc |  1 -
 reg-tests/filters/random-forwarding.vtc   |  1 -
 reg-tests/http-capture/multiple_headers.vtc   |  1 -
 .../http-cookies/cookie_insert_indirect.vtc   |  1 -
 reg-tests/http-messaging/h1_to_h1.vtc |  1 -
 reg-tests/http-messaging/h2_to_h1.vtc |  1 -
 .../http-messaging/http_abortonclose.vtc  |  1 -
 .../http-messaging/http_bodyless_response.vtc |  1 -
 .../http-messaging/http_request_buffer.vtc|  1 -
 .../http-messaging/http_wait_for_body.vtc |  1 -
 reg-tests/http-messaging/protocol_upgrade.vtc |  1 -
 reg-tests/http-messaging/websocket.vtc|  3 ---
 reg-tests/http-rules/acl_cli_spaces.vtc   |  1 -
 ...erters_ipmask_concat_strcmp_field_word.vtc |  2 --
 reg-tests/http-rules/h1or2_to_h1c.vtc |  1 -
 reg-tests/http-rules/map_redirect.vtc |  1 -
 .../http-rules/map_regm_with_backref.vtc  |  1 -
 reg-tests/lua/bad_http_clt_req_duration.vtc   |  2 --
 reg-tests/lua/close_wait_lf.vtc   |  1 -
 reg-tests/lua/h_txn_get_priv.vtc  |  2 --
 reg-tests/lua/lua_socket.vtc  |  2 --
 reg-tests/lua/set_var.vtc |  2 --
 reg-tests/lua/txn_get_priv-thread.vtc |  3 ---
 reg-tests/lua/txn_get_priv.vtc|  3 ---
 reg-tests/lua/wrong_types_usage.vtc   |  2 --
 reg-tests/mcli/mcli_show_info.vtc |  1 -
 reg-tests/mcli/mcli_start_progs.vtc   |  1 -
 reg-tests/seamless-reload/abns_socket.vtc |  1 -
 reg-tests/server/cli_add_server.vtc   |  1 -
 reg-tests/server/cli_delete_server.vtc|  1 -
 reg-tests/server/cli_set_fdqn.vtc |  1 -
 reg-tests/server/cli_set_ssl.vtc  |  1 -
 reg-tests/ssl/add_ssl_crt-list.vtc|  2 --
 reg-tests/ssl/del_ssl_crt-list.vtc|  3 ---
 reg-tests/ssl/new_del_ssl_cafile.vtc  |  1 -
 reg-tests/ssl/new_del_ssl_crlfile.vtc |  1 -
 reg-tests/ssl/set_ssl_cafile.vtc  |  1 -
 reg-tests/ssl/set_ssl_cert.vtc|  1 -
 reg-tests/ssl/set_ssl_cert_bundle.vtc |  1 -
 reg-tests/ssl/set_ssl_cert_noext.vtc  |  1 -
 reg-tests/ssl/set_ssl_crlfile.vtc |  1 -
 reg-tests/ssl/set_ssl_server_cert.vtc |  1 -
 reg-tests/ssl/ssl_client_auth.vtc |  1 -
 reg-tests/ssl/ssl_client_samples.vtc  |  2 --
 reg-tests/ssl/ssl_crt-list_filters.vtc|  1 -
 reg-tests/ssl/ssl_frontend_samples.vtc|  2 --
 reg-tests/ssl/ssl_server_samples.vtc  |  2 --
 reg-tests/ssl/ssl_simple_crt-list.vtc |  1 -
 reg-tests/ssl/wrong_ctx_storage.vtc   |  1 -
 .../converteers_ref_cnt_never_dec.vtc |  1 -
 reg-tests/stick-table/src_conn_rate.vtc   |  1 -
 reg-tests/stick-table/unknown_key.vtc |  1 -
 reg-tests/stickiness/lb-services.vtc  |  2 --
 reg-tests/stickiness/srvkey-addr.vtc  |  2 --
 .../webstats-scope-and-post-change.vtc|  1 -
 scripts/run-regtests.sh   | 22 +--
 71 files changed, 2 insertions(+), 111 deletions(-)

diff --git a/reg-tests/README b/reg-tests/README
index d05a2a805..6993d9782 100644
--- a/reg-tests/README
+++ b/reg-tests/README
@@ -38,7 +38,7 @@ See also: doc/regression-testing.txt
   You must set HAPROXY_PROGRAM environment variable to give the location
   of the HAProxy program to test to vtest:
 
-$ HAPROXY_PROGRAM= vtest [-Dno-htx=] ...
+$ HAPROXY_PROGRAM= vtest ...
 
   The HAProxy VTC files found in HAProxy sources may be run with the reg-tests
   Makefile target. You must set the VTEST_PROGRAM environment variable to
diff --git a/reg-tests/cache/basic.vtc b/reg-tests/cache/basic.vtc
index 849057d9e..30f463108 100644
--- a/reg-tests/cache/basic.vtc
+++ b/reg-tests/cache/basic.vtc
@@ -27,7 +27,6 @@ server s1 {
 haproxy h1 -conf {
 defaults
 mode http
-${no-htx} option http-use-htx
 timeout connect 1s
 timeout client  1s
 timeout server  1s
diff --git a/reg-tests/cache/caching_rules.vtc 
b/reg-tests/cache/caching_rules.vtc

[PATCH] MINOR: cfgparse: Fail when encountering extra arguments in macro

2021-05-26 Thread Tim Duesterhus
This resolves GitHub issue #1124.

This change should be backported as a *warning* to 2.4.
---
 src/cfgparse.c | 59 --
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 8132e47e8..adc903b53 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -2056,6 +2056,13 @@ int readcfgfile(const char *file)
char *errmsg = NULL;
int cond;
 
+   if (*args[2]) {
+   ha_alert("parsing [%s:%d]: Unexpected 
argument '%s' for '%s'.\n",
+file, linenum, args[2], 
args[0]);
+   err_code |= ERR_ALERT | ERR_FATAL | 
ERR_ABORT;
+   break;
+   }
+
nested_cond_lvl++;
if (nested_cond_lvl >= MAXNESTEDCONDS) {
ha_alert("parsing [%s:%d]: too many 
nested '.if', max is %d.\n", file, linenum, MAXNESTEDCONDS);
@@ -2098,6 +2105,13 @@ int readcfgfile(const char *file)
char *errmsg = NULL;
int cond;
 
+   if (*args[2]) {
+   ha_alert("parsing [%s:%d]: Unexpected 
argument '%s' for '%s'.\n",
+file, linenum, args[2], 
args[0]);
+   err_code |= ERR_ALERT | ERR_FATAL | 
ERR_ABORT;
+   break;
+   }
+
if (!nested_cond_lvl) {
ha_alert("parsing [%s:%d]: lone '.elif' 
with no matching '.if'.\n", file, linenum);
err_code |= ERR_ALERT | ERR_FATAL | 
ERR_ABORT;
@@ -2140,6 +2154,13 @@ int readcfgfile(const char *file)
goto next_line;
}
else if (strcmp(args[0], ".else") == 0) {
+   if (*args[1]) {
+   ha_alert("parsing [%s:%d]: Unxpected 
argument '%s' for '%s'.\n",
+file, linenum, args[1], 
args[0]);
+   err_code |= ERR_ALERT | ERR_FATAL | 
ERR_ABORT;
+   break;
+   }
+
if (!nested_cond_lvl) {
ha_alert("parsing [%s:%d]: lone '.else' 
with no matching '.if'.\n", file, linenum);
err_code |= ERR_ALERT | ERR_FATAL | 
ERR_ABORT;
@@ -2165,10 +2186,16 @@ int readcfgfile(const char *file)
goto next_line;
}
else if (strcmp(args[0], ".endif") == 0) {
+   if (*args[1]) {
+   ha_alert("parsing [%s:%d]: Unxpected 
argument '%s' for '%s'.\n",
+file, linenum, args[1], 
args[0]);
+   err_code |= ERR_ALERT | ERR_FATAL | 
ERR_ABORT;
+   break;
+   }
+
if (!nested_cond_lvl) {
ha_alert("parsing [%s:%d]: lone 
'.endif' with no matching '.if'.\n", file, linenum);
-   err_code |= ERR_ALERT | ERR_FATAL;
-   fatal++;
+   err_code |= ERR_ALERT | ERR_FATAL | 
ERR_ABORT;
break;
}
nested_cond_lvl--;
@@ -2189,20 +2216,48 @@ int readcfgfile(const char *file)
/* .warning/.error/.notice/.diag */
if (*args[0] == '.') {
if (strcmp(args[0], ".alert") == 0) {
+   if (*args[2]) {
+   ha_alert("parsing [%s:%d]: Unexpected 
argument '%s' for '%s'. Use quotes if the message should contain spaces.\n",
+  file, linenum, args[2], 
args[0]);
+   err_code |= ERR_ALERT | ERR_FATAL;
+   goto next_line;
+   }
+
ha_alert("parsing [%s:%d]: '%s'.\n", file, 
linenum, args[1]);
err_code |= ERR_ALERT | ERR_FATAL | ERR_ABORT;
goto err;
}
else if (strcmp(args[0], ".warning") == 0) {
+

[PATCH] Revert "CI: Build VTest with clang"

2021-05-12 Thread Tim Duesterhus
The issue with VTest not building properly in gcc is fixed since commit
vtest/VTest@0730540c43a2a23436b43f46327d6bac644d816d. Revert the patch to keep
the CI configuration simple.

This reverts commit e61f53eb44a390f9a8c8c4f34077c365942e0729.
---
 .github/workflows/vtest.yml | 2 +-
 .travis.yml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/vtest.yml b/.github/workflows/vtest.yml
index 1d62f98f3..cb52f27d6 100644
--- a/.github/workflows/vtest.yml
+++ b/.github/workflows/vtest.yml
@@ -63,7 +63,7 @@ jobs:
 curl -fsSL https://github.com/vtest/VTest/archive/master.tar.gz -o 
VTest.tar.gz
 mkdir VTest
 tar xvf VTest.tar.gz -C VTest --strip-components=1
-make -C VTest -j$(nproc) FLAGS="-O2 -s -Wall" CC=clang
+make -C VTest -j$(nproc) FLAGS="-O2 -s -Wall"
 sudo install -m755 VTest/vtest /usr/local/bin/vtest
 - name: Install SSL ${{ matrix.ssl }}
   if: ${{ matrix.ssl && matrix.ssl != 'stock' }}
diff --git a/.travis.yml b/.travis.yml
index 37b667bc1..1aa415aa8 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -40,7 +40,7 @@ matrix:
 install:
   - git clone https://github.com/VTest/VTest.git ../vtest
   # Special flags due to: https://github.com/vtest/VTest/issues/12
-  - make -C ../vtest FLAGS="-O2 -s -Wall" CC=clang
+  - make -C ../vtest FLAGS="-O2 -s -Wall"
 
 script:
   - make -j$(nproc) ERR=1 TARGET=linux-glibc CC=$CC DEBUG=-DDEBUG_STRICT=1 
$FLAGS
-- 
2.31.1




[PATCH] BUG/MINOR: http_act: Fix normalizer names in error messages

2021-05-10 Thread Tim Duesterhus
These places were forgotten when the normalizers were renamed.

Bug introduced in 5be6ab269e5606aef954f39d6717b024f97b3789, which is 2.4. No
backport needed.
---
 src/http_act.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/http_act.c b/src/http_act.c
index b8413f331..96ac8f87b 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -401,7 +401,7 @@ static enum act_parse_ret parse_http_normalize_uri(const 
char **args, int *orig_
rule->action = ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT;
}
else if (strcmp(args[cur_arg], "if") != 0 && 
strcmp(args[cur_arg], "unless") != 0) {
-   memprintf(err, "unknown argument '%s' for 'dotdot' 
normalizer", args[cur_arg]);
+   memprintf(err, "unknown argument '%s' for 
'path-strip-dotdot' normalizer", args[cur_arg]);
return ACT_RET_PRS_ERR;
}
}
@@ -421,7 +421,7 @@ static enum act_parse_ret parse_http_normalize_uri(const 
char **args, int *orig_
rule->action = ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE;
}
else if (strcmp(args[cur_arg], "if") != 0 && 
strcmp(args[cur_arg], "unless") != 0) {
-   memprintf(err, "unknown argument '%s' for 
'percent-upper' normalizer", args[cur_arg]);
+   memprintf(err, "unknown argument '%s' for 
'percent-to-uppercase' normalizer", args[cur_arg]);
return ACT_RET_PRS_ERR;
}
}
-- 
2.31.1




[PATCH] CI: Build VTest with clang

2021-05-10 Thread Tim Duesterhus
Willy,
Ilya,

not tested, but it should be simple enough to not mess it up.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
Current VTest master fails to build using gcc, see vtest/VTest#27.

This patch is to be reverted once VTest is fixed.
---
 .github/workflows/vtest.yml | 2 +-
 .travis.yml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/vtest.yml b/.github/workflows/vtest.yml
index cb52f27d6..1d62f98f3 100644
--- a/.github/workflows/vtest.yml
+++ b/.github/workflows/vtest.yml
@@ -63,7 +63,7 @@ jobs:
 curl -fsSL https://github.com/vtest/VTest/archive/master.tar.gz -o 
VTest.tar.gz
 mkdir VTest
 tar xvf VTest.tar.gz -C VTest --strip-components=1
-make -C VTest -j$(nproc) FLAGS="-O2 -s -Wall"
+make -C VTest -j$(nproc) FLAGS="-O2 -s -Wall" CC=clang
 sudo install -m755 VTest/vtest /usr/local/bin/vtest
 - name: Install SSL ${{ matrix.ssl }}
   if: ${{ matrix.ssl && matrix.ssl != 'stock' }}
diff --git a/.travis.yml b/.travis.yml
index 1aa415aa8..37b667bc1 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -40,7 +40,7 @@ matrix:
 install:
   - git clone https://github.com/VTest/VTest.git ../vtest
   # Special flags due to: https://github.com/vtest/VTest/issues/12
-  - make -C ../vtest FLAGS="-O2 -s -Wall"
+  - make -C ../vtest FLAGS="-O2 -s -Wall" CC=clang
 
 script:
   - make -j$(nproc) ERR=1 TARGET=linux-glibc CC=$CC DEBUG=-DDEBUG_STRICT=1 
$FLAGS
-- 
2.31.1




[PATCH 2/2] MINOR: uri_normalizer: Add `fragment-encode` normalizer

2021-05-10 Thread Tim Duesterhus
This normalizer encodes '#' as '%23'.

See GitHub Issue #714.
---
 doc/configuration.txt  |  9 +++
 include/haproxy/action-t.h |  1 +
 include/haproxy/uri_normalizer.h   |  1 +
 reg-tests/http-rules/normalize_uri.vtc | 36 +-
 src/http_act.c | 22 
 src/uri_normalizer.c   | 35 +
 6 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 00749e5ee..bc63f51e5 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6172,6 +6172,7 @@ http-request early-hint   [ { if | unless } 
 ]
   See RFC 8297 for more information.
 
 http-request normalize-uri  [ { if | unless }  ]
+http-request normalize-uri fragment-encode [ { if | unless }  ]
 http-request normalize-uri fragment-strip [ { if | unless }  ]
 http-request normalize-uri path-merge-slashes [ { if | unless }  ]
 http-request normalize-uri path-strip-dot [ { if | unless }  ]
@@ -6210,6 +6211,14 @@ http-request normalize-uri query-sort-by-name [ { if | 
unless }  ]
 
   The following normalizers are available:
 
+  - fragment-encode: Encodes "#" as "%23".
+
+  The "fragment-strip" normalizer should be preferred, unless it is known
+  that broken clients do not correctly encode '#' within the path 
component.
+
+  Example:
+  - /#foo  -> /%23foo
+
   - fragment-strip: Removes the URI's "fragment" component.
 
   According to RFC 3986#3.5 the "fragment" component of an URI should not
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index 56ac32f7f..d4fc3f6da 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -112,6 +112,7 @@ enum act_normalize_uri {
ACT_NORMALIZE_URI_PERCENT_DECODE_UNRESERVED,
ACT_NORMALIZE_URI_PERCENT_DECODE_UNRESERVED_STRICT,
ACT_NORMALIZE_URI_FRAGMENT_STRIP,
+   ACT_NORMALIZE_URI_FRAGMENT_ENCODE,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index fa5d89dd0..b384007f5 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -26,6 +26,7 @@ static inline enum uri_normalizer_err 
uri_normalizer_fragment_strip(const struct
return URI_NORMALIZER_ERR_NONE;
 }
 
+enum uri_normalizer_err uri_normalizer_fragment_encode(const struct ist input, 
struct ist *dst);
 enum uri_normalizer_err uri_normalizer_percent_decode_unreserved(const struct 
ist input, int strict, struct ist *dst);
 enum uri_normalizer_err uri_normalizer_percent_upper(const struct ist input, 
int strict, struct ist *dst);
 enum uri_normalizer_err uri_normalizer_path_dot(const struct ist path, struct 
ist *dst);
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
index 792bea5d4..7e2d7491f 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -8,7 +8,7 @@ feature ignore_unknown_macro
 server s1 {
 rxreq
 txresp
-} -repeat 66 -start
+} -repeat 70 -start
 
 haproxy h1 -conf {
 global
@@ -137,6 +137,18 @@ haproxy h1 -conf {
 
 default_backend be
 
+frontend fe_fragment_encode
+bind "fd@${fe_fragment_encode}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri fragment-encode
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
 backend be
 server s1 ${s1_addr}:${s1_port}
 
@@ -500,3 +512,25 @@ client c9 -connect ${h1_fe_fragment_strip_sock} {
 expect resp.http.before == "*"
 expect resp.http.after == "*"
 } -run
+
+client c10 -connect ${h1_fe_fragment_encode_sock} {
+txreq -url "/#foo"
+rxresp
+expect resp.http.before == "/#foo"
+expect resp.http.after == "/%23foo"
+
+txreq -url "/#foo/#foo"
+rxresp
+expect resp.http.before == "/#foo/#foo"
+expect resp.http.after == "/%23foo/%23foo"
+
+txreq -url "/%23foo"
+rxresp
+expect resp.http.before == "/%23foo"
+expect resp.http.after == "/%23foo"
+
+txreq -req OPTIONS -url "*"
+rxresp
+expect resp.http.before == "*"
+expect resp.http.after == "*"
+} -run
diff --git a/src/http_act.c b/src/http_act.c
index 5eeba631b..a92e5674c 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -329,6 +329,23 @@ static enum act_return http_action_normalize_uri(struct 
act_rule *rule, struct p
 
err = uri_normalizer_fragment_strip(path, );
 
+   if (err != URI_NORMALIZER_ERR_NONE)
+   break;
+
+   if (!http_replace_req_path(htx, newpath, 1))
+   goto fail_rewrite;
+
+   break;
+   }
+ 

[PATCH 1/2] MINOR: uri_normalizer: Add `fragment-strip` normalizer

2021-05-10 Thread Tim Duesterhus
This normalizer strips the URI's fragment component which should never be sent
to the server.

See GitHub Issue #714.
---
 doc/configuration.txt  | 12 ++
 include/haproxy/action-t.h |  1 +
 include/haproxy/uri_normalizer.h   |  8 +++
 reg-tests/http-rules/normalize_uri.vtc | 31 +-
 src/http_act.c | 22 ++
 5 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 964bc04ce..00749e5ee 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6172,6 +6172,7 @@ http-request early-hint   [ { if | unless } 
 ]
   See RFC 8297 for more information.
 
 http-request normalize-uri  [ { if | unless }  ]
+http-request normalize-uri fragment-strip [ { if | unless }  ]
 http-request normalize-uri path-merge-slashes [ { if | unless }  ]
 http-request normalize-uri path-strip-dot [ { if | unless }  ]
 http-request normalize-uri path-strip-dotdot [ full ] [ { if | unless } 
 ]
@@ -6209,6 +6210,17 @@ http-request normalize-uri query-sort-by-name [ { if | 
unless }  ]
 
   The following normalizers are available:
 
+  - fragment-strip: Removes the URI's "fragment" component.
+
+  According to RFC 3986#3.5 the "fragment" component of an URI should not
+  be sent, but handled by the User Agent after retrieving a resource.
+
+  This normalizer should be applied first to ensure that the fragment is
+  not interpreted as part of the request's path component.
+
+  Example:
+  - /#foo  -> /
+
   - path-strip-dot: Removes "/./" segments within the "path" component
   (RFC 3986#6.2.2.3).
 
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index 5b9f543ae..56ac32f7f 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -111,6 +111,7 @@ enum act_normalize_uri {
ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE_STRICT,
ACT_NORMALIZE_URI_PERCENT_DECODE_UNRESERVED,
ACT_NORMALIZE_URI_PERCENT_DECODE_UNRESERVED_STRICT,
+   ACT_NORMALIZE_URI_FRAGMENT_STRIP,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index 06f237e45..fa5d89dd0 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -18,6 +18,14 @@
 
 #include 
 
+/* Cuts the input at the first '#'. */
+static inline enum uri_normalizer_err uri_normalizer_fragment_strip(const 
struct ist input, struct ist *dst)
+{
+   *dst = iststop(input, '#');
+
+   return URI_NORMALIZER_ERR_NONE;
+}
+
 enum uri_normalizer_err uri_normalizer_percent_decode_unreserved(const struct 
ist input, int strict, struct ist *dst);
 enum uri_normalizer_err uri_normalizer_percent_upper(const struct ist input, 
int strict, struct ist *dst);
 enum uri_normalizer_err uri_normalizer_path_dot(const struct ist path, struct 
ist *dst);
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
index 42c4c428b..792bea5d4 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -8,7 +8,7 @@ feature ignore_unknown_macro
 server s1 {
 rxreq
 txresp
-} -repeat 63 -start
+} -repeat 66 -start
 
 haproxy h1 -conf {
 global
@@ -125,6 +125,18 @@ haproxy h1 -conf {
 
 default_backend be
 
+frontend fe_fragment_strip
+bind "fd@${fe_fragment_strip}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri fragment-strip
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
 backend be
 server s1 ${s1_addr}:${s1_port}
 
@@ -471,3 +483,20 @@ client c8 -connect 
${h1_fe_percent_decode_unreserved_strict_sock} {
 rxresp
 expect resp.status == 400
 } -run
+
+client c9 -connect ${h1_fe_fragment_strip_sock} {
+txreq -url "/#foo"
+rxresp
+expect resp.http.before == "/#foo"
+expect resp.http.after == "/"
+
+txreq -url "/%23foo"
+rxresp
+expect resp.http.before == "/%23foo"
+expect resp.http.after == "/%23foo"
+
+txreq -req OPTIONS -url "*"
+rxresp
+expect resp.http.before == "*"
+expect resp.http.after == "*"
+} -run
diff --git a/src/http_act.c b/src/http_act.c
index b8413f331..5eeba631b 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -312,6 +312,23 @@ static enum act_return http_action_normalize_uri(struct 
act_rule *rule, struct p
 
err = uri_normalizer_percent_decode_unreserved(path, 
rule->action == ACT_NORMALIZE_URI_PERCENT_DECODE_UNRESERVED_STRICT, );
 
+   if (err != URI_NORMALIZER_ERR_NONE)
+   break;
+
+   if (!http_replace_req_path(htx, newpath, 1))
+   goto 

[PATCH 1/4] DOC: Fix indentation for `path-strip-dot` normalizer

2021-04-21 Thread Tim Duesterhus
The long explanation should be indented two additional spaces.
---
 doc/configuration.txt | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index de9100439..92d5ce315 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6038,11 +6038,11 @@ http-request normalize-uri query-sort-by-name [ { if | 
unless }  ]
 
   - path-strip-dot: Removes "/./" segments within the "path" component.
 
-Example:
-- /.-> /
-- /./bar/   -> /bar/
-- /a/./a-> /a/a
-- /.well-known/ -> /.well-known/ (no change)
+  Example:
+  - /.-> /
+  - /./bar/   -> /bar/
+  - /a/./a-> /a/a
+  - /.well-known/ -> /.well-known/ (no change)
 
   - path-strip-dotdot: Normalizes "/../" segments within the "path" component.
   This merges segments that attempt to access the parent directory with
-- 
2.31.1




[PATCH 2/4] DOC: Fix RFC reference for the percent-to-uppercase normalizer

2021-04-21 Thread Tim Duesterhus
The section is 6.2.2.1, not 6.2.21 (missing dot).
---
 doc/configuration.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 92d5ce315..3330f5d43 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6072,7 +6072,7 @@ http-request normalize-uri query-sort-by-name [ { if | 
unless }  ]
   - /foo//bar -> /foo/bar
 
   - percent-to-uppercase: Uppercases letters within percent-encoded sequences
-  (RFC 3986#6.2.21).
+  (RFC 3986#6.2.2.1).
 
   Example:
   - /%6f -> /%6F
-- 
2.31.1




[PATCH 4/4] MINOR: uri_normalizer: Add a `percent-decode-unreserved` normalizer

2021-04-21 Thread Tim Duesterhus
This normalizer decodes percent encoded characters within the RFC 3986
unreserved set.

See GitHub Issue #714.
---
 doc/configuration.txt  | 44 +++-
 include/haproxy/action-t.h |  2 +
 include/haproxy/uri_normalizer.h   |  1 +
 reg-tests/http-rules/normalize_uri.vtc | 75 +++-
 src/http_act.c | 33 +
 src/uri_normalizer.c   | 95 ++
 6 files changed, 247 insertions(+), 3 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 322932b5e..df01dd848 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6015,6 +6015,7 @@ http-request normalize-uri  [ { if | unless } 
 ]
 http-request normalize-uri path-merge-slashes [ { if | unless }  ]
 http-request normalize-uri path-strip-dot [ { if | unless }  ]
 http-request normalize-uri path-strip-dotdot [ full ] [ { if | unless } 
 ]
+http-request normalize-uri percent-decode-unreserved [ strict ] [ { if | 
unless }  ]
 http-request normalize-uri percent-to-uppercase [ strict ] [ { if | unless } 
 ]
 http-request normalize-uri query-sort-by-name [ { if | unless }  ]
 
@@ -6034,11 +6035,25 @@ http-request normalize-uri query-sort-by-name [ { if | 
unless }  ]
   filesystem. However it might break routing of an API that expects a specific
   number of segments in the path.
 
+  It is important to note that some normalizers might result in unsafe
+  transformations for broken URIs. It might also be possible that a combination
+  of normalizers that are safe by themselves results in unsafe transformations
+  when improperly combined.
+
+  As an example the "percent-decode-unreserved" normalizer might result in
+  unexpected results when a broken URI includes bare percent characters. One
+  such a broken URI is "/%%36%36" which would be decoded to "/%66" which in
+  turn is equivalent to "/f". By specifying the "strict" option requests to
+  such a broken URI would safely be rejected.
+
   The following normalizers are available:
 
   - path-strip-dot: Removes "/./" segments within the "path" component
   (RFC 3986#6.2.2.3).
 
+  Segments including percent encoded dots ("%2E") will not be detected. Use
+  the "percent-decode-unreserved" normalizer first if this is undesired.
+
   Example:
   - /.-> /
   - /./bar/   -> /bar/
@@ -6049,8 +6064,13 @@ http-request normalize-uri query-sort-by-name [ { if | 
unless }  ]
   (RFC 3986#6.2.2.3).
 
   This merges segments that attempt to access the parent directory with
-  their preceding segment. Empty segments do not receive special treatment.
-  Use the "path-merge-slashes" normalizer first if this is undesired.
+  their preceding segment.
+
+  Empty segments do not receive special treatment. Use the "merge-slashes"
+  normalizer first if this is undesired.
+
+  Segments including percent encoded dots ("%2E") will not be detected. Use
+  the "percent-decode-unreserved" normalizer first if this is undesired.
 
   Example:
   - /foo/../ -> /
@@ -6059,6 +6079,7 @@ http-request normalize-uri query-sort-by-name [ { if | 
unless }  ]
   - /../bar/ -> /../bar/
   - /bar/../../  -> /../
   - /foo//../-> /foo/
+  - /foo/%2E%2E/ -> /foo/%2E%2E/
 
   If the "full" option is specified then "../" at the beginning will be
   removed as well:
@@ -6074,6 +6095,25 @@ http-request normalize-uri query-sort-by-name [ { if | 
unless }  ]
   - //-> /
   - /foo//bar -> /foo/bar
 
+  - percent-decode-unreserved: Decodes unreserved percent encoded characters to
+  their representation as a regular character (RFC 3986#6.2.2.2).
+
+  The set of unreserved characters includes all letters, all digits, "-",
+  ".", "_", and "~".
+
+  Example:
+  - /%61dmin   -> /admin
+  - /foo%3Fbar=baz -> /foo%3Fbar=baz (no change)
+  - /%%36%36   -> /%66   (unsafe)
+  - /%ZZ   -> /%ZZ
+
+  If the "strict" option is specified then invalid sequences will result
+  in a HTTP 400 Bad Request being returned.
+
+  Example:
+  - /%%36%36 -> HTTP 400
+  - /%ZZ -> HTTP 400
+
   - percent-to-uppercase: Uppercases letters within percent-encoded sequences
   (RFC 3986#6.2.2.1).
 
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index 2e3edeaf3..1d0b0d229 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -109,6 +109,8 @@ enum act_normalize_uri {
ACT_NORMALIZE_URI_QUERY_SORT_BY_NAME,
ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE,
ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE_STRICT,
+   ACT_NORMALIZE_URI_PERCENT_DECODE_UNRESERVED,
+   ACT_NORMALIZE_URI_PERCENT_DECODE_UNRESERVED_STRICT,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h

[PATCH 3/4] DOC: Add RFC references for the path-strip-dot(dot)? normalizers

2021-04-21 Thread Tim Duesterhus
This is RFC 3986#6.2.2.3.
---
 doc/configuration.txt | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 3330f5d43..322932b5e 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6036,7 +6036,8 @@ http-request normalize-uri query-sort-by-name [ { if | 
unless }  ]
 
   The following normalizers are available:
 
-  - path-strip-dot: Removes "/./" segments within the "path" component.
+  - path-strip-dot: Removes "/./" segments within the "path" component
+  (RFC 3986#6.2.2.3).
 
   Example:
   - /.-> /
@@ -6044,7 +6045,9 @@ http-request normalize-uri query-sort-by-name [ { if | 
unless }  ]
   - /a/./a-> /a/a
   - /.well-known/ -> /.well-known/ (no change)
 
-  - path-strip-dotdot: Normalizes "/../" segments within the "path" component.
+  - path-strip-dotdot: Normalizes "/../" segments within the "path" component
+  (RFC 3986#6.2.2.3).
+
   This merges segments that attempt to access the parent directory with
   their preceding segment. Empty segments do not receive special treatment.
   Use the "path-merge-slashes" normalizer first if this is undesired.
-- 
2.31.1




[PATCH v2 6/8] MINOR: uri_normalizer: Add support for supressing leading `../` for dotdot normalizer

2021-04-15 Thread Tim Duesterhus
Christopher,

the general logic of the normalizer is unchanged, but the whole framework was
refactored.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This adds an option to supress `../` at the start of the resulting path.
---
 doc/configuration.txt  | 10 +-
 include/haproxy/action-t.h |  1 +
 include/haproxy/uri_normalizer.h   |  2 +-
 reg-tests/http-rules/normalize_uri.vtc | 16 
 src/http_act.c | 17 ++---
 src/uri_normalizer.c   | 25 -
 6 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index a6110eb19..a073da5fe 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6012,7 +6012,7 @@ http-request early-hint   [ { if | unless } 
 ]
   See RFC 8297 for more information.
 
 http-request normalize-uri  [ { if | unless }  ]
-http-request normalize-uri dotdot [ { if | unless }  ]
+http-request normalize-uri dotdot [ full ] [ { if | unless }  ]
 http-request normalize-uri merge-slashes [ { if | unless }  ]
 
   Performs normalization of the request's URI. The following normalizers are
@@ -6028,8 +6028,16 @@ http-request normalize-uri merge-slashes [ { if | unless 
}  ]
   - /foo/../bar/ -> /bar/
   - /foo/bar/../ -> /foo/
   - /../bar/ -> /../bar/
+  - /bar/../../  -> /../
   - /foo//../-> /foo/
 
+  If the "full" option is specified then "../" at the beginning will be
+  removed as well:
+
+  Example:
+  - /../bar/ -> /bar/
+  - /bar/../../  -> /
+
   - merge-slashes: Merges adjacent slashes within the "path" component into a
   single slash.
 
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index ac9399a6b..5a8155929 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -104,6 +104,7 @@ enum act_timeout_name {
 enum act_normalize_uri {
ACT_NORMALIZE_URI_MERGE_SLASHES,
ACT_NORMALIZE_URI_DOTDOT,
+   ACT_NORMALIZE_URI_DOTDOT_FULL,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index 9dbbe5826..811a7ebb6 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -18,7 +18,7 @@
 
 #include 
 
-enum uri_normalizer_err uri_normalizer_path_dotdot(const struct ist path, 
struct ist *dst);
+enum uri_normalizer_err uri_normalizer_path_dotdot(const struct ist path, int 
full, struct ist *dst);
 enum uri_normalizer_err uri_normalizer_path_merge_slashes(const struct ist 
path, struct ist *dst);
 
 #endif /* _HAPROXY_URI_NORMALIZER_H */
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
index e66bdc47b..5ee73a308 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -36,8 +36,13 @@ haproxy h1 -conf {
 http-request normalize-uri dotdot
 http-request set-var(txn.after) url
 
+http-request set-uri %[var(txn.before)]
+http-request normalize-uri dotdot full
+http-request set-var(txn.after_full) url
+
 http-response add-header before  %[var(txn.before)]
 http-response add-header after  %[var(txn.after)]
+http-response add-header after-full  %[var(txn.after_full)]
 
 default_backend be
 
@@ -103,54 +108,65 @@ client c2 -connect ${h1_fe_dotdot_sock} {
 rxresp
 expect resp.http.before == "/foo/bar"
 expect resp.http.after == "/foo/bar"
+expect resp.http.after-full == "/foo/bar"
 
 txreq -url "/foo/.."
 rxresp
 expect resp.http.before == "/foo/.."
 expect resp.http.after == "/"
+expect resp.http.after-full == "/"
 
 txreq -url "/foo/../"
 rxresp
 expect resp.http.before == "/foo/../"
 expect resp.http.after == "/"
+expect resp.http.after-full == "/"
 
 txreq -url "/foo/bar/../"
 rxresp
 expect resp.http.before == "/foo/bar/../"
 expect resp.http.after == "/foo/"
+expect resp.http.after-full == "/foo/"
 
 txreq -url "/foo/../bar"
 rxresp
 expect resp.http.before == "/foo/../bar"
 expect resp.http.after == "/bar"
+expect resp.http.after-full == "/bar"
 
 txreq -url "/foo/../bar/"
 rxresp
 expect resp.http.before == "/foo/../bar/"
 expect resp.http.after == "/bar/"
+expect resp.http.after-full == "/bar/"
 
 txreq -url "/foo/../../bar/"
 rxresp
 expect resp.http.before == "/foo/../../bar/"
 expect resp.http.after == "/../bar/"
+expect resp.http.after-full == "/bar/"
 
 txreq -url "/foo//../../bar/"
 rxresp
 expect resp.http.before == "/foo//../../bar/"
 expect resp.http.after == "/bar/"
+expect resp.http.after-full == "/bar/"
 
 txreq -url "/foo/?bar=/foo/../"
 rxresp
 expect 

[PATCH v2 0/8] URI normalization / Issue #714

2021-04-15 Thread Tim Duesterhus
Christopher,

again: Thank you for the very helpful review. In this series you can find v2 of
my URI normalization patches. I hope I did not forget to incorporate any of
your feedback.

Some general notes:

- I completely cleaned up the commit history to group similar patches (e.g. the
  two patches for dotdot) and to avoid later commits completely refactoring
  earlier commits (e.g. the error handling).
- As suggested I am now returning the error code and taking a `struct ist *dst`.
- The values of `enum uri_normalizer_err` are cleaned up.
- I cleaned up the error handling in `http_action_normalize_uri`.
- I am now using `istdiff()`.
- Dynamic allocation is no more.
- I fixed some parts of the code style (`struct ist* foo` -> `struct ist *foo`).
- I const'ified as much as possible.

Tim Düsterhus (8):
  MINOR: uri_normalizer: Add uri_normalizer module
  MINOR: uri_normalizer: Add `enum uri_normalizer_err`
  MINOR: uri_normalizer: Add `http-request normalize-uri`
  MINOR: uri_normalizer: Add a `merge-slashes` normalizer to
http-request normalize-uri
  MINOR: uri_normalizer: Add a `dotdot` normalizer to http-request
normalize-uri
  MINOR: uri_normalizer: Add support for supressing leading `../` for
dotdot normalizer
  MINOR: uri_normalizer: Add a `sort-query` normalizer
  MINOR: uri_normalizer: Add a `percent-upper` normalizer

 Makefile   |   2 +-
 doc/configuration.txt  |  58 +
 include/haproxy/action-t.h |   9 +
 include/haproxy/uri_normalizer-t.h |  31 +++
 include/haproxy/uri_normalizer.h   |  33 +++
 reg-tests/http-rules/normalize_uri.vtc | 314 +
 src/http_act.c | 201 
 src/uri_normalizer.c   | 296 +++
 8 files changed, 943 insertions(+), 1 deletion(-)
 create mode 100644 include/haproxy/uri_normalizer-t.h
 create mode 100644 include/haproxy/uri_normalizer.h
 create mode 100644 reg-tests/http-rules/normalize_uri.vtc
 create mode 100644 src/uri_normalizer.c

-- 
2.31.1




[PATCH v2 7/8] MINOR: uri_normalizer: Add a `sort-query` normalizer

2021-04-15 Thread Tim Duesterhus
Christopher,

here I am using `istdiff` and a trash buffer instead of dynamic allocation.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This normalizer sorts the `&` delimited query parameters by parameter name.

See GitHub Issue #714.
---
 doc/configuration.txt  | 10 +++
 include/haproxy/action-t.h |  1 +
 include/haproxy/uri_normalizer.h   |  1 +
 reg-tests/http-rules/normalize_uri.vtc | 81 ++-
 src/http_act.c | 22 +++
 src/uri_normalizer.c   | 89 ++
 6 files changed, 203 insertions(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index a073da5fe..862427e51 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6014,6 +6014,7 @@ http-request early-hint   [ { if | unless } 
 ]
 http-request normalize-uri  [ { if | unless }  ]
 http-request normalize-uri dotdot [ full ] [ { if | unless }  ]
 http-request normalize-uri merge-slashes [ { if | unless }  ]
+http-request normalize-uri sort-query [ { if | unless }  ]
 
   Performs normalization of the request's URI. The following normalizers are
   available:
@@ -6045,6 +6046,15 @@ http-request normalize-uri merge-slashes [ { if | unless 
}  ]
   - //-> /
   - /foo//bar -> /foo/bar
 
+  - sort-query: Sorts the query string parameters by parameter name.
+  Parameters are assumed to be delimited by '&'. Shorter names sort before
+  longer names and identical parameter names maintain their relative order.
+
+  Example:
+  - /?c=3=1=2 -> /?a=1=2=3
+  - /?aaa=3=1=2  -> /?a=1=2=3
+  - /?a=3=4=1=5=2 -> /?a=3=1=2=4=5
+
 http-request redirect  [ { if | unless }  ]
 
   This performs an HTTP redirection based on a redirect rule. This is exactly
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index 5a8155929..ae43a936d 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -105,6 +105,7 @@ enum act_normalize_uri {
ACT_NORMALIZE_URI_MERGE_SLASHES,
ACT_NORMALIZE_URI_DOTDOT,
ACT_NORMALIZE_URI_DOTDOT_FULL,
+   ACT_NORMALIZE_URI_SORT_QUERY,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index 811a7ebb6..c16dd3ffa 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -20,6 +20,7 @@
 
 enum uri_normalizer_err uri_normalizer_path_dotdot(const struct ist path, int 
full, struct ist *dst);
 enum uri_normalizer_err uri_normalizer_path_merge_slashes(const struct ist 
path, struct ist *dst);
+enum uri_normalizer_err uri_normalizer_query_sort(const struct ist query, 
const char delim, struct ist *dst);
 
 #endif /* _HAPROXY_URI_NORMALIZER_H */
 
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
index 5ee73a308..cb3fa2f63 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -8,7 +8,7 @@ feature ignore_unknown_macro
 server s1 {
 rxreq
 txresp
-} -repeat 21 -start
+} -repeat 34 -start
 
 haproxy h1 -conf {
 defaults
@@ -46,6 +46,18 @@ haproxy h1 -conf {
 
 default_backend be
 
+frontend fe_sort_query
+bind "fd@${fe_sort_query}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri sort-query
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
 backend be
 server s1 ${s1_addr}:${s1_port}
 
@@ -170,3 +182,70 @@ client c2 -connect ${h1_fe_dotdot_sock} {
 expect resp.http.after == "*"
 expect resp.http.after-full == "*"
 } -run
+
+client c3 -connect ${h1_fe_sort_query_sock} {
+txreq -url "/?a=a"
+rxresp
+expect resp.http.before == "/?a=a"
+expect resp.http.after == "/?a=a"
+
+txreq -url "/?a=a=z"
+rxresp
+expect resp.http.before == "/?a=a=z"
+expect resp.http.after == "/?a=a=z"
+
+txreq -url "/?z=z=a"
+rxresp
+expect resp.http.before == "/?z=z=a"
+expect resp.http.after == "/?a=a=z"
+
+txreq -url "/?a=z=a"
+rxresp
+expect resp.http.before == "/?a=z=a"
+expect resp.http.after == "/?a=z=a"
+
+txreq -url "/?z=a=z"
+rxresp
+expect resp.http.before == "/?z=a=z"
+expect resp.http.after == "/?a=z=a"
+
+txreq -url "/?c"
+rxresp
+expect resp.http.before == "/?c"
+expect resp.http.after == "/?a"
+
+txreq -url "/?a"
+rxresp
+expect resp.http.before == "/?a"
+expect resp.http.after == "/?a"
+
+txreq -url "/?"
+rxresp
+expect resp.http.before == "/?"
+expect resp.http.after == "/?a"
+
+txreq -url "/?a=5=3=1=2=4"
+rxresp
+expect 

[PATCH v2 8/8] MINOR: uri_normalizer: Add a `percent-upper` normalizer

2021-04-15 Thread Tim Duesterhus
Christopher,

the general logic of the normalizer is unchanged, but the whole framework was
refactored.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This normalizer uppercases the hexadecimal characters used in percent-encoding.

See GitHub Issue #714.
---
 doc/configuration.txt  | 14 ++
 include/haproxy/action-t.h |  2 +
 include/haproxy/uri_normalizer.h   |  1 +
 reg-tests/http-rules/normalize_uri.vtc | 65 +-
 src/http_act.c | 33 +
 src/uri_normalizer.c   | 58 +++
 6 files changed, 172 insertions(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 862427e51..1e2f72b61 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6014,6 +6014,7 @@ http-request early-hint   [ { if | unless } 
 ]
 http-request normalize-uri  [ { if | unless }  ]
 http-request normalize-uri dotdot [ full ] [ { if | unless }  ]
 http-request normalize-uri merge-slashes [ { if | unless }  ]
+http-request normalize-uri percent-upper [ strict ] [ { if | unless } 
 ]
 http-request normalize-uri sort-query [ { if | unless }  ]
 
   Performs normalization of the request's URI. The following normalizers are
@@ -6046,6 +6047,19 @@ http-request normalize-uri sort-query [ { if | unless } 
 ]
   - //-> /
   - /foo//bar -> /foo/bar
 
+  - percent-upper: Uppercases letters within percent-encoded sequences
+  (RFC 3986#6.2.21).
+
+  Example:
+  - /%6f -> /%6F
+  - /%zz -> /%zz
+
+  If the "strict" option is specified then invalid sequences will result
+  in a HTTP 400 Bad Request being returned.
+
+  Example:
+  - /%zz -> HTTP 400
+
   - sort-query: Sorts the query string parameters by parameter name.
   Parameters are assumed to be delimited by '&'. Shorter names sort before
   longer names and identical parameter names maintain their relative order.
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index ae43a936d..cce2a2e23 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -106,6 +106,8 @@ enum act_normalize_uri {
ACT_NORMALIZE_URI_DOTDOT,
ACT_NORMALIZE_URI_DOTDOT_FULL,
ACT_NORMALIZE_URI_SORT_QUERY,
+   ACT_NORMALIZE_URI_PERCENT_UPPER,
+   ACT_NORMALIZE_URI_PERCENT_UPPER_STRICT,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index c16dd3ffa..180936eae 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -18,6 +18,7 @@
 
 #include 
 
+enum uri_normalizer_err uri_normalizer_percent_upper(const struct ist input, 
int strict, struct ist *dst);
 enum uri_normalizer_err uri_normalizer_path_dotdot(const struct ist path, int 
full, struct ist *dst);
 enum uri_normalizer_err uri_normalizer_path_merge_slashes(const struct ist 
path, struct ist *dst);
 enum uri_normalizer_err uri_normalizer_query_sort(const struct ist query, 
const char delim, struct ist *dst);
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
index cb3fa2f63..e900677e9 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -8,7 +8,7 @@ feature ignore_unknown_macro
 server s1 {
 rxreq
 txresp
-} -repeat 34 -start
+} -repeat 43 -start
 
 haproxy h1 -conf {
 defaults
@@ -58,6 +58,30 @@ haproxy h1 -conf {
 
 default_backend be
 
+frontend fe_percent_upper
+bind "fd@${fe_percent_upper}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri percent-upper
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
+frontend fe_percent_upper_strict
+bind "fd@${fe_percent_upper_strict}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri percent-upper strict
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
 backend be
 server s1 ${s1_addr}:${s1_port}
 
@@ -249,3 +273,42 @@ client c3 -connect ${h1_fe_sort_query_sock} {
 expect resp.http.before == "*"
 expect resp.http.after == "*"
 } -run
+
+client c4 -connect ${h1_fe_percent_upper_sock} {
+txreq -url "/a?a=a"
+rxresp
+expect resp.http.before == "/a?a=a"
+expect resp.http.after == "/a?a=a"
+
+txreq -url "/%aa?a=%aa"
+rxresp
+expect resp.http.before == "/%aa?a=%aa"
+expect resp.http.after == "/%AA?a=%AA"
+
+txreq -url "/%zz?a=%zz"
+rxresp
+expect resp.status == 200
+expect 

[PATCH v2 5/8] MINOR: uri_normalizer: Add a `dotdot` normalizer to http-request normalize-uri

2021-04-15 Thread Tim Duesterhus
Christopher,

the general logic of the normalizer is unchanged, but the whole framework was
refactored.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This normalizer merges `../` path segments with the predecing segment, removing
both the preceding segment and the `../`.

Empty segments do not receive special treatment. The `merge-slashes` normalizer
should be executed first.

See GitHub Issue #714.
---
 doc/configuration.txt  | 13 
 include/haproxy/action-t.h |  1 +
 include/haproxy/uri_normalizer.h   |  1 +
 reg-tests/http-rules/normalize_uri.vtc | 71 +-
 src/http_act.c | 22 +++
 src/uri_normalizer.c   | 82 ++
 6 files changed, 189 insertions(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 61cb0b5ad..a6110eb19 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6012,11 +6012,24 @@ http-request early-hint   [ { if | unless } 
 ]
   See RFC 8297 for more information.
 
 http-request normalize-uri  [ { if | unless }  ]
+http-request normalize-uri dotdot [ { if | unless }  ]
 http-request normalize-uri merge-slashes [ { if | unless }  ]
 
   Performs normalization of the request's URI. The following normalizers are
   available:
 
+  - dotdot: Normalizes "/../" segments within the "path" component. This merges
+  segments that attempt to access the parent directory with their preceding
+  segment. Empty segments do not receive special treatment. Use the
+  "merge-slashes" normalizer first if this is undesired.
+
+  Example:
+  - /foo/../ -> /
+  - /foo/../bar/ -> /bar/
+  - /foo/bar/../ -> /foo/
+  - /../bar/ -> /../bar/
+  - /foo//../-> /foo/
+
   - merge-slashes: Merges adjacent slashes within the "path" component into a
   single slash.
 
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index 4a3e3f8bd..ac9399a6b 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -103,6 +103,7 @@ enum act_timeout_name {
 
 enum act_normalize_uri {
ACT_NORMALIZE_URI_MERGE_SLASHES,
+   ACT_NORMALIZE_URI_DOTDOT,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index 416f5b7c5..9dbbe5826 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -18,6 +18,7 @@
 
 #include 
 
+enum uri_normalizer_err uri_normalizer_path_dotdot(const struct ist path, 
struct ist *dst);
 enum uri_normalizer_err uri_normalizer_path_merge_slashes(const struct ist 
path, struct ist *dst);
 
 #endif /* _HAPROXY_URI_NORMALIZER_H */
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
index 3303760d4..e66bdc47b 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -8,7 +8,7 @@ feature ignore_unknown_macro
 server s1 {
 rxreq
 txresp
-} -repeat 10 -start
+} -repeat 21 -start
 
 haproxy h1 -conf {
 defaults
@@ -29,6 +29,18 @@ haproxy h1 -conf {
 
 default_backend be
 
+frontend fe_dotdot
+bind "fd@${fe_dotdot}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri dotdot
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
 backend be
 server s1 ${s1_addr}:${s1_port}
 
@@ -85,3 +97,60 @@ client c1 -connect ${h1_fe_merge_slashes_sock} {
 expect resp.http.before == "*"
 expect resp.http.after == "*"
 } -run
+
+client c2 -connect ${h1_fe_dotdot_sock} {
+txreq -url "/foo/bar"
+rxresp
+expect resp.http.before == "/foo/bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "/foo/.."
+rxresp
+expect resp.http.before == "/foo/.."
+expect resp.http.after == "/"
+
+txreq -url "/foo/../"
+rxresp
+expect resp.http.before == "/foo/../"
+expect resp.http.after == "/"
+
+txreq -url "/foo/bar/../"
+rxresp
+expect resp.http.before == "/foo/bar/../"
+expect resp.http.after == "/foo/"
+
+txreq -url "/foo/../bar"
+rxresp
+expect resp.http.before == "/foo/../bar"
+expect resp.http.after == "/bar"
+
+txreq -url "/foo/../bar/"
+rxresp
+expect resp.http.before == "/foo/../bar/"
+expect resp.http.after == "/bar/"
+
+txreq -url "/foo/../../bar/"
+rxresp
+expect resp.http.before == "/foo/../../bar/"
+expect resp.http.after == "/../bar/"
+
+txreq -url "/foo//../../bar/"
+rxresp
+expect resp.http.before == "/foo//../../bar/"
+expect resp.http.after == "/bar/"
+
+txreq -url "/foo/?bar=/foo/../"
+rxresp
+expect resp.http.before == 

[PATCH v2 3/8] MINOR: uri_normalizer: Add `http-request normalize-uri`

2021-04-15 Thread Tim Duesterhus
Christopher,

this one is completely new. It cleanly separates the patches for adding the
action from the patches adding the normalizers.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This patch adds the `http-request normalize-uri` action that was requested in
GitHub issue #714.

Normalizers will be added in the next patches.
---
 include/haproxy/action-t.h |  4 ++
 src/http_act.c | 96 ++
 2 files changed, 100 insertions(+)

diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index 9009e4aae..2909b0da2 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -101,6 +101,10 @@ enum act_timeout_name {
ACT_TIMEOUT_TUNNEL,
 };
 
+enum act_normalize_uri {
+   ACT_NORMALIZE_URI_PLACEHOLDER,
+};
+
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
  *   called regardless the action type. */
 struct act_rule {
diff --git a/src/http_act.c b/src/http_act.c
index c699671a3..134c9037b 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 
@@ -194,6 +195,100 @@ static enum act_parse_ret parse_set_req_line(const char 
**args, int *orig_arg, s
return ACT_RET_PRS_OK;
 }
 
+/* This function executes the http-request normalize-uri action.
+ * `rule->action` is expected to be a value from `enum act_normalize_uri`.
+ *
+ * On success, it returns ACT_RET_CONT. If an error
+ * occurs while soft rewrites are enabled, the action is canceled, but the rule
+ * processing continue. Otherwsize ACT_RET_ERR is returned.
+ */
+static enum act_return http_action_normalize_uri(struct act_rule *rule, struct 
proxy *px,
+ struct session *sess, struct 
stream *s, int flags)
+{
+   enum act_return ret = ACT_RET_CONT;
+   struct htx *htx = htxbuf(>req.buf);
+   const struct ist uri = htx_sl_req_uri(http_get_stline(htx));
+   struct buffer *replace = alloc_trash_chunk();
+   enum uri_normalizer_err err = URI_NORMALIZER_ERR_INTERNAL_ERROR;
+
+   if (!replace)
+   goto fail_alloc;
+
+   switch ((enum act_normalize_uri) rule->action) {
+   case ACT_NORMALIZE_URI_PLACEHOLDER:
+   (void) uri;
+   }
+
+   switch (err) {
+   case URI_NORMALIZER_ERR_NONE:
+   break;
+   case URI_NORMALIZER_ERR_INTERNAL_ERROR:
+   ret = ACT_RET_ERR;
+   break;
+   case URI_NORMALIZER_ERR_INVALID_INPUT:
+   ret = ACT_RET_INV;
+   break;
+   case URI_NORMALIZER_ERR_ALLOC:
+   goto fail_alloc;
+   }
+
+  leave:
+   free_trash_chunk(replace);
+   return ret;
+
+  fail_alloc:
+   if (!(s->flags & SF_ERR_MASK))
+   s->flags |= SF_ERR_RESOURCE;
+   ret = ACT_RET_ERR;
+   goto leave;
+
+  fail_rewrite:
+   _HA_ATOMIC_ADD(>fe->fe_counters.failed_rewrites, 1);
+   if (s->flags & SF_BE_ASSIGNED)
+   _HA_ATOMIC_ADD(>be->be_counters.failed_rewrites, 1);
+   if (sess->listener && sess->listener->counters)
+   _HA_ATOMIC_ADD(>listener->counters->failed_rewrites, 1);
+   if (objt_server(s->target))
+   
_HA_ATOMIC_ADD(&__objt_server(s->target)->counters.failed_rewrites, 1);
+
+   if (!(s->txn->req.flags & HTTP_MSGF_SOFT_RW)) {
+   ret = ACT_RET_ERR;
+   if (!(s->flags & SF_ERR_MASK))
+   s->flags |= SF_ERR_PRXCOND;
+   }
+   goto leave;
+}
+
+/* Parses the http-request normalize-uri action. It expects a single 

+ * argument, corresponding too a value in `enum act_normalize_uri`.
+ *
+ * It returns ACT_RET_PRS_OK on success, ACT_RET_PRS_ERR on error.
+ */
+static enum act_parse_ret parse_http_normalize_uri(const char **args, int 
*orig_arg, struct proxy *px,
+   struct act_rule *rule, char 
**err)
+{
+   int cur_arg = *orig_arg;
+
+   rule->action_ptr = http_action_normalize_uri;
+   rule->release_ptr = NULL;
+
+   if (!*args[cur_arg]) {
+   memprintf(err, "missing argument ");
+   return ACT_RET_PRS_ERR;
+   }
+
+   if (0) {
+
+   }
+   else {
+   memprintf(err, "unknown normalizer '%s'", args[cur_arg]);
+   return ACT_RET_PRS_ERR;
+   }
+
+   *orig_arg = cur_arg;
+   return ACT_RET_PRS_OK;
+}
+
 /* This function executes a replace-uri action. It finds its arguments in
  * .arg.http. It builds a string in the trash from the format string
  * previously filled by function parse_replace_uri() and will execute the regex
@@ -2194,6 +2289,7 @@ static struct action_kw_list http_req_actions = {
{ "deny", parse_http_deny, 0 },
{ "disable-l7-retry", 

[PATCH v2 4/8] MINOR: uri_normalizer: Add a `merge-slashes` normalizer to http-request normalize-uri

2021-04-15 Thread Tim Duesterhus
Christopher,

the general logic of the normalizer is unchanged, but the whole framework was
refactored.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This normalizer merges adjacent slashes into a single slash, thus removing
empty path segments.

See GitHub Issue #714.
---
 doc/configuration.txt  | 13 
 include/haproxy/action-t.h |  2 +-
 include/haproxy/uri_normalizer.h   |  4 ++
 reg-tests/http-rules/normalize_uri.vtc | 87 ++
 src/http_act.c | 23 ++-
 src/uri_normalizer.c   | 39 
 6 files changed, 164 insertions(+), 4 deletions(-)
 create mode 100644 reg-tests/http-rules/normalize_uri.vtc

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 61c2a6dd9..61cb0b5ad 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6011,6 +6011,19 @@ http-request early-hint   [ { if | unless } 
 ]
 
   See RFC 8297 for more information.
 
+http-request normalize-uri  [ { if | unless }  ]
+http-request normalize-uri merge-slashes [ { if | unless }  ]
+
+  Performs normalization of the request's URI. The following normalizers are
+  available:
+
+  - merge-slashes: Merges adjacent slashes within the "path" component into a
+  single slash.
+
+  Example:
+  - //-> /
+  - /foo//bar -> /foo/bar
+
 http-request redirect  [ { if | unless }  ]
 
   This performs an HTTP redirection based on a redirect rule. This is exactly
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index 2909b0da2..4a3e3f8bd 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -102,7 +102,7 @@ enum act_timeout_name {
 };
 
 enum act_normalize_uri {
-   ACT_NORMALIZE_URI_PLACEHOLDER,
+   ACT_NORMALIZE_URI_MERGE_SLASHES,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index 20341a907..416f5b7c5 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -14,8 +14,12 @@
 #ifndef _HAPROXY_URI_NORMALIZER_H
 #define _HAPROXY_URI_NORMALIZER_H
 
+#include 
+
 #include 
 
+enum uri_normalizer_err uri_normalizer_path_merge_slashes(const struct ist 
path, struct ist *dst);
+
 #endif /* _HAPROXY_URI_NORMALIZER_H */
 
 /*
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
new file mode 100644
index 0..3303760d4
--- /dev/null
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -0,0 +1,87 @@
+varnishtest "normalize-uri tests"
+#REQUIRE_VERSION=2.4
+
+# This reg-test tests the http-request normalize-uri action.
+
+feature ignore_unknown_macro
+
+server s1 {
+rxreq
+txresp
+} -repeat 10 -start
+
+haproxy h1 -conf {
+defaults
+mode http
+timeout connect 1s
+timeout client  1s
+timeout server  1s
+
+frontend fe_merge_slashes
+bind "fd@${fe_merge_slashes}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri merge-slashes
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
+backend be
+server s1 ${s1_addr}:${s1_port}
+
+} -start
+
+client c1 -connect ${h1_fe_merge_slashes_sock} {
+txreq -url "/foo/bar"
+rxresp
+expect resp.http.before == "/foo/bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "/foo//bar"
+rxresp
+expect resp.http.before == "/foo//bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "/foo///bar"
+rxresp
+expect resp.http.before == "/foo///bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "///foo///bar"
+rxresp
+expect resp.http.before == "///foo///bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "///foo/bar"
+rxresp
+expect resp.http.before == "///foo/bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "///foo///bar///"
+rxresp
+expect resp.http.before == "///foo///bar///"
+expect resp.http.after == "/foo/bar/"
+
+txreq -url "///"
+rxresp
+expect resp.http.before == "///"
+expect resp.http.after == "/"
+
+txreq -url "/foo?bar=///"
+rxresp
+expect resp.http.before == "/foo?bar=///"
+expect resp.http.after == "/foo?bar=///"
+
+txreq -url "//foo?bar=///"
+rxresp
+expect resp.http.before == "//foo?bar=///"
+expect resp.http.after == "/foo?bar=///"
+
+txreq -req OPTIONS -url "*"
+rxresp
+expect resp.http.before == "*"
+expect resp.http.after == "*"
+} -run
diff --git a/src/http_act.c b/src/http_act.c
index 134c9037b..2af4d471a 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -215,8 +215,23 @@ static enum act_return http_action_normalize_uri(struct 
act_rule *rule, 

[PATCH v2 2/8] MINOR: uri_normalizer: Add `enum uri_normalizer_err`

2021-04-15 Thread Tim Duesterhus
Christopher,

in this one I cleaned up the values of the enum.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This enum will serve as the return type for each normalizer.
---
 include/haproxy/uri_normalizer-t.h | 31 ++
 include/haproxy/uri_normalizer.h   |  2 ++
 2 files changed, 33 insertions(+)
 create mode 100644 include/haproxy/uri_normalizer-t.h

diff --git a/include/haproxy/uri_normalizer-t.h 
b/include/haproxy/uri_normalizer-t.h
new file mode 100644
index 0..bcbcaef8e
--- /dev/null
+++ b/include/haproxy/uri_normalizer-t.h
@@ -0,0 +1,31 @@
+/*
+ * include/haproxy/uri_normalizer.h
+ * HTTP request URI normalization.
+ *
+ * Copyright 2021 Tim Duesterhus 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#ifndef _HAPROXY_URI_NORMALIZER_T_H
+#define _HAPROXY_URI_NORMALIZER_T_H
+
+enum uri_normalizer_err {
+   URI_NORMALIZER_ERR_NONE = 0,
+   URI_NORMALIZER_ERR_ALLOC,
+   URI_NORMALIZER_ERR_INVALID_INPUT,
+   URI_NORMALIZER_ERR_INTERNAL_ERROR = 0xdead,
+};
+
+#endif /* _HAPROXY_URI_NORMALIZER_T_H */
+
+/*
+ * Local variables:
+ *  c-indent-level: 8
+ *  c-basic-offset: 8
+ * End:
+ */
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index 82ef97324..20341a907 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -14,6 +14,8 @@
 #ifndef _HAPROXY_URI_NORMALIZER_H
 #define _HAPROXY_URI_NORMALIZER_H
 
+#include 
+
 #endif /* _HAPROXY_URI_NORMALIZER_H */
 
 /*
-- 
2.31.1




[PATCH v2 1/8] MINOR: uri_normalizer: Add uri_normalizer module

2021-04-15 Thread Tim Duesterhus
Christopher,

this one should be unchanged. I just fixed the conflict with Aleks' JSON patch.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This is in preparation for future patches.
---
 Makefile |  2 +-
 include/haproxy/uri_normalizer.h | 24 
 src/uri_normalizer.c | 21 +
 3 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 include/haproxy/uri_normalizer.h
 create mode 100644 src/uri_normalizer.c

diff --git a/Makefile b/Makefile
index 559248867..7b760ba51 100644
--- a/Makefile
+++ b/Makefile
@@ -884,7 +884,7 @@ OBJS += src/mux_h2.o src/mux_fcgi.o src/http_ana.o 
src/stream.o\
 src/hpack-enc.o src/hpack-huff.o src/ebtree.o src/base64.o 
\
 src/hash.o src/dgram.o src/version.o src/fix.o src/mqtt.o src/dns.o
\
 src/server_state.o src/proto_uxdg.o src/init.o src/cfgdiag.o   
\
-src/mjson.o
+src/mjson.o src/uri_normalizer.o
 
 ifneq ($(TRACE),)
 OBJS += src/calltrace.o
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
new file mode 100644
index 0..82ef97324
--- /dev/null
+++ b/include/haproxy/uri_normalizer.h
@@ -0,0 +1,24 @@
+/*
+ * include/haproxy/uri_normalizer.h
+ * HTTP request URI normalization.
+ *
+ * Copyright 2021 Tim Duesterhus 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#ifndef _HAPROXY_URI_NORMALIZER_H
+#define _HAPROXY_URI_NORMALIZER_H
+
+#endif /* _HAPROXY_URI_NORMALIZER_H */
+
+/*
+ * Local variables:
+ *  c-indent-level: 8
+ *  c-basic-offset: 8
+ * End:
+ */
diff --git a/src/uri_normalizer.c b/src/uri_normalizer.c
new file mode 100644
index 0..7db47d198
--- /dev/null
+++ b/src/uri_normalizer.c
@@ -0,0 +1,21 @@
+/*
+ * HTTP request URI normalization.
+ *
+ * Copyright 2021 Tim Duesterhus 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#include 
+#include 
+
+/*
+ * Local variables:
+ *  c-indent-level: 8
+ *  c-basic-offset: 8
+ * End:
+ */
-- 
2.31.1




[PATCH] MINOR: ist: Add `istclear(struct ist*)`

2021-04-14 Thread Tim Duesterhus
Christopher,
Willy,

On 4/13/21 6:34 PM, Christopher Faulet wrote:
>> Thus I can't simply take a `struct ist*` for the destination, as an ist
>> cannot communicate the size of the underlying buffer. I could
>> technically take a `struct buffer`, but I'd still like the result to
>> reside in an ist, because this is what the HTX API expects.
> 
> Hum, I don't understand. If you create an ist using the trash buffer 
> this way:
> 
> struct ist dst = ist2(replace->area, replace->size);
> 
> You can pass a pointer on dst. In the normalizer, you can update its 
> size. It is thus possible to use dst when calling 
> http_replace_req_path() or http_replace_req_query().
> 

I see, that makes sense to me. I thought of the size being the current length
of the ist and did not think of simply resetting it to 0 before starting the
processing.

Here's a preparation patch to the ist library, allowing this pattern to be
cleanly implemented. I've already incorporated it into my local normalization
branch, but I'd like to implement the remaining feedback before sending a new
series for it.

So: Please already take this patch if you think it is good.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
istclear allows one to easily reset an ist to zero-size, while preserving the
previous size, indicating the length of the underlying buffer.
---
 include/import/ist.h | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/include/import/ist.h b/include/import/ist.h
index af9bbac3c..0dc3008f5 100644
--- a/include/import/ist.h
+++ b/include/import/ist.h
@@ -281,6 +281,36 @@ static inline struct ist isttrim(const struct ist ist, 
size_t size)
return ret;
 }
 
+/* Sets the  of the  to zero and returns the previous length.
+ *
+ * This function is meant to be used in functions that receive an ist 
containing
+ * the destination buffer and the buffer's size. The returned size must be 
stored
+ * to prevent an overflow of such a destination buffer.
+ *
+ * If you simply want to clear an ist and do not care about the previous length
+ * then you should use `isttrim(ist, 0)`.
+ *
+ * Example Usage (fill the complete buffer with 'x'):
+ *
+ * void my_func(struct ist* dst)
+ * {
+ * size_t dst_size = istclear(dst);
+ * size_t i;
+ *
+ * for (i = 0; i < dst_size; i++)
+ * *dst = __istappend(*dst, 'x');
+ * }
+ */
+__attribute__((warn_unused_result))
+static inline size_t istclear(struct ist* ist)
+{
+   size_t len = ist->len;
+
+   ist->len = 0;
+
+   return len;
+}
+
 /* trims string  to no more than -1 characters and ensures that a
  * zero is placed after  (possibly reduced by one) and before ,
  * unless  is already zero. The string is returned. This is mostly aimed
-- 
2.31.1




[RFC PATCH 8/8] MINOR: uri_normalizer: Add a `percent-upper` normalizer

2021-04-08 Thread Tim Duesterhus
Willy,
Christopher,

and this final one adds a normalizer to turn the hex digits of percent
encoding into uppercase. Uppercase is the variant preferred by the URI RFC, so
this is what we do.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This normalizer uppercases the hexadecimal characters used in percent-encoding.

See GitHub Issue #714.
---
 doc/configuration.txt  | 14 ++
 include/haproxy/action-t.h |  2 +
 include/haproxy/uri_normalizer.h   |  1 +
 reg-tests/http-rules/normalize_uri.vtc | 65 +-
 src/http_act.c | 33 +
 src/uri_normalizer.c   | 56 ++
 6 files changed, 170 insertions(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 3422d3aa6..7d073cae6 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6014,6 +6014,7 @@ http-request early-hint   [ { if | unless } 
 ]
 http-request normalize-uri  [ { if | unless }  ]
 http-request normalize-uri dotdot [ full ] [ { if | unless }  ]
 http-request normalize-uri merge-slashes [ { if | unless }  ]
+http-request normalize-uri percent-upper [ strict ] [ { if | unless } 
 ]
 http-request normalize-uri sort-query [ { if | unless }  ]
 
   Performs normalization of the request's URI. The following normalizers are
@@ -6046,6 +6047,19 @@ http-request normalize-uri sort-query [ { if | unless } 
 ]
   - //-> /
   - /foo//bar -> /foo/bar
 
+  - percent-upper: Uppercases letters within percent-encoded sequences
+  (RFC 3986#6.2.21).
+
+  Example:
+  - /%6f -> /%6F
+  - /%zz -> /%zz
+
+  If the "strict" option is specified then invalid sequences will result
+  in a HTTP 400 Bad Request being returned.
+
+  Example:
+  - /%zz -> HTTP 400
+
   - sort-query: Sorts the query string parameters by parameter name.
   Parameters are assumed to be delimited by '&'. Shorter names sort before
   longer names and identical parameter names maintain their relative order.
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index ae43a936d..cce2a2e23 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -106,6 +106,8 @@ enum act_normalize_uri {
ACT_NORMALIZE_URI_DOTDOT,
ACT_NORMALIZE_URI_DOTDOT_FULL,
ACT_NORMALIZE_URI_SORT_QUERY,
+   ACT_NORMALIZE_URI_PERCENT_UPPER,
+   ACT_NORMALIZE_URI_PERCENT_UPPER_STRICT,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index b8bb62525..dc732daec 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -18,6 +18,7 @@
 
 #include 
 
+struct ist uri_normalizer_percent_upper(const struct ist input, int strict, 
char *trash, size_t len, enum uri_normalizer_err *err);
 struct ist uri_normalizer_path_dotdot(const struct ist path, int full, char 
*trash, size_t len, enum uri_normalizer_err *err);
 struct ist uri_normalizer_path_merge_slashes(const struct ist path, char 
*trash, size_t len, enum uri_normalizer_err *err);
 struct ist uri_normalizer_query_sort(const struct ist query, const char delim, 
char *trash, size_t len, enum uri_normalizer_err *err);
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
index cb3fa2f63..e900677e9 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -8,7 +8,7 @@ feature ignore_unknown_macro
 server s1 {
 rxreq
 txresp
-} -repeat 34 -start
+} -repeat 43 -start
 
 haproxy h1 -conf {
 defaults
@@ -58,6 +58,30 @@ haproxy h1 -conf {
 
 default_backend be
 
+frontend fe_percent_upper
+bind "fd@${fe_percent_upper}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri percent-upper
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
+frontend fe_percent_upper_strict
+bind "fd@${fe_percent_upper_strict}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri percent-upper strict
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
 backend be
 server s1 ${s1_addr}:${s1_port}
 
@@ -249,3 +273,42 @@ client c3 -connect ${h1_fe_sort_query_sock} {
 expect resp.http.before == "*"
 expect resp.http.after == "*"
 } -run
+
+client c4 -connect ${h1_fe_percent_upper_sock} {
+txreq -url "/a?a=a"
+rxresp
+expect resp.http.before == "/a?a=a"
+expect resp.http.after == "/a?a=a"
+
+txreq -url "/%aa?a=%aa"
+rxresp
+   

[RFC PATCH 7/8] MINOR: uri_normalizer: Support returning detailed errors from uri normalization

2021-04-08 Thread Tim Duesterhus
Willy,
Christopher,

this is in prepatation for the next normalizer which normalizes character case
of the percent encoding.

The resources I found are not clear on whether a percent that is not followed
by two hex digits is valid or not. Most browsers and servers appear to support
it, so I opted to leave the user a choice whether to reject it or not.

To support this I need to differ between "normalizing failed for internal
reasons" and "normalizing failed, because the user sent garbage".

Overall I am not happy with this patch, because the control flow is ugly. I
also wasn't sure in which cases I need to increase server counters (e.g.
failed_rewrites) or not. I'd be happy if you could give some advice!

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
Specific normalizations might not be possible, because the input is completely
bogus. Support returning a HTTP 400 in those cases, with regular rewriting
failures remaining a 500.
---
 include/haproxy/uri_normalizer-t.h | 32 
 include/haproxy/uri_normalizer.h   |  8 +--
 src/http_act.c | 33 +---
 src/uri_normalizer.c   | 81 --
 4 files changed, 118 insertions(+), 36 deletions(-)
 create mode 100644 include/haproxy/uri_normalizer-t.h

diff --git a/include/haproxy/uri_normalizer-t.h 
b/include/haproxy/uri_normalizer-t.h
new file mode 100644
index 0..d98ded976
--- /dev/null
+++ b/include/haproxy/uri_normalizer-t.h
@@ -0,0 +1,32 @@
+/*
+ * include/haproxy/uri_normalizer.h
+ * HTTP request URI normalization.
+ *
+ * Copyright 2021 Tim Duesterhus 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#ifndef _HAPROXY_URI_NORMALIZER_T_H
+#define _HAPROXY_URI_NORMALIZER_T_H
+
+enum uri_normalizer_err {
+   URI_NORMALIZER_ERR_NONE = 0,
+   URI_NORMALIZER_ERR_TRASH,
+   URI_NORMALIZER_ERR_ALLOC,
+   URI_NORMALIZER_ERR_INVALID_INPUT,
+   URI_NORMALIZER_ERR_BUG = 0xdead,
+};
+
+#endif /* _HAPROXY_URI_NORMALIZER_T_H */
+
+/*
+ * Local variables:
+ *  c-indent-level: 8
+ *  c-basic-offset: 8
+ * End:
+ */
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index 52bb004db..b8bb62525 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -16,9 +16,11 @@
 
 #include 
 
-struct ist uri_normalizer_path_dotdot(const struct ist path, int full, char 
*trash, size_t len);
-struct ist uri_normalizer_path_merge_slashes(const struct ist path, char 
*trash, size_t len);
-struct ist uri_normalizer_query_sort(const struct ist query, const char delim, 
char *trash, size_t len);
+#include 
+
+struct ist uri_normalizer_path_dotdot(const struct ist path, int full, char 
*trash, size_t len, enum uri_normalizer_err *err);
+struct ist uri_normalizer_path_merge_slashes(const struct ist path, char 
*trash, size_t len, enum uri_normalizer_err *err);
+struct ist uri_normalizer_query_sort(const struct ist query, const char delim, 
char *trash, size_t len, enum uri_normalizer_err *err);
 
 #endif /* _HAPROXY_URI_NORMALIZER_H */
 
diff --git a/src/http_act.c b/src/http_act.c
index 2fa11dbdf..f6b0901c4 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -209,6 +209,7 @@ static enum act_return http_action_normalize_uri(struct 
act_rule *rule, struct p
struct htx *htx = htxbuf(>req.buf);
const struct ist uri = htx_sl_req_uri(http_get_stline(htx));
struct buffer *replace = alloc_trash_chunk();
+   enum uri_normalizer_err err;
 
if (!replace)
goto fail_alloc;
@@ -223,10 +224,10 @@ static enum act_return http_action_normalize_uri(struct 
act_rule *rule, struct p
 
path = iststop(path, '?');
 
-   newpath = uri_normalizer_path_merge_slashes(path, 
replace->area, replace->size);
+   newpath = uri_normalizer_path_merge_slashes(path, 
replace->area, replace->size, );
 
if (!isttest(newpath))
-   goto fail_rewrite;
+   goto err;
 
if (!http_replace_req_path(htx, newpath, 0))
goto fail_rewrite;
@@ -243,10 +244,10 @@ static enum act_return http_action_normalize_uri(struct 
act_rule *rule, struct p
 
path = iststop(path, '?');
 
-   newpath = uri_normalizer_path_dotdot(path, rule->action 
== ACT_NORMALIZE_URI_DOTDOT_FULL, replace->area, replace->size);
+   newpath = uri_normalizer_path_dotdot(path, rule->action 
== ACT_NORMALIZE_URI_DOTDOT_FULL, replace->area, replace->

[RFC PATCH 2/8] MINOR: uri_normalizer: Add `http-request normalize-uri`

2021-04-08 Thread Tim Duesterhus
Willy,
Christopher,

something simple for a start. This one adds the http-request action and a
very simple normalizer to test whether it works. Turns out it does :-)

You can see the new `ist` helpers in action already. I'm pretty happy that
I was able to implement this completely with the new `ist` API.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This patch adds the `http-request normalize-uri` action that was requested in
GitHub issue #714.

Currently only a `merge-slashes` normalizer is implemented. This normalizer
merges adjacent slashes into a single slash, thus removing empty path segments.
---
 doc/configuration.txt  |  12 +++
 include/haproxy/action-t.h |   4 +
 include/haproxy/uri_normalizer.h   |   4 +
 reg-tests/http-rules/normalize_uri.vtc |  87 +
 src/http_act.c | 101 +
 src/uri_normalizer.c   |  29 +++
 6 files changed, 237 insertions(+)
 create mode 100644 reg-tests/http-rules/normalize_uri.vtc

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 01a01eccc..d3030b478 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6011,6 +6011,18 @@ http-request early-hint   [ { if | unless } 
 ]
 
   See RFC 8297 for more information.
 
+http-request normalize-uri  [ { if | unless }  ]
+
+  Performs normalization of the request's URI. The following normalizers are
+  available:
+
+  - merge-slashes: Merges adjacent slashes within the "path" component into a
+  single slash.
+
+  Example:
+  - //-> /
+  - /foo//bar -> /foo/bar
+
 http-request redirect  [ { if | unless }  ]
 
   This performs an HTTP redirection based on a redirect rule. This is exactly
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index 9009e4aae..4a3e3f8bd 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -101,6 +101,10 @@ enum act_timeout_name {
ACT_TIMEOUT_TUNNEL,
 };
 
+enum act_normalize_uri {
+   ACT_NORMALIZE_URI_MERGE_SLASHES,
+};
+
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
  *   called regardless the action type. */
 struct act_rule {
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index 82ef97324..b6e15e281 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -14,6 +14,10 @@
 #ifndef _HAPROXY_URI_NORMALIZER_H
 #define _HAPROXY_URI_NORMALIZER_H
 
+#include 
+
+struct ist uri_normalizer_path_merge_slashes(const struct ist path, char 
*trash, size_t len);
+
 #endif /* _HAPROXY_URI_NORMALIZER_H */
 
 /*
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
new file mode 100644
index 0..3303760d4
--- /dev/null
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -0,0 +1,87 @@
+varnishtest "normalize-uri tests"
+#REQUIRE_VERSION=2.4
+
+# This reg-test tests the http-request normalize-uri action.
+
+feature ignore_unknown_macro
+
+server s1 {
+rxreq
+txresp
+} -repeat 10 -start
+
+haproxy h1 -conf {
+defaults
+mode http
+timeout connect 1s
+timeout client  1s
+timeout server  1s
+
+frontend fe_merge_slashes
+bind "fd@${fe_merge_slashes}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri merge-slashes
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
+backend be
+server s1 ${s1_addr}:${s1_port}
+
+} -start
+
+client c1 -connect ${h1_fe_merge_slashes_sock} {
+txreq -url "/foo/bar"
+rxresp
+expect resp.http.before == "/foo/bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "/foo//bar"
+rxresp
+expect resp.http.before == "/foo//bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "/foo///bar"
+rxresp
+expect resp.http.before == "/foo///bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "///foo///bar"
+rxresp
+expect resp.http.before == "///foo///bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "///foo/bar"
+rxresp
+expect resp.http.before == "///foo/bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "///foo///bar///"
+rxresp
+expect resp.http.before == "///foo///bar///"
+expect resp.http.after == "/foo/bar/"
+
+txreq -url "///"
+rxresp
+expect resp.http.before == "///"
+expect resp.http.after == "/"
+
+txreq -url "/foo?bar=///"
+rxresp
+expect resp.http.before == "/foo?bar=///"
+expect resp.http.after == "/foo?bar=///"
+
+txreq -url "//foo?bar=///"
+rxresp
+expect resp.http.before == "//foo?bar=///"
+expect resp.http.after == "/foo?bar=///"
+
+txreq -req OPTIONS -url "*"
+   

[RFC PATCH 5/8] OPTIMIZE: uri_normalizer: Optimize allocations in uri_normalizer_query_sort

2021-04-08 Thread Tim Duesterhus
Willy,
Christopher,

I did not perform any measurements at all. But not reallocating for every
parameter should be better :-)

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
Do not reallocate for each parameter.
---
 src/uri_normalizer.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/uri_normalizer.c b/src/uri_normalizer.c
index e7b2b2cd3..8dc74788e 100644
--- a/src/uri_normalizer.c
+++ b/src/uri_normalizer.c
@@ -153,20 +153,26 @@ struct ist uri_normalizer_query_sort(const struct ist 
query, const char delim, c
struct ist scanner = istadv(query, 1);
struct ist *params = NULL;
struct ist newquery = ist2(trash, 0);
+   size_t param_size = 128;
size_t param_count = 0;
size_t i;
 
if (len < istlen(query))
-   return IST_NULL;
+   goto fail;
 
while (istlen(scanner) > 0) {
const struct ist param = istsplit(, delim);
-   struct ist *realloc = reallocarray(params, param_count + 1, 
sizeof(*realloc));
 
-   if (!realloc)
-   goto fail;
+   if (!params || param_count == param_size) {
+   struct ist *realloc;
+
+   param_size *= 2;
 
-   params = realloc;
+   if (!(realloc = reallocarray(params, param_size, 
sizeof(*realloc
+   goto fail;
+
+   params = realloc;
+   }
 
params[param_count] = param;
param_count++;
-- 
2.31.1




[RFC PATCH 3/8] MINOR: uri_normalizer: Add a `dotdot` normalizer to http-request normalize-uri

2021-04-08 Thread Tim Duesterhus
Willy,
Christopher,

I'm not very happy with the normalization logic, because am processing the URI
in reverse. This requires me to directly access offsets instead of using the
`ist` API. However this way I don't need to backtrack once I encounter a `../`
which I consider to be a win.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This normalizer merges `../` path segments with the predecing segment, removing
both the preceding segment and the `../`.

Empty segments do not receive special treatment. The `merge-slashes` normalizer
should be executed first.

See GitHub Issue #714.
---
 doc/configuration.txt  | 12 +
 include/haproxy/action-t.h |  1 +
 include/haproxy/uri_normalizer.h   |  1 +
 reg-tests/http-rules/normalize_uri.vtc | 71 +-
 src/http_act.c | 22 
 src/uri_normalizer.c   | 71 ++
 6 files changed, 177 insertions(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index d3030b478..99c845ac7 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6016,6 +6016,18 @@ http-request normalize-uri  [ { if | unless 
}  ]
   Performs normalization of the request's URI. The following normalizers are
   available:
 
+  - dotdot: Normalizes "/../" segments within the "path" component. This merges
+  segments that attempt to access the parent directory with their preceding
+  segment. Empty segments do not receive special treatment. Use the
+  "merge-slashes" normalizer first if this is undesired.
+
+  Example:
+  - /foo/../ -> /
+  - /foo/../bar/ -> /bar/
+  - /foo/bar/../ -> /foo/
+  - /../bar/ -> /../bar/
+  - /foo//../-> /foo/
+
   - merge-slashes: Merges adjacent slashes within the "path" component into a
   single slash.
 
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index 4a3e3f8bd..ac9399a6b 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -103,6 +103,7 @@ enum act_timeout_name {
 
 enum act_normalize_uri {
ACT_NORMALIZE_URI_MERGE_SLASHES,
+   ACT_NORMALIZE_URI_DOTDOT,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index b6e15e281..99b27330e 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -16,6 +16,7 @@
 
 #include 
 
+struct ist uri_normalizer_path_dotdot(const struct ist path, char *trash, 
size_t len);
 struct ist uri_normalizer_path_merge_slashes(const struct ist path, char 
*trash, size_t len);
 
 #endif /* _HAPROXY_URI_NORMALIZER_H */
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
index 3303760d4..e66bdc47b 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -8,7 +8,7 @@ feature ignore_unknown_macro
 server s1 {
 rxreq
 txresp
-} -repeat 10 -start
+} -repeat 21 -start
 
 haproxy h1 -conf {
 defaults
@@ -29,6 +29,18 @@ haproxy h1 -conf {
 
 default_backend be
 
+frontend fe_dotdot
+bind "fd@${fe_dotdot}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri dotdot
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
 backend be
 server s1 ${s1_addr}:${s1_port}
 
@@ -85,3 +97,60 @@ client c1 -connect ${h1_fe_merge_slashes_sock} {
 expect resp.http.before == "*"
 expect resp.http.after == "*"
 } -run
+
+client c2 -connect ${h1_fe_dotdot_sock} {
+txreq -url "/foo/bar"
+rxresp
+expect resp.http.before == "/foo/bar"
+expect resp.http.after == "/foo/bar"
+
+txreq -url "/foo/.."
+rxresp
+expect resp.http.before == "/foo/.."
+expect resp.http.after == "/"
+
+txreq -url "/foo/../"
+rxresp
+expect resp.http.before == "/foo/../"
+expect resp.http.after == "/"
+
+txreq -url "/foo/bar/../"
+rxresp
+expect resp.http.before == "/foo/bar/../"
+expect resp.http.after == "/foo/"
+
+txreq -url "/foo/../bar"
+rxresp
+expect resp.http.before == "/foo/../bar"
+expect resp.http.after == "/bar"
+
+txreq -url "/foo/../bar/"
+rxresp
+expect resp.http.before == "/foo/../bar/"
+expect resp.http.after == "/bar/"
+
+txreq -url "/foo/../../bar/"
+rxresp
+expect resp.http.before == "/foo/../../bar/"
+expect resp.http.after == "/../bar/"
+
+txreq -url "/foo//../../bar/"
+rxresp
+expect resp.http.before == "/foo//../../bar/"
+expect resp.http.after == "/bar/"
+
+txreq -url "/foo/?bar=/foo/../"
+rxresp
+expect resp.http.before == "/foo/?bar=/foo/../"
+expect resp.http.after == 

[RFC PATCH 1/8] MINOR: uri_normalizer: Add uri_normalizer module

2021-04-08 Thread Tim Duesterhus
Willy,
Christopher,

I used uri_auth.[ch] as the basis for the source file structure (comments and
stuff).

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This is in preparation for future patches.
---
 Makefile |  3 ++-
 include/haproxy/uri_normalizer.h | 24 
 src/uri_normalizer.c | 21 +
 3 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 include/haproxy/uri_normalizer.h
 create mode 100644 src/uri_normalizer.c

diff --git a/Makefile b/Makefile
index 9b22fe4be..0d497747b 100644
--- a/Makefile
+++ b/Makefile
@@ -883,7 +883,8 @@ OBJS += src/mux_h2.o src/mux_fcgi.o src/http_ana.o 
src/stream.o\
 src/ebistree.o src/auth.o src/wdt.o src/http_acl.o 
\
 src/hpack-enc.o src/hpack-huff.o src/ebtree.o src/base64.o 
\
 src/hash.o src/dgram.o src/version.o src/fix.o src/mqtt.o src/dns.o
\
-src/server_state.o src/proto_uxdg.o src/init.o src/cfgdiag.o
+src/server_state.o src/proto_uxdg.o src/init.o src/cfgdiag.o   
\
+src/uri_normalizer.o
 
 ifneq ($(TRACE),)
 OBJS += src/calltrace.o
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
new file mode 100644
index 0..82ef97324
--- /dev/null
+++ b/include/haproxy/uri_normalizer.h
@@ -0,0 +1,24 @@
+/*
+ * include/haproxy/uri_normalizer.h
+ * HTTP request URI normalization.
+ *
+ * Copyright 2021 Tim Duesterhus 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#ifndef _HAPROXY_URI_NORMALIZER_H
+#define _HAPROXY_URI_NORMALIZER_H
+
+#endif /* _HAPROXY_URI_NORMALIZER_H */
+
+/*
+ * Local variables:
+ *  c-indent-level: 8
+ *  c-basic-offset: 8
+ * End:
+ */
diff --git a/src/uri_normalizer.c b/src/uri_normalizer.c
new file mode 100644
index 0..7db47d198
--- /dev/null
+++ b/src/uri_normalizer.c
@@ -0,0 +1,21 @@
+/*
+ * HTTP request URI normalization.
+ *
+ * Copyright 2021 Tim Duesterhus 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#include 
+#include 
+
+/*
+ * Local variables:
+ *  c-indent-level: 8
+ *  c-basic-offset: 8
+ * End:
+ */
-- 
2.31.1




[RFC PATCH 6/8] MINOR: uri_normalizer: Add support for supressing leading `../` for dotdot normalizer

2021-04-08 Thread Tim Duesterhus
Willy,
Christopher,

most of the patch is moving around the config parser to support ingesting the
new argument.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This adds an option to supress `../` at the start of the resulting path.
---
 doc/configuration.txt  | 11 ++
 include/haproxy/action-t.h |  1 +
 include/haproxy/uri_normalizer.h   |  2 +-
 reg-tests/http-rules/normalize_uri.vtc | 16 ++
 src/http_act.c | 29 +++---
 src/uri_normalizer.c   | 22 ---
 6 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index eacd8ff26..3422d3aa6 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6012,6 +6012,9 @@ http-request early-hint   [ { if | unless } 
 ]
   See RFC 8297 for more information.
 
 http-request normalize-uri  [ { if | unless }  ]
+http-request normalize-uri dotdot [ full ] [ { if | unless }  ]
+http-request normalize-uri merge-slashes [ { if | unless }  ]
+http-request normalize-uri sort-query [ { if | unless }  ]
 
   Performs normalization of the request's URI. The following normalizers are
   available:
@@ -6026,8 +6029,16 @@ http-request normalize-uri  [ { if | unless 
}  ]
   - /foo/../bar/ -> /bar/
   - /foo/bar/../ -> /foo/
   - /../bar/ -> /../bar/
+  - /bar/../../  -> /../
   - /foo//../-> /foo/
 
+  If the "full" option is specified then `../` at the beginning will be
+  removed as well:
+
+  Example:
+  - /../bar/ -> /bar/
+  - /bar/../../  -> /
+
   - merge-slashes: Merges adjacent slashes within the "path" component into a
   single slash.
 
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index 332be513f..ae43a936d 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -104,6 +104,7 @@ enum act_timeout_name {
 enum act_normalize_uri {
ACT_NORMALIZE_URI_MERGE_SLASHES,
ACT_NORMALIZE_URI_DOTDOT,
+   ACT_NORMALIZE_URI_DOTDOT_FULL,
ACT_NORMALIZE_URI_SORT_QUERY,
 };
 
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index 5884e5ab9..52bb004db 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -16,7 +16,7 @@
 
 #include 
 
-struct ist uri_normalizer_path_dotdot(const struct ist path, char *trash, 
size_t len);
+struct ist uri_normalizer_path_dotdot(const struct ist path, int full, char 
*trash, size_t len);
 struct ist uri_normalizer_path_merge_slashes(const struct ist path, char 
*trash, size_t len);
 struct ist uri_normalizer_query_sort(const struct ist query, const char delim, 
char *trash, size_t len);
 
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
index 8be81c574..cb3fa2f63 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -36,8 +36,13 @@ haproxy h1 -conf {
 http-request normalize-uri dotdot
 http-request set-var(txn.after) url
 
+http-request set-uri %[var(txn.before)]
+http-request normalize-uri dotdot full
+http-request set-var(txn.after_full) url
+
 http-response add-header before  %[var(txn.before)]
 http-response add-header after  %[var(txn.after)]
+http-response add-header after-full  %[var(txn.after_full)]
 
 default_backend be
 
@@ -115,56 +120,67 @@ client c2 -connect ${h1_fe_dotdot_sock} {
 rxresp
 expect resp.http.before == "/foo/bar"
 expect resp.http.after == "/foo/bar"
+expect resp.http.after-full == "/foo/bar"
 
 txreq -url "/foo/.."
 rxresp
 expect resp.http.before == "/foo/.."
 expect resp.http.after == "/"
+expect resp.http.after-full == "/"
 
 txreq -url "/foo/../"
 rxresp
 expect resp.http.before == "/foo/../"
 expect resp.http.after == "/"
+expect resp.http.after-full == "/"
 
 txreq -url "/foo/bar/../"
 rxresp
 expect resp.http.before == "/foo/bar/../"
 expect resp.http.after == "/foo/"
+expect resp.http.after-full == "/foo/"
 
 txreq -url "/foo/../bar"
 rxresp
 expect resp.http.before == "/foo/../bar"
 expect resp.http.after == "/bar"
+expect resp.http.after-full == "/bar"
 
 txreq -url "/foo/../bar/"
 rxresp
 expect resp.http.before == "/foo/../bar/"
 expect resp.http.after == "/bar/"
+expect resp.http.after-full == "/bar/"
 
 txreq -url "/foo/../../bar/"
 rxresp
 expect resp.http.before == "/foo/../../bar/"
 expect resp.http.after == "/../bar/"
+expect resp.http.after-full == "/bar/"
 
 txreq -url "/foo//../../bar/"
 rxresp
 expect resp.http.before == "/foo//../../bar/"
 expect resp.http.after == "/bar/"
+expect resp.http.after-full == "/bar/"
 
 txreq -url "/foo/?bar=/foo/../"
 

[RFC PATCH 4/8] MINOR: uri_normalizer: Add a `sort-query` normalizer

2021-04-08 Thread Tim Duesterhus
Willy,
Christopher,

This one comes with dynamic allocation. The next patch will add an optimization
for a small number of arguments. However dynamic allocation within the main
processing logic is pretty ugly, so this should be looked at further.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This normalizer sorts the `&` delimited query parameters by parameter name.

See GitHub Issue #714.
---
 doc/configuration.txt  |  9 +++
 include/haproxy/action-t.h |  1 +
 include/haproxy/uri_normalizer.h   |  1 +
 reg-tests/http-rules/normalize_uri.vtc | 81 +-
 src/http_act.c | 23 
 src/uri_normalizer.c   | 80 +
 6 files changed, 194 insertions(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 99c845ac7..eacd8ff26 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6035,6 +6035,15 @@ http-request normalize-uri  [ { if | unless 
}  ]
   - //-> /
   - /foo//bar -> /foo/bar
 
+  - sort-query: Sorts the query string parameters by parameter name.
+  Parameters are assumed to be delimited by '&'. Shorter names sort before
+  longer names and identical parameter names maintain their relative order.
+
+  Example:
+  - /?c=3=1=2 -> /?a=1=2=3
+  - /?aaa=3=1=2  -> /?a=1=2=3
+  - /?a=3=4=1=5=2 -> /?a=3=1=2=4=5
+
 http-request redirect  [ { if | unless }  ]
 
   This performs an HTTP redirection based on a redirect rule. This is exactly
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index ac9399a6b..332be513f 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -104,6 +104,7 @@ enum act_timeout_name {
 enum act_normalize_uri {
ACT_NORMALIZE_URI_MERGE_SLASHES,
ACT_NORMALIZE_URI_DOTDOT,
+   ACT_NORMALIZE_URI_SORT_QUERY,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/include/haproxy/uri_normalizer.h b/include/haproxy/uri_normalizer.h
index 99b27330e..5884e5ab9 100644
--- a/include/haproxy/uri_normalizer.h
+++ b/include/haproxy/uri_normalizer.h
@@ -18,6 +18,7 @@
 
 struct ist uri_normalizer_path_dotdot(const struct ist path, char *trash, 
size_t len);
 struct ist uri_normalizer_path_merge_slashes(const struct ist path, char 
*trash, size_t len);
+struct ist uri_normalizer_query_sort(const struct ist query, const char delim, 
char *trash, size_t len);
 
 #endif /* _HAPROXY_URI_NORMALIZER_H */
 
diff --git a/reg-tests/http-rules/normalize_uri.vtc 
b/reg-tests/http-rules/normalize_uri.vtc
index e66bdc47b..8be81c574 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -8,7 +8,7 @@ feature ignore_unknown_macro
 server s1 {
 rxreq
 txresp
-} -repeat 21 -start
+} -repeat 34 -start
 
 haproxy h1 -conf {
 defaults
@@ -41,6 +41,18 @@ haproxy h1 -conf {
 
 default_backend be
 
+frontend fe_sort_query
+bind "fd@${fe_sort_query}"
+
+http-request set-var(txn.before) url
+http-request normalize-uri sort-query
+http-request set-var(txn.after) url
+
+http-response add-header before  %[var(txn.before)]
+http-response add-header after  %[var(txn.after)]
+
+default_backend be
+
 backend be
 server s1 ${s1_addr}:${s1_port}
 
@@ -154,3 +166,70 @@ client c2 -connect ${h1_fe_dotdot_sock} {
 expect resp.http.before == "*"
 expect resp.http.after == "*"
 } -run
+
+client c3 -connect ${h1_fe_sort_query_sock} {
+txreq -url "/?a=a"
+rxresp
+expect resp.http.before == "/?a=a"
+expect resp.http.after == "/?a=a"
+
+txreq -url "/?a=a=z"
+rxresp
+expect resp.http.before == "/?a=a=z"
+expect resp.http.after == "/?a=a=z"
+
+txreq -url "/?z=z=a"
+rxresp
+expect resp.http.before == "/?z=z=a"
+expect resp.http.after == "/?a=a=z"
+
+txreq -url "/?a=z=a"
+rxresp
+expect resp.http.before == "/?a=z=a"
+expect resp.http.after == "/?a=z=a"
+
+txreq -url "/?z=a=z"
+rxresp
+expect resp.http.before == "/?z=a=z"
+expect resp.http.after == "/?a=z=a"
+
+txreq -url "/?c"
+rxresp
+expect resp.http.before == "/?c"
+expect resp.http.after == "/?a"
+
+txreq -url "/?a"
+rxresp
+expect resp.http.before == "/?a"
+expect resp.http.after == "/?a"
+
+txreq -url "/?"
+rxresp
+expect resp.http.before == "/?"
+expect resp.http.after == "/?a"
+
+txreq -url "/?a=5=3=1=2=4"
+rxresp
+expect resp.http.before == "/?a=5=3=1=2=4"
+expect resp.http.after == "/?a=5=3=1=2=4"
+
+txreq -url "/?a=5=3=1=2=4"
+rxresp
+expect resp.http.before == "/?a=5=3=1=2=4"
+expect resp.http.after == "/?a=5=1=2=3=4"
+
+txreq -url "/"
+rxresp
+expect resp.http.before == "/"
+

[RFC PATCH 0/8] URI normalization / Issue #714

2021-04-08 Thread Tim Duesterhus
Willy,
Christopher,

Not sure who of you is better suited to review this series, so I'm adding both
of you :-)

I'm tagging this as RFC, because it's large and quite a bit outside of my
comfort zone. However the patches are as clean as possible. They include full
documentation and each normalizer comes with a bunch of reg-tests ensuring they
behave like I expect them to behave. So if you have nothing to complain, then
this series is in a mergeable state. I plan to add a few more normalizers after
this passes an initial review.

I'll add additional remarks to each patch, explaining my decisions in more
detail.

Best regards

Tim Düsterhus (8):
  MINOR: uri_normalizer: Add uri_normalizer module
  MINOR: uri_normalizer: Add `http-request normalize-uri`
  MINOR: uri_normalizer: Add a `dotdot` normalizer to http-request
normalize-uri
  MINOR: uri_normalizer: Add a `sort-query` normalizer
  OPTIMIZE: uri_normalizer: Optimize allocations in
uri_normalizer_query_sort
  MINOR: uri_normalizer: Add support for supressing leading `../` for
dotdot normalizer
  MINOR: uri_normalizer: Support returning detailed errors from uri
normalization
  MINOR: uri_normalizer: Add a `percent-upper` normalizer

 Makefile   |   3 +-
 doc/configuration.txt  |  58 +
 include/haproxy/action-t.h |   9 +
 include/haproxy/uri_normalizer-t.h |  32 +++
 include/haproxy/uri_normalizer.h   |  33 +++
 reg-tests/http-rules/normalize_uri.vtc | 314 +
 src/http_act.c | 213 +
 src/uri_normalizer.c   | 298 +++
 8 files changed, 959 insertions(+), 1 deletion(-)
 create mode 100644 include/haproxy/uri_normalizer-t.h
 create mode 100644 include/haproxy/uri_normalizer.h
 create mode 100644 reg-tests/http-rules/normalize_uri.vtc
 create mode 100644 src/uri_normalizer.c

-- 
2.31.1




[PATCH] CLEANUP: Remove useless malloc() casts

2021-04-08 Thread Tim Duesterhus
Willy,

apparently I still had this one lying around. Found it while cleaning up local
branches :-)

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This is not C++.
---
 include/haproxy/chunk.h| 2 +-
 include/haproxy/listener.h | 2 +-
 src/ev_select.c| 4 ++--
 src/http_htx.c | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/haproxy/chunk.h b/include/haproxy/chunk.h
index b5052d135..af9ef816b 100644
--- a/include/haproxy/chunk.h
+++ b/include/haproxy/chunk.h
@@ -283,7 +283,7 @@ static inline char *chunk_dup(struct buffer *dst, const 
struct buffer *src)
if (dst->size < src->size || !src->size)
dst->size++;
 
-   dst->area = (char *)malloc(dst->size);
+   dst->area = malloc(dst->size);
if (!dst->area) {
dst->head = 0;
dst->data = 0;
diff --git a/include/haproxy/listener.h b/include/haproxy/listener.h
index e4cea50fc..17fe9e090 100644
--- a/include/haproxy/listener.h
+++ b/include/haproxy/listener.h
@@ -179,7 +179,7 @@ unsigned int bind_map_thread_id(const struct bind_conf 
*conf, unsigned int r);
 static inline struct bind_conf *bind_conf_alloc(struct proxy *fe, const char 
*file,
  int line, const char *arg, struct xprt_ops 
*xprt)
 {
-   struct bind_conf *bind_conf = (void *)calloc(1, sizeof(struct 
bind_conf));
+   struct bind_conf *bind_conf = calloc(1, sizeof(struct bind_conf));
 
bind_conf->file = strdup(file);
bind_conf->line = line;
diff --git a/src/ev_select.c b/src/ev_select.c
index 1e9c48cf5..ab5da35b7 100644
--- a/src/ev_select.c
+++ b/src/ev_select.c
@@ -226,10 +226,10 @@ static int init_select_per_thread()
int fd_set_bytes;
 
fd_set_bytes = sizeof(fd_set) * (global.maxsock + FD_SETSIZE - 1) / 
FD_SETSIZE;
-   tmp_evts[DIR_RD] = (fd_set *)calloc(1, fd_set_bytes);
+   tmp_evts[DIR_RD] = calloc(1, fd_set_bytes);
if (tmp_evts[DIR_RD] == NULL)
goto fail;
-   tmp_evts[DIR_WR] = (fd_set *)calloc(1, fd_set_bytes);
+   tmp_evts[DIR_WR] = calloc(1, fd_set_bytes);
if (tmp_evts[DIR_WR] == NULL)
goto fail;
return 1;
diff --git a/src/http_htx.c b/src/http_htx.c
index a7951c01d..53732d146 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -909,7 +909,7 @@ int http_str_to_htx(struct buffer *buf, struct ist raw, 
char **errmsg)
}
 
buf->size = global.tune.bufsize;
-   buf->area = (char *)malloc(buf->size);
+   buf->area = malloc(buf->size);
if (!buf->area)
goto error;
 
-- 
2.31.1




[PATCH 0/3] Additional ist functions

2021-04-05 Thread Tim Duesterhus
Willy,

some more `ist` helper functions that allows consumers to avoid directly
operating on the raw pointer, instead using safe high level functions.

These will be used in a future series of mine. I'm sending them for early
review and integration, because I believe their existence is useful on its
own.

Best regards

Tim Düsterhus (3):
  MINOR: ist: Add `istappend(struct ist, char)`
  MINOR: ist: Add `istshift(struct ist*)`
  MINOR: ist: Add `istsplit(struct ist*, char)`

 include/import/ist.h | 39 +++
 1 file changed, 39 insertions(+)

-- 
2.31.1




[PATCH 1/3] MINOR: ist: Add `istappend(struct ist, char)`

2021-04-05 Thread Tim Duesterhus
This function appends the given char to the given `ist` and returns
the resulting `ist`.
---
 include/import/ist.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/include/import/ist.h b/include/import/ist.h
index 1262e8f59..3f63ed2dd 100644
--- a/include/import/ist.h
+++ b/include/import/ist.h
@@ -407,6 +407,16 @@ static inline int istneq(const struct ist ist1, const 
struct ist ist2, size_t co
return isteq(l, r);
 }
 
+/* appends  after . The caller must ensure that the underlying buffer
+ * is large enough to fit the character.
+ */
+static inline struct ist istappend(struct ist dst, const char src)
+{
+   dst.ptr[dst.len++] = src;
+
+   return dst;
+}
+
 /* copies  over  for a maximum of  bytes. Returns the number
  * of characters copied (src.len), or -1 if it does not fit. In all cases, the
  * contents are copied prior to reporting an error, so that the destination
-- 
2.31.1




[PATCH 2/3] MINOR: ist: Add `istshift(struct ist*)`

2021-04-05 Thread Tim Duesterhus
istshift() returns the first character and advances the ist by 1.
---
 include/import/ist.h | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/include/import/ist.h b/include/import/ist.h
index 3f63ed2dd..6ece7cdf9 100644
--- a/include/import/ist.h
+++ b/include/import/ist.h
@@ -240,6 +240,21 @@ static inline struct ist istnext(const struct ist ist)
return ret;
 }
 
+/* Returns the first character of the  and advances the  by 1.
+ * If the  is empty the result is undefined.
+ */
+static inline char istshift(struct ist *ist)
+{
+   if (ist->len) {
+   char c = *ist->ptr;
+   *ist = istnext(*ist);
+
+   return c;
+   }
+
+   return 0;
+}
+
 /* copies the contents from string  to buffer  and adds a trailing
  * zero. The caller must ensure  is large enough.
  */
-- 
2.31.1




[PATCH 3/3] MINOR: ist: Add `istsplit(struct ist*, char)`

2021-04-05 Thread Tim Duesterhus
istsplit is a combination of iststop + istadv.
---
 include/import/ist.h | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/include/import/ist.h b/include/import/ist.h
index 6ece7cdf9..daf23d552 100644
--- a/include/import/ist.h
+++ b/include/import/ist.h
@@ -801,6 +801,20 @@ static inline struct ist istadv(const struct ist ist, 
const size_t nb)
return ist2(ist.ptr + nb, ist.len - nb);
 }
 
+/* Splits the given  at the given character. The returned ist is
+ * equivalent to iststop(ist, delim). The passed  will contain the
+ * remainder of the string, not including the delimiter. In other words
+ * it will be advanced by the length of the returned string plus 1.
+ */
+static inline struct ist istsplit(struct ist *ist, char delim)
+{
+   const struct ist result = iststop(*ist, delim);
+
+   *ist = istadv(*ist, result.len + 1);
+
+   return result;
+}
+
 /*
  * compare 2 ists and return non-zero if they are the same
  */
-- 
2.31.1




[PATCH 2/2] CLEANUP: ist: Remove unused `count` argument from `ist2str*`

2021-04-03 Thread Tim Duesterhus
This argument is not being used inside the function (and the functions
themselves are unused as well) and not documented. Its purpose is not clear.
Just remove it.
---
 include/import/ist.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/import/ist.h b/include/import/ist.h
index 1262e8f59..5318a0349 100644
--- a/include/import/ist.h
+++ b/include/import/ist.h
@@ -529,7 +529,7 @@ static inline struct ist ist2bin(char *dst, const struct 
ist src)
  * already been checked. An ist made of the output and its length (not counting
  * the trailing zero) are returned.
  */
-static inline struct ist ist2str(char *dst, const struct ist src, size_t count)
+static inline struct ist ist2str(char *dst, const struct ist src)
 {
size_t ofs = 0;
 
@@ -574,7 +574,7 @@ static inline struct ist ist2bin_lc(char *dst, const struct 
ist src)
  * the frame length has already been checked. An ist made of the output and its
  * length (not counting the trailing zero) are returned.
  */
-static inline struct ist ist2str_lc(char *dst, const struct ist src, size_t 
count)
+static inline struct ist ist2str_lc(char *dst, const struct ist src)
 {
size_t ofs = 0;
 
@@ -619,7 +619,7 @@ static inline struct ist ist2bin_uc(char *dst, const struct 
ist src)
  * the frame length has already been checked. An ist made of the output and its
  * length (not counting the trailing zero) are returned.
  */
-static inline struct ist ist2str_uc(char *dst, const struct ist src, size_t 
count)
+static inline struct ist ist2str_uc(char *dst, const struct ist src)
 {
size_t ofs = 0;
 
-- 
2.31.1




  1   2   3   4   5   >