From 4f777a626805123f5356546736c121ab40c68576 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 4 Nov 2026 08:52:44 +0100 Subject: [PATCH] MINOR: h1: rearrange the header value parser to better deal with CTL chars The header value parser only made a special case of NUL, CR and LF in its core loop, which doesn't ease detection of other CTL chars. Let's first slightly rearrange the loop to make the special case of chars below 0x20 first. The checks were only moved inside a new "if" block but nothing was changed. Use diff -b to better see. --- src/h1.c | 55 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/src/h1.c b/src/h1.c index e3f55e3a0..a7455c296 100644 --- a/src/h1.c +++ b/src/h1.c @@ -936,21 +936,21 @@ int h1_headers_to_hdr_list(char *start, const char *stop, * points to the first character of the value. */ - /* speedup: we'll skip packs of 4 or 8 bytes not containing bytes 0x0D + /* speedup: we'll skip packs of 4 or 8 bytes not containing bytes 0x1F * and lower. In fact since most of the time is spent in the loop, we - * also remove the sign bit test so that bytes 0x8e..0x0d break the + * also remove the sign bit test so that bytes 0xA0..0x1F break the * loop, but we don't care since they're very rare in header values. */ #ifdef HA_UNALIGNED_LE64 while (ptr <= end - sizeof(long)) { - if (is_char8_below_opt(*(ulong *)ptr, 0x0e)) + if (is_char8_outside(*(ulong *)ptr, 0x20, 0x7e)) goto http_msg_hdr_val2; ptr += sizeof(long); } #endif #ifdef HA_UNALIGNED_LE while (ptr <= end - sizeof(int)) { - if (is_char4_below_opt(*(uint *)ptr, 0x0e)) + if (is_char4_outside(*(uint *)ptr, 0x20, 0x7e)) goto http_msg_hdr_val2; ptr += sizeof(int); } @@ -960,30 +960,35 @@ int h1_headers_to_hdr_list(char *start, const char *stop, goto http_msg_ood; } http_msg_hdr_val2: - if (likely(!*ptr)) { - /* RFC9110 clarified that NUL is explicitly forbidden in header values - * (like CR and LF). - */ - if (h1m->err_pos < -1 || /* PR_O2_R*BUG_OK not set */ - (!(h1m->flags & H1_MF_RESP) && !(global.acc_hreq_vio & HTTP_VIO_HDR_VALUE_NUL)) || - ((h1m->flags & H1_MF_RESP) && !(global.acc_hres_vio & HTTP_VIO_HDR_VALUE_NUL))) { - state = H1_MSG_HDR_VAL; - goto http_msg_invalid; + if (unlikely((uchar)*ptr < 0x20 || *ptr == 0x7f)) { /* NUL, CTL, TAB, CR/LF */ + if (likely(!*ptr)) { + /* RFC9110 clarified that NUL is explicitly forbidden in header values + * (like CR and LF). + */ + if (h1m->err_pos < -1 || /* PR_O2_R*BUG_OK not set */ + (!(h1m->flags & H1_MF_RESP) && !(global.acc_hreq_vio & HTTP_VIO_HDR_VALUE_NUL)) || + ((h1m->flags & H1_MF_RESP) && !(global.acc_hres_vio & HTTP_VIO_HDR_VALUE_NUL))) { + state = H1_MSG_HDR_VAL; + goto http_msg_invalid; + } + if (h1m->err_pos == -1) /* PR_O2_REQBUG_OK set: just log */ + h1m->err_pos = ptr - start + skip; + } + + if (likely(HTTP_IS_CRLF(*ptr))) { + eol = ptr - start; + /* Note: we could also copy eol into ->eoh so that we have the + * real header end in case it ends with lots of LWS, but is this + * really needed ? + */ + if (likely(*ptr == '\r')) + EAT_AND_JUMP_OR_RETURN(ptr, end, http_msg_hdr_l2_lf, http_msg_ood, state, H1_MSG_HDR_L2_LF); + goto http_msg_hdr_l2_lf; } - if (h1m->err_pos == -1) /* PR_O2_REQBUG_OK set: just log */ - h1m->err_pos = ptr - start + skip; } - if (likely(!HTTP_IS_CRLF(*ptr))) - EAT_AND_JUMP_OR_RETURN(ptr, end, http_msg_hdr_val2, http_msg_ood, state, H1_MSG_HDR_VAL); - eol = ptr - start; - /* Note: we could also copy eol into ->eoh so that we have the - * real header end in case it ends with lots of LWS, but is this - * really needed ? - */ - if (likely(*ptr == '\r')) - EAT_AND_JUMP_OR_RETURN(ptr, end, http_msg_hdr_l2_lf, http_msg_ood, state, H1_MSG_HDR_L2_LF); - goto http_msg_hdr_l2_lf; + /* OK, not < 0x20 or a valid one (e.g. tab) */ + EAT_AND_JUMP_OR_RETURN(ptr, end, http_msg_hdr_val2, http_msg_ood, state, H1_MSG_HDR_VAL); case H1_MSG_HDR_L2_LF: http_msg_hdr_l2_lf: -- 2.47.3