[libmicrohttpd] www-form-urlencoded: dealing with keys with no value

[Ethan Tuttle]

Given post body "a&b=1", how should MHD interpret the data?

I'm at the end of a long investigation and it's come down to that question for post_process_urlencoded() in MHD.

MHD currently calls back with ("a&b", "1").  The app I'm working breaks when it doesn't receive a callback for "b".  The http client in this case (that did the urlencoding) is google-http-java-client 1.23.0.

The client behavior may be questionable, but MHD's behavior doesn't seem right either.  Isn't there some principle, "clients should strive for conformance, servers should strive for forgiveness".

I've attached a patch to MHD to add a failing test, but without a fix.  Is MHD open to changing this behavior, given its maturity?

Should I post a bug to the bug tracker?

As for relevant standards, the W3C[1] is not detailed enough to cover it.  The WhatWG[2] is more specific and allows for empty values and even empty keys.  I'd like to callout uriparser[3], another C library I've patched in as a work-around.  Uriparser documents their handling of these cases well:

* NULL in the value member means there was no '=' in the item text as with "?abc&def".
* An empty string in the value member means there was '=' in the item as with "?abc=&def".

[1] https://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.1
[2] https://url.spec.whatwg.org/#urlencoded-parsing
[3] https://uriparser.github.io/doc/api/latest/#querystrings

commit aa0534af56d135e1b261d127af09c22015c1ff87
Author: Ethan Tuttle <address@hidden>
Date:   Tue Dec 24 03:50:59 2019 -0800

    urlencoding post-processor: add failing tests for keys without values

diff --git a/src/microhttpd/test_postprocessor.c b/src/microhttpd/test_postprocessor.c
index 75b5ba33..6d5be040 100644
--- a/src/microhttpd/test_postprocessor.c
+++ b/src/microhttpd/test_postprocessor.c
@@ -42,8 +42,18 @@
  * five NULL-entries.
  */
 const char *want[] = {
+#define URL_NOVALUE1_DATA "abc&x=5"
+#define URL_NOVALUE1_START 0
+        "abc", NULL, NULL, NULL, NULL,
+        "x", NULL, NULL, NULL, "5",
+#define URL_NOVALUE1_END (URL_NOVALUE1_START + 10)
+#define URL_NOVALUE2_DATA "abc=&x=5"
+#define URL_NOVALUE2_START URL_NOVALUE1_END
+        "abc", NULL, NULL, NULL, "",
+        "x", NULL, NULL, NULL, "5",
+#define URL_NOVALUE2_END (URL_NOVALUE2_START + 10)
 #define URL_DATA "abc=def&x=5"
-#define URL_START 0
+#define URL_START URL_NOVALUE2_END
   "abc", NULL, NULL, NULL, "def",
   "x", NULL, NULL, NULL, "5",
 #define URL_END (URL_START + 10)
@@ -125,12 +135,14 @@ value_checker (void *cls,
 
 
 static int
-test_urlencoding (void)
+test_urlencoding_case (unsigned int want_start,
+                       unsigned int want_end,
+                       const char *url_data)
 {
   struct MHD_Connection connection;
   struct MHD_HTTP_Header header;
   struct MHD_PostProcessor *pp;
-  unsigned int want_off = URL_START;
+  unsigned int want_off = want_start;
   size_t i;
   size_t delta;
   size_t size;
@@ -147,19 +159,27 @@ test_urlencoding (void)
   pp = MHD_create_post_processor (&connection,
                                   1024, &value_checker, &want_off);
   i = 0;
-  size = strlen (URL_DATA);
+  size = strlen (url_data);
   while (i < size)
   {
     delta = 1 + MHD_random_ () % (size - i);
-    MHD_post_process (pp, &URL_DATA[i], delta);
+    MHD_post_process (pp, &url_data[i], delta);
     i += delta;
   }
   MHD_destroy_post_processor (pp);
-  if (want_off != URL_END)
+  if (want_off != want_end)
     return 1;
   return 0;
 }
 
+static int
+test_urlencoding (void) {
+  unsigned int errorCount = 0;
+  errorCount += test_urlencoding_case(URL_START, URL_END, URL_DATA);
+  errorCount += test_urlencoding_case(URL_NOVALUE1_START, URL_NOVALUE1_END, URL_NOVALUE1_DATA);
+  errorCount += test_urlencoding_case(URL_NOVALUE2_START, URL_NOVALUE2_END, URL_NOVALUE2_DATA);
+  return errorCount;
+}
 
 static int
 test_multipart_garbage (void)