MINOR: http-htx: Add understandable errors for the errorfiles parsing
authorChristopher Faulet <cfaulet@haproxy.com>
Thu, 5 Nov 2020 21:43:41 +0000 (22:43 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Fri, 6 Nov 2020 09:00:32 +0000 (10:00 +0100)
No details are provided when an error occurs during the parsing of an errorfile,
Thus it is a bit hard to diagnose where the problem is. Now, when it happens, an
understandable error message is reported.

This patch is not a bug fix in itself. But it will be required to change an
fatal error into a warning in last stable releases. Thus it must be backported
as far as 2.0.

(cherry picked from commit a66adf41ea28a0fa29437d1675f225b5cc589b59)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 3a10710b777370ef36763b8b6658ae63ef5c4ec4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 58f55acf4eafd19ed35ad8506064198229ba69c3)
[cf: Adapted for the 2.1]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

include/proto/http_htx.h
src/cfgparse-listen.c
src/http_htx.c

index 5aff678..0fd543a 100644 (file)
@@ -48,6 +48,6 @@ unsigned int http_get_htx_hdr(const struct htx *htx, const struct ist hdr,
                              int occ, struct http_hdr_ctx *ctx, char **vptr, size_t *vlen);
 unsigned int http_get_htx_fhdr(const struct htx *htx, const struct ist hdr,
                               int occ, struct http_hdr_ctx *ctx, char **vptr, size_t *vlen);
-int http_str_to_htx(struct buffer *buf, struct ist raw);
+int http_str_to_htx(struct buffer *buf, struct ist raw, char **errmsg);
 
 #endif /* _PROTO_HTTP_HTX_H */
index f54912f..62d1388 100644 (file)
@@ -3997,13 +3997,19 @@ stats_error_parsing:
                        if (http_err_codes[rc] == errnum) {
                                struct buffer chk;
 
-                               if (!http_str_to_htx(&chk, ist2(err, errlen))) {
-                                       ha_alert("parsing [%s:%d] : unable to convert message in HTX for HTTP return code %d.\n",
-                                                file, linenum, http_err_codes[rc]);
+                               if (!http_str_to_htx(&chk, ist2(err, errlen), &errmsg)) {
+                                       ha_alert("parsing [%s:%d] : invalid error message: %s.\n",
+                                                file, linenum, errmsg);
                                        err_code |= ERR_ALERT | ERR_FATAL;
                                        free(err);
                                        goto out;
                                }
+                               if (errmsg) {
+                                       ha_warning("parsing [%s:%d] : %s\n", file, linenum, errmsg);
+                                       err_code |= ERR_WARN;
+                                       free(errmsg);
+                                       errmsg = NULL;
+                               }
                                chunk_destroy(&curproxy->errmsg[rc]);
                                curproxy->errmsg[rc] = chk;
                                break;
@@ -4066,13 +4072,19 @@ stats_error_parsing:
                        if (http_err_codes[rc] == errnum) {
                                struct buffer chk;
 
-                               if (!http_str_to_htx(&chk, ist2(err, errlen))) {
-                                       ha_alert("parsing [%s:%d] : unable to convert message in HTX for HTTP return code %d.\n",
-                                                file, linenum, http_err_codes[rc]);
+                               if (!http_str_to_htx(&chk, ist2(err, errlen), &errmsg)) {
+                                       ha_alert("parsing [%s:%d] : %s: %s.\n",
+                                                file, linenum, args[2], errmsg);
                                        err_code |= ERR_ALERT | ERR_FATAL;
                                        free(err);
                                        goto out;
                                }
+                               if (errmsg) {
+                                       ha_warning("parsing [%s:%d] : %s\n", file, linenum, errmsg);
+                                       err_code |= ERR_WARN;
+                                       free(errmsg);
+                                       errmsg = NULL;
+                               }
                                chunk_destroy(&curproxy->errmsg[rc]);
                                curproxy->errmsg[rc] = chk;
                                break;
index 91f6a17..8158395 100644 (file)
@@ -755,7 +755,7 @@ unsigned int http_get_htx_fhdr(const struct htx *htx, const struct ist hdr,
        return 1;
 }
 
-int http_str_to_htx(struct buffer *buf, struct ist raw)
+int http_str_to_htx(struct buffer *buf, struct ist raw, char **errmsg)
 {
        struct htx *htx;
        struct htx_sl *sl;
@@ -781,17 +781,24 @@ int http_str_to_htx(struct buffer *buf, struct ist raw)
        h1m.flags |= H1_MF_NO_PHDR;
        ret = h1_headers_to_hdr_list(raw.ptr, raw.ptr + raw.len,
                                     hdrs, sizeof(hdrs)/sizeof(hdrs[0]), &h1m, &h1sl);
-       if (ret <= 0)
+       if (ret <= 0) {
+               memprintf(errmsg, "unabled to parse headers (error offset: %d)", h1m.err_pos);
                goto error;
+       }
 
-       if (unlikely(h1sl.st.v.len != 8))
+       if (unlikely(h1sl.st.v.len != 8)) {
+               memprintf(errmsg, "invalid http version (%.*s)", (int)h1sl.st.v.len, h1sl.st.v.ptr);
                goto error;
+       }
        if ((*(h1sl.st.v.ptr + 5) > '1') ||
            ((*(h1sl.st.v.ptr + 5) == '1') && (*(h1sl.st.v.ptr + 7) >= '1')))
                h1m.flags |= H1_MF_VER_11;
 
-       if (h1sl.st.status < 200 && (h1sl.st.status == 100 || h1sl.st.status >= 102))
+       if (h1sl.st.status < 200 && (h1sl.st.status == 100 || h1sl.st.status >= 102)) {
+               memprintf(errmsg, "invalid http status code for an error message (%u)",
+                         h1sl.st.status);
                goto error;
+       }
 
        if (h1sl.st.status == 204 || h1sl.st.status == 304) {
                /* Responses known to have no body. */
@@ -808,8 +815,10 @@ int http_str_to_htx(struct buffer *buf, struct ist raw)
                flags |= HTX_SL_F_XFER_ENC;
        if (h1m.flags & H1_MF_XFER_LEN) {
                flags |= HTX_SL_F_XFER_LEN;
-               if (h1m.flags & H1_MF_CHNK)
-                       goto error; /* Unsupported because there is no body parsing */
+               if (h1m.flags & H1_MF_CHNK) {
+                       memprintf(errmsg, "chunk-encoded payload not supported");
+                       goto error;
+               }
                else if (h1m.flags & H1_MF_CLEN) {
                        flags |= HTX_SL_F_CLEN;
                        if (h1m.body_len == 0)
@@ -819,26 +828,37 @@ int http_str_to_htx(struct buffer *buf, struct ist raw)
                        flags |= HTX_SL_F_BODYLESS;
        }
 
-       if ((flags & HTX_SL_F_BODYLESS) && raw.len > ret)
-               goto error; /* No body expected */
-       if ((flags & HTX_SL_F_CLEN) && h1m.body_len != (raw.len - ret))
-               goto error; /* body with wrong length */
+       if ((flags & HTX_SL_F_BODYLESS) && raw.len > ret) {
+               memprintf(errmsg, "message payload not expected");
+               goto error;
+       }
+       if ((flags & HTX_SL_F_CLEN) && h1m.body_len != (raw.len - ret)) {
+               memprintf(errmsg, "payload size does not match the announced content-length (%lu != %lu)",
+                         (raw.len - ret), h1m.body_len);
+               goto error;
+       }
 
        htx = htx_from_buf(buf);
        sl = htx_add_stline(htx, HTX_BLK_RES_SL, flags, h1sl.st.v, h1sl.st.c, h1sl.st.r);
-       if (!sl || !htx_add_all_headers(htx, hdrs))
+       if (!sl || !htx_add_all_headers(htx, hdrs)) {
+               memprintf(errmsg, "unable to add headers into the HTX message");
                goto error;
+       }
        sl->info.res.status = h1sl.st.status;
 
        while (raw.len > ret) {
                int sent = htx_add_data(htx, ist2(raw.ptr + ret, raw.len - ret));
-               if (!sent)
+               if (!sent) {
+                       memprintf(errmsg, "unable to add payload into the HTX message");
                        goto error;
+               }
                ret += sent;
        }
 
-       if (!htx_add_endof(htx, HTX_BLK_EOM))
+       if (!htx_add_endof(htx, HTX_BLK_EOM)) {
+               memprintf(errmsg, "unable to add EOM into the HTX message");
                goto error;
+       }
 
        return 1;
 
@@ -852,22 +872,32 @@ static int http_htx_init(void)
 {
        struct buffer chk;
        struct ist raw;
+       char *errmsg = NULL;
        int rc;
        int err_code = 0;
 
        for (rc = 0; rc < HTTP_ERR_SIZE; rc++) {
                if (!http_err_msgs[rc]) {
-                       ha_alert("Internal error: no message defined for HTTP return code %d", rc);
+                       ha_alert("Internal error: no default message defined for HTTP return code %d", rc);
                        err_code |= ERR_ALERT | ERR_FATAL;
                        continue;
                }
 
                raw = ist2(http_err_msgs[rc], strlen(http_err_msgs[rc]));
-               if (!http_str_to_htx(&chk, raw)) {
-                       ha_alert("Internal error: Unable to convert message in HTX for HTTP return code %d.\n",
-                                http_err_codes[rc]);
+               if (!http_str_to_htx(&chk, raw, &errmsg)) {
+                       ha_alert("Internal error: invalid default message for HTTP return code %d: %s.\n",
+                                http_err_codes[rc], errmsg);
                        err_code |= ERR_ALERT | ERR_FATAL;
                }
+               else if (errmsg) {
+                       ha_warning("invalid default message for HTTP return code %d: %s.\n", http_err_codes[rc], errmsg);
+                       err_code |= ERR_WARN;
+               }
+
+               /* Reset errmsg */
+               free(errmsg);
+               errmsg = NULL;
+
                http_err_chunks[rc] = chk;
        }
 end: