BUG/MINOR: http: make url_decode() optionally convert '+' to SP

The url_decode() function used by the url_dec converter and a few other
call points is ambiguous on its processing of the '+' character which
itself isn't stable in the spec. This one belongs to the reserved
characters for the query string but not for the path nor the scheme,
in which it must be left as-is. It's only in argument strings that
follow the application/x-www-form-urlencoded encoding that it must be
turned into a space, that is, in query strings and POST arguments.

The problem is that the function is used to process full URLs and
paths in various configs, and to process query strings from the stats
page for example.

This patch updates the function to differentiate the situation where
it's parsing a path and a query string. A new argument indicates if a
query string should be assumed, otherwise it's only assumed after seeing
a question mark.

The various locations in the code making use of this function were
updated to take care of this (most call places were using it to decode
POST arguments).

The url_dec converter is usually called on path or url samples, so it
needs to remain compatible with this and will default to parsing a path
and turning the '+' to a space only after a question mark. However in
situations where it would explicitly be extracted from a POST or a
query string, it now becomes possible to enforce the decoding by passing
a non-null value in argument.

It seems to be what was reported in issue #585. This fix may be
backported to older stable releases.

diff --git a/contrib/prometheus-exporter/service-prometheus.c b/contrib/prometheus-exporter/service-prometheus.c
index 8ffeb82033a92293632a7a570f2f8918d2f06f64..54770ef5bbb1859273368f2e22d01b566a9d97aa 100644 (file)
--- a/contrib/prometheus-exporter/service-prometheus.c
+++ b/contrib/prometheus-exporter/service-prometheus.c
@@ -2292,7 +2292,7 @@ static int promex_parse_uri(struct appctx *appctx, struct stream_interface *si)
                        *(p++) = 0;
                else if (*p == '#')
                        *p = 0;
-               len = url_decode(key);
+               len = url_decode(key, 1);
                if (len == -1)
                        goto error;
 
@@ -2306,7 +2306,7 @@ static int promex_parse_uri(struct appctx *appctx, struct stream_interface *si)
                                *(p++) = 0;
                        else if (*p == '#')
                                *p = 0;
-                       len = url_decode(value);
+                       len = url_decode(value, 1);
                        if (len == -1)
                                goto error;
                }

diff --git a/doc/configuration.txt b/doc/configuration.txt
index f3e6aa147b43c2cd4fe4b9a5521f82db1b219cd2..2dbb893a2487cac04e1012273beec7e5cf640ba2 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -14540,9 +14540,13 @@ upper
   sample fetch function or after a transformation keyword returning a string
   type. The result is of type string.
 
-url_dec
-  Takes an url-encoded string provided as input and returns the decoded
-  version as output. The input and the output are of type string.
+url_dec([<in_form>])
+  Takes an url-encoded string provided as input and returns the decoded version
+  as output. The input and the output are of type string. If the <in_form>
+  argument is set to a non-zero integer value, the input string is assumed to
+  be part of a form or query string and the '+' character will be turned into a
+  space (' '). Otherwise this will only happen after a question mark indicating
+  a query string ('?').
 
 ungrpc(<field_number>,[<field_type>])
   This extracts the protocol buffers message field in raw mode of an input binary

diff --git a/include/common/standard.h b/include/common/standard.h
index 81d6093166f9d0190cd8f1b0a37cfdf055347946..a6c39104c98c0c3411867cc7978462c8b9ec9677 100644 (file)
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -609,8 +609,12 @@ static inline const char *csv_enc(const char *str, int quote,
  * be shorter. If some forbidden characters are found, the conversion is
  * aborted, the string is truncated before the issue and non-zero is returned,
  * otherwise the operation returns non-zero indicating success.
+ * If the 'in_form' argument is non-nul the string is assumed to be part of
+ * an "application/x-www-form-urlencoded" encoded string, and the '+' will be
+ * turned to a space. If it's zero, this will only be done after a question
+ * mark ('?').
  */
-int url_decode(char *string);
+int url_decode(char *string, int in_form);
 
 /* This one is 6 times faster than strtoul() on athlon, but does
  * no check at all.

diff --git a/src/http_conv.c b/src/http_conv.c
index cd93aa9667d403aa47236e9ab3132707622a9a00..f496c560cf7d0e58d992af5f6b65b3a569e56ff8 100644 (file)
--- a/src/http_conv.c
+++ b/src/http_conv.c
@@ -246,6 +246,7 @@ expect_comma:
 /* This fetch url-decode any input string. */
 static int sample_conv_url_dec(const struct arg *args, struct sample *smp, void *private)
 {
+       int in_form = 0;
        int len;
 
        /* If the constant flag is set or if not size is available at
@@ -262,7 +263,11 @@ static int sample_conv_url_dec(const struct arg *args, struct sample *smp, void
 
        /* Add final \0 required by url_decode(), and convert the input string. */
        smp->data.u.str.area[smp->data.u.str.data] = '\0';
-       len = url_decode(smp->data.u.str.area);
+
+       if (args && (args[0].type == ARGT_SINT))
+               in_form = !!args[0].data.sint;
+
+       len = url_decode(smp->data.u.str.area, in_form);
        if (len < 0)
                return 0;
        smp->data.u.str.data = len;
@@ -361,7 +366,7 @@ static struct sample_conv_kw_list sample_conv_kws = {ILH, {
        { "language",       sample_conv_q_preferred,  ARG2(1,STR,STR),  NULL,   SMP_T_STR,  SMP_T_STR},
        { "capture-req",    smp_conv_req_capture,     ARG1(1,SINT),     NULL,   SMP_T_STR,  SMP_T_STR},
        { "capture-res",    smp_conv_res_capture,     ARG1(1,SINT),     NULL,   SMP_T_STR,  SMP_T_STR},
-       { "url_dec",        sample_conv_url_dec,      0,                NULL,   SMP_T_STR,  SMP_T_STR},
+       { "url_dec",        sample_conv_url_dec,      ARG1(0,SINT),     NULL,   SMP_T_STR,  SMP_T_STR},
        { NULL, NULL, 0, 0, 0 },
 }};
 

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index 2f712f3aca6017235698136e6df22570f9d8cacb..63be2d6587a447e2752b3bcc84b53e8912bccfc9 100644 (file)
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -1306,7 +1306,7 @@ static int fcgi_set_default_param(struct fcgi_conn *fconn, struct fcgi_strm *fst
                chunk_memcat(params->p, path.ptr, path.len);
                path.ptr = b_tail(params->p) - path.len;
                path.ptr[path.len] = '\0';
-               len = url_decode(path.ptr);
+               len = url_decode(path.ptr, 0);
                if (len < 0)
                        goto error;
                path.len = len;

diff --git a/src/standard.c b/src/standard.c
index e3e81ba54f028a03e3b4eafd4082a5dfe6b94637..cf2724846ce6b15dd4c8c2df0f5bf57a6c0b0453 100644 (file)
--- a/src/standard.c
+++ b/src/standard.c
@@ -1749,8 +1749,12 @@ const char *csv_enc_append(const char *str, int quote, struct buffer *output)
  * be shorter. If some forbidden characters are found, the conversion is
  * aborted, the string is truncated before the issue and a negative value is
  * returned, otherwise the operation returns the length of the decoded string.
+ * If the 'in_form' argument is non-nul the string is assumed to be part of
+ * an "application/x-www-form-urlencoded" encoded string, and the '+' will be
+ * turned to a space. If it's zero, this will only be done after a question
+ * mark ('?').
  */
-int url_decode(char *string)
+int url_decode(char *string, int in_form)
 {
        char *in, *out;
        int ret = -1;
@@ -1760,7 +1764,7 @@ int url_decode(char *string)
        while (*in) {
                switch (*in) {
                case '+' :
-                       *out++ = ' ';
+                       *out++ = in_form ? ' ' : *in;
                        break;
                case '%' :
                        if (!ishex(in[1]) || !ishex(in[2]))
@@ -1768,6 +1772,9 @@ int url_decode(char *string)
                        *out++ = (hex2i(in[1]) << 4) + hex2i(in[2]);
                        in += 2;
                        break;
+               case '?':
+                       in_form = 1;
+                       /* fall through */
                default:
                        *out++ = *in;
                        break;

diff --git a/src/stats.c b/src/stats.c
index 46475cc8fc5967426f38099ea1441565eeee6158..f76fd379081fa5418d8c24c3c7d8053140f12e96 100644 (file)
--- a/src/stats.c
+++ b/src/stats.c
@@ -2902,7 +2902,7 @@ static int stats_process_http_post(struct stream_interface *si)
                                /* Ok, a value is found, we can mark the end of the key */
                                *value++ = '\0';
                        }
-                       if (url_decode(key) < 0 || url_decode(value) < 0)
+                       if (url_decode(key, 1) < 0 || url_decode(value, 1) < 0)
                                break;
 
                        /* Now we can check the key to see what to do */