MINOR: http: don't %-encode the payload when not relevant

As reported by Pierre Maoui in GH #2477, it's not possible to render
control chars from variables or expressions verbatim in the payload part
of http-return statements. That's a problem because this part should not
require to be encoded at all (we could even imagine building favicons on
the fly for example).

In fact it is the LOG_OPT_HTTP option when passed as default options on
parse_logformat_string() which tells the log encoder that the payload
should be http-encoded using lf_chunk() instead of being printed using the
per-type encoder.

This option was set when parsing logformat expressions for lf-string
expression under http-return statements, as well as logformat expressions
for set-map action. While it is true that those actions may only be
used under http context, the LOG_OPT_HTTP logformat option is not relevant
there, because the payload is expected to be used without being encoded.

So let's simply get rid of this option when parsing logformat expressions
for set-map action key/value and lf-string from http-request return
action, and add a note next to LOG_OPT_HTTP option to indicate that it is
used to tell the log encoder that the payload should be HTTP-encoded.

Thanks to Pierre for having reported the issue and Willy for the
analysis and patch proposal.

diff --git a/include/haproxy/log-t.h b/include/haproxy/log-t.h
index 7447c862d3093257ee2d05c0e0a1d650bb60470a..a2cd187548ab5676d80b68a77c518ffdf188fc7a 100644 (file)
--- a/include/haproxy/log-t.h
+++ b/include/haproxy/log-t.h
@@ -44,7 +44,7 @@
 #define LOG_OPT_QUOTE           0x00000004
 #define LOG_OPT_REQ_CAP         0x00000008
 #define LOG_OPT_RES_CAP         0x00000010
-#define LOG_OPT_HTTP            0x00000020
+#define LOG_OPT_HTTP            0x00000020 // forces http-encoding on the payload, incompatible with LOG_OPT_ENCODE
 #define LOG_OPT_ESC             0x00000040
 #define LOG_OPT_MERGE_SPACES    0x00000080
 #define LOG_OPT_BIN             0x00000100

diff --git a/src/http_act.c b/src/http_act.c
index f0a19f362ec0b92dce72af9848ddca29cba8aa4a..b7cc5f209ed0fc658b212a6d539883348b804ca0 100644 (file)
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -1952,7 +1952,7 @@ static enum act_parse_ret parse_http_set_map(const char **args, int *orig_arg, s
 
        /* key pattern */
        lf_expr_init(&rule->arg.map.key);
-       if (!parse_logformat_string(args[cur_arg], px, &rule->arg.map.key, LOG_OPT_HTTP, cap, err)) {
+       if (!parse_logformat_string(args[cur_arg], px, &rule->arg.map.key, LOG_OPT_NONE, cap, err)) {
                free(rule->arg.map.ref);
                return ACT_RET_PRS_ERR;
        }
@@ -1961,7 +1961,7 @@ static enum act_parse_ret parse_http_set_map(const char **args, int *orig_arg, s
                /* value pattern for set-map only */
                cur_arg++;
                lf_expr_init(&rule->arg.map.value);
-               if (!parse_logformat_string(args[cur_arg], px, &rule->arg.map.value, LOG_OPT_HTTP, cap, err)) {
+               if (!parse_logformat_string(args[cur_arg], px, &rule->arg.map.value, LOG_OPT_NONE, cap, err)) {
                        free(rule->arg.map.ref);
                        return ACT_RET_PRS_ERR;
                }

diff --git a/src/http_htx.c b/src/http_htx.c
index 36356edf021f9f35e3d4a2d16c3954afb41356e5..1e0e94066798eb034a420cfb18bdcdaa2cae7de3 100644 (file)
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -1799,7 +1799,7 @@ struct http_reply *http_parse_http_reply(const char **args, int *orig_arg, struc
                        memprintf(errmsg, "a content type must be defined with a log-format payload");
                        goto error;
                }
-               if (!parse_logformat_string(obj, px, &reply->body.fmt, LOG_OPT_HTTP, cap, errmsg))
+               if (!parse_logformat_string(obj, px, &reply->body.fmt, LOG_OPT_NONE, cap, errmsg))
                        goto error;
        }