Hi Christian. I finally got around to testing your form-decoding fix
in f34781c8. It works great for me!
I hope that change makes it into an MHD release soon. I'll file a bug
against the app where I encountered the problem (glewlwyd), and
suggest they pull in that fix asap.
Thanks for all your work,
Ethan
On Fri, Dec 27, 2019 at 3:52 PM Christian Grothoff <groth...@gnunet.org> wrote:
>
> Hi Ethan,
>
> I've implemented this now in Git master. Feedback welcome...
>
> Happy hacking!
>
> Christian
>
> On 12/25/19 9:45 AM, Ethan Tuttle wrote:
> > Thanks for the response, Chrstian. I'll take a look at the parsing
> > code, but I don't have high confidence I can fix it to your liking
> > (and in a secure / performant way).
> >
> > Otherwise, I'll stick with my uriparser patch until you get around to it.
> >
> > Happy hacking and happy holidays!
> >
> > Ethan
> >
> > On Tue, Dec 24, 2019 at 6:30 AM Christian Grothoff <groth...@gnunet.org>
> > wrote:
> >>
> >> Hi Ethan,
> >>
> >> I agree that this is a bug, and yes, we should fix it. Thanks for the
> >> test case, that is very helpful.
> >>
> >> I don't know when I can work on it, but worst-case expect a patch by
> >> the end of January, if you're lucky and my other bugs go fast (or
> >> someone else sends a fix), might of course happen earlier.
> >>
> >> Happy hacking!
> >>
> >> Christian
> >>
> >> On Tue, 2019-12-24 at 04:11 -0800, Ethan Tuttle wrote:
> >>> 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 <et...@ethantuttle.com>
> >>> 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)
> >>
> >>
> >
>
