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.
This commit is contained in:
Aurelien DARRAGON 2024-11-06 09:48:32 +01:00
parent 97d3096040
commit 24dd7154a6
3 changed files with 4 additions and 4 deletions

View File

@ -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

View File

@ -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;
}

View File

@ -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;
}