BUG/MEDIUM: chunks: always reject negative-length chunks
The recent addition of "show env" on the CLI has revealed an interesting
design bug. Chunks are supposed to support a negative length to indicate
that they carry no data. chunk_printf() sets this size to -1 if the string
is too large for the buffer. At a few places in the http engine we may end
up with trash.len = -1. But bi_putchk(), chunk_appendf() and a few other
chunks consumers don't consider this case as possible and will use such a
chunk, possibly restoring an invalid string or trying to copy -1 bytes.
This fix takes care of clarifying the situation in a backportable way
where such sizes are used, so that a negative length indicating an error
remains present until the chunk is reinitialized or overwritten. But a
cleaner design adjustment needs to be done so that there's a clear contract
on how to use these chunks. At first glance it doesn't seem *that* useful
to support negative sizes, so probably this is what should change.
This fix must be backported to 1.6 and 1.5.
(cherry picked from commit 320ec2a7454a84a6bb318a8a132d0300b712937f)
(cherry picked from commit 4d747c277358907f1a68a96d890d984d7ebfb0eb)
diff --git a/src/channel.c b/src/channel.c
index 2f98396..36b4e44 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -87,7 +87,7 @@
if (len == 0)
return -1;
- if (len > chn->buf->size) {
+ if (len < 0 || len > chn->buf->size) {
/* we can't write this chunk and will never be able to, because
* it is larger than the buffer. This must be reported as an
* error. Then we return -2 so that writers that don't care can
@@ -157,6 +157,9 @@
if (unlikely(channel_input_closed(chn)))
return -2;
+ if (len < 0)
+ return -3;
+
max = buffer_max_len(chn);
if (unlikely(len > max - buffer_len(chn->buf))) {
/* we can't write this chunk right now because the buffer is
diff --git a/src/chunk.c b/src/chunk.c
index 3c1cfdf..fbba6d5 100644
--- a/src/chunk.c
+++ b/src/chunk.c
@@ -100,7 +100,7 @@
va_list argp;
int ret;
- if (!chk->str || !chk->size)
+ if (chk->len < 0 || !chk->str || !chk->size)
return 0;
va_start(argp, fmt);
@@ -125,6 +125,9 @@
int olen, free;
char c;
+ if (dst->len < 0)
+ return dst->len;
+
olen = dst->len;
for (i = 0; i < src->len; i++) {
@@ -166,6 +169,9 @@
int olen, free;
char c;
+ if (dst->len < 0)
+ return dst->len;
+
olen = dst->len;
for (i = 0; i < src->len; i++) {