BUG/MAJOR: filters/htx: Add a flag to state the payload is altered by a filter

When a filter is registered on the data, it means it may change the payload
length by rewritting data. It means consumers of the message cannot trust the
expected length of payload as announced by the producer. The commit 8bd835b2d2
("MEDIUM: filters/htx: Don't rely on HTX extra field if payload is filtered")
was pushed to solve this issue. When the HTTP payload of a message is filtered,
the extra field is set to 0 to be sure it will never be used by error by any
consumer. However, it is not enough.

Indeed, the filters must be called before fowarding some data. They cannot be
by-passed. But if a consumer is unable to flush the HTX message, some outgoing
data can remain blocked in the channel's buffer. If some new data are then
pushed because there is some room in the channel's buffe, the producer will set
the HTX extra field. At this stage, if the consumer is unblocked and can send
again data, it is possible to call it to forward outgoing data blocked in the
channel's buffer before waking the stream up to filter new input data. It is the
purpose of the data fast-forwarding. In this case, the HTX extra field will be
seen by the consumer. It is unexpected and leads to undefined behavior.

One consequence of this bug is to perform a wrong chunking on compressed
messages, leading to processing errors at the end of the message, reported as
"ID--" in logs.

To fix the bug, a HTX flag is added to state the payload of the current HTX
message is altered. When this flag is set (HTX_FL_ALTERED_PAYLOAD), the HTX
extra field must not be trusted. And to keep things simple, when this flag is
set, the HTX extra field is automatically set to 0 when the HTX message is
loaded, in htxbuf() function.

It is probably the less intrusive way to fix the bug for now. But this part must
be reviewed to save meta-info of the HTX message outside of the message itself.

This commit should solve the issue #2741. It must be backported as far as 2.9.

include/haproxy/htx-t.h

@@ -177,7 +177,7 @@ static forceinline char *hsl_show_flags(char *buf, size_t len, const char *delim
 #define HTX_FL_PARSING_ERROR     0x00000001 /* Set when a parsing error occurred */
 #define HTX_FL_PROCESSING_ERROR  0x00000002 /* Set when a processing error occurred */
 #define HTX_FL_FRAGMENTED        0x00000004 /* Set when the HTX buffer is fragmented */
-/* 0x00000008 unused */
+#define HTX_FL_ALTERED_PAYLOAD   0x00000008 /* The payload is altered, the extra value must not be trusted */
 #define HTX_FL_EOM               0x00000010 /* Set when end-of-message is reached from the HTTP point of view
 					     * (at worst, on the EOM block is missing)
 					     */

include/haproxy/htx.h

@@ -700,6 +700,8 @@ static inline struct htx *htxbuf(const struct buffer *buf)
 		htx->size = buf->size - sizeof(*htx);
 		htx_reset(htx);
 	}
+	if (htx->flags & HTX_FL_ALTERED_PAYLOAD)
+		htx->extra = 0;
 	return htx;
 }
 

src/filters.c

@@ -704,6 +704,7 @@ flt_http_payload(struct stream *s, struct http_msg *msg, unsigned int len)
 	*strm_off += ret;
  end:
 	htx = htxbuf(&msg->chn->buf);
+	htx->flags |= HTX_FL_ALTERED_PAYLOAD;
 	if (msg->flags & HTTP_MSGF_XFER_LEN)
 		htx->extra = 0;
 	DBG_TRACE_LEAVE(STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA|STRM_EV_FLT_ANA, s);