BUG/MINOR: h1: Don't support LF only at the end of chunks

When the message is chunked, all chunks must ends with a CRLF. However, on
old versions, to support bad client or server implementations, the LF only
was also accepted. Nowadays, it seems useless and can even be considered as
an issue. Just forbid LF only at the end of chunks, it seems reasonnable.

This patch must be backported to 2.9 and probably to all stable versions
because there is no reason to still support LF without CR in this case.

diff --git a/include/haproxy/h1.h b/include/haproxy/h1.h
index 8f520595c178738aa62628ca3192143f548f7f89..9283df63e4d525964de543667c9acfab03c1b814 100644 (file)
--- a/include/haproxy/h1.h
+++ b/include/haproxy/h1.h
@@ -203,8 +203,8 @@ static inline const char *h1m_state_str(enum h1m_state msg_state)
        }
 }
 
-/* This function may be called only in HTTP_MSG_CHUNK_CRLF. It reads the CRLF or
- * a possible LF alone at the end of a chunk. The caller should adjust msg->next
+/* This function may be called only in HTTP_MSG_CHUNK_CRLF. It reads the CRLF
+ * at the end of a chunk. The caller should adjust msg->next
  * in order to include this part into the next forwarding phase.  Note that the
  * caller must ensure that head+start points to the first byte to parse.  It
  * returns the number of bytes parsed on success, so the caller can set msg_state
@@ -221,16 +221,17 @@ static inline int h1_skip_chunk_crlf(const struct buffer *buf, int start, int st
        if (stop <= start)
                return 0;
 
+       if (unlikely(*ptr != '\r')) // negative position to stop
+               return ptr - __b_peek(buf, stop);
+
        /* NB: we'll check data availability at the end. It's not a
         * problem because whatever we match first will be checked
         * against the correct length.
         */
-       if (*ptr == '\r') {
-               bytes++;
-               ptr++;
-               if (ptr >= b_wrap(buf))
-                       ptr = b_orig(buf);
-       }
+       bytes++;
+       ptr++;
+       if (ptr >= b_wrap(buf))
+               ptr = b_orig(buf);
 
        if (bytes > stop - start)
                return 0;

diff --git a/src/h1_htx.c b/src/h1_htx.c
index f3b61b70ed4168c4fa2f9d35b79b23ac318c1e78..b8c6ccb0567972ce7b5830627631ffc045cd376d 100644 (file)
--- a/src/h1_htx.c
+++ b/src/h1_htx.c
@@ -740,9 +740,12 @@ static size_t h1_parse_full_contig_chunks(struct h1m *h1m, struct htx **dsthtx,
                dpos += chksz;
                ridx += chksz;
 
-               /* Parse CRLF or LF (always present) */
-               if (likely(end[ridx] == '\r'))
-                       ++ridx;
+               /* Parse CRLF */
+               if (unlikely(end[ridx] != '\r')) {
+                       h1m->state = H1_MSG_CHUNK_CRLF;
+                       goto parsing_error;
+               }
+               ++ridx;
                if (end[ridx] != '\n') {
                        h1m->state = H1_MSG_CHUNK_CRLF;
                        goto parsing_error;