Re: [libmicrohttpd] 0.9.67 regression

[Tim Rühsen]

Hi Christian,

that's wow and great ! Thank you so much :-)

I just pushed a change to SKIP the two affected tests with 0x00096701
and 0x00096702.

But it would be nice to have release, else we'll likely see 0.9.67 going
into several distributions and it would take a while for them to be
updated then. E.g. Debian is still a 0.9.66 - and it would be great if
they directly jump onto 0.9.68.

Regards, Tim

On 26.10.19 18:55, Christian Grothoff wrote:
> Hi Tim,
> 
> minimal_example_empty was just running an HTTP server that would only
> return empty replies ;-).  Anyway, the key answer was to try with HTTPS.
> With that, I could reproduce and fix the issue.
> 
> It has been fixed in g5e5dbf98..1fe2d0d3 (MHD_VERSION: 0x00096703)
> 
> I've also added testcases for this.
> 
> The issue seems minor, but would a new release be useful for you at this
> time?
> 
> Happy hacking!
> 
> Christian
> 
> On 10/26/19 6:07 PM, Tim Rühsen wrote:
>> Hi Christian,
>>
>> I am just staring at minimal_example_empty.c and wonder what should
>> happen when I call the executable !? Anyways, I can't easily fill it
>> with content that works as a reproducer.
>>
>> It seems to occur only with HTTPS, HTTP definitely works as expected.
>>
>> To reproduce:
>> git clone https://gitlab.com/gnuwget/wget2.git
>> cd wget2
>> ./bootstrap
>> ./configure
>> make -j$(nproc)
>> make -j$(nproc) check -C tests TESTS=test--https-enforce-soft2
>>
>> It doesn't come back immediately... so open up a second terminal and
>>   tail -f tests/test--https-enforce-soft2.log
>>
>> You will see the GET request from wget2 and then waiting 10s without
>> seeing the response. With MHD <= 0.9.66 you'll immediately get the response.
>>
>> The MHD server code is in tests/libtest.c.
>> wget_test_start_server() starts the servers, one for HTTPS and another
>> one for HTTP (which is likely not relevant here).
>>
>> The HTTPS server is started around L733 with
>>      httpsdaemon = MHD_start_daemon(MHD_USE_SELECT_INTERNALLY | MHD_USE_TLS
>> #if MHD_VERSION >= 0x00096302
>>              | MHD_USE_POST_HANDSHAKE_AUTH_SUPPORT
>> #endif
>>      ,
>>      port_num, _check_to_accept, (void *) (ptrdiff_t) SERVER_MODE,
>> &_answer_to_connection, NULL,
>>      MHD_OPTION_HTTPS_MEM_KEY, key_pem,
>>      MHD_OPTION_HTTPS_MEM_CERT, cert_pem,
>> #ifdef MHD_OPTION_STRICT_FOR_CLIENT
>>      MHD_OPTION_STRICT_FOR_CLIENT, 1,
>> #endif
>>      MHD_OPTION_CONNECTION_MEMORY_LIMIT, (size_t) 1*1024*1024,
>>      MHD_OPTION_END);
>>
>>
>> The test executes wget2 sends a request to the HTTPS server (as you can
>> see in the .log file of the test):
>>
>> 26.172501.240 GnuTLS init
>> 26.172501.253 Certificates loaded: 128
>> 26.172501.253 GnuTLS init done
>> 26.172501.253 TLS False Start requested
>> 26.172501.253 ALPN offering h2
>> 26.172501.253 ALPN offering http/1.1
>> WARNING: The certificate's (stapled) OCSP status has not been sent
>> 26.172501.267 TLS False Start: off
>> 26.172501.267 GnuTLS: Get ALPN: The requested data were not available.
>> 26.172501.267 Handshake completed
>> 26.172501.267 established connection localhost
>> 26.172501.267 # sent 233 bytes:
>> GET /robots.txt HTTP/1.1
>> Host: localhost
>> Accept-Encoding: gzip, deflate, bzip2, xz, lzma, br, zstd, lzip
>> Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
>> User-Agent: wget2/1.99.2
>> Connection: keep-alive
>>
>> 26.172501.267 [0] action=2 pending=1 host=0x5606a570abf0
>> 26.172501.267 ### req 0x7f331c2e9380 pending requests = 1
>> ###### here the client is waiting for the response for max 10s
>>
>>
>> Hopefully, the above information gives you a chance to track the issue down.
>>
>> Regards, Tim
>>
>> On 24.10.19 18:01, Christian Grothoff wrote:
>>> Hi Tim,
>>>
>>> Your one-liner just fixes the FTBFS and is itself correct.
>>> The commit just before that broke everything is the one activating ng0's
>>> big GSoC work which significantly changes the transmission logic (and
>>> was thus prepared by many, many other patches before).
>>>
>>> I do of course have a hunch as to where the issue might be (0 bytes body
>>> means no send() for the body, after all. So that might prevent proper
>>> uncorking, and that might trigger in a test suite that expects low
>>> latency. But then again, your report says "no reply at all", not just
>>> +200 ms. So strange.
>>>
>>> Anyway, I would really appreciate being able to reproduce this one. You
>>> could also point me to the wget code, I'd dig there myself, but it would
>>> likely take a bit longer in that case ;-).
>>>
>>> Happy hacking!
>>>
>>> Christian
>>>
>>> On 10/24/19 4:07 PM, Tim Rühsen wrote:
>>>> Hi Christian,
>>>>
>>>> thanks for the test file - I'll try to copy our code into it.
>>>>
>>>> Meanwhile I bisected the git commits...
>>>>
>>>> The last functioning is
>>>> 02a9ae28d64498266869b49b042905946df7ce66 "synt"
>>>>
>>>> The next one is not buildable here
>>>> 379da4ce093bdc957b53b563aa1ae0c7c37c19ac "configure.ac: define a check
>>>> for HAVE_SENDMSG"
>>>>
>>>> And this is the commit that broke our test suite
>>>> 8ce24c2ae433fdf1ec125211d3622f3c27b56797 "_len -> _size"
>>>>
>>>> It's just a one-liner... I hope this helps :-)
>>>>
>>>> Regards, Tim
>>>>
>>>>
>>>> On 10/24/19 12:48 PM, Christian Grothoff wrote:
>>>>> Hi Tim,
>>>>>
>>>>> I cannot reproduce this. We do test for empty body in test_get.c, and
>>>>> that test passes. I also wrote (and now pushed to master) a
>>>>> minimal_example_empty.c, using both the 200 and 204 status codes. It
>>>>> works for me on loopback as well as over the network, using both curl
>>>>> and wget as clients.
>>>>>
>>>>> Maybe your issue is about a specific threading mode or something else
>>>>> you are doing?  Please modify the minimal_example_empty.c to emulate the
>>>>> situation that is causing the issue.
>>>>>
>>>>> Thanks!
>>>>>
>>>>> Christian
>>>>>
>>>>> On 10/23/19 3:39 PM, Tim Rühsen wrote:
>>>>>> Hi,
>>>>>>
>>>>>> with 0.9.67 some tests for Wget2 fail which were fine with 0.9.66. This
>>>>>> is still reproducible with latest master.
>>>>>>
>>>>>> As it seems, a response with an empty body doesn't send a response any 
>>>>>> more.
>>>>>>
>>>>>> That means
>>>>>>
>>>>>>   response = MHD_create_response_from_buffer(0, (void *) "",
>>>>>> MHD_RESPMEM_PERSISTENT);
>>>>>>
>>>>>> doesn't work, while
>>>>>>
>>>>>>   response = MHD_create_response_from_buffer(1, (void *) "x",
>>>>>> MHD_RESPMEM_PERSISTENT);
>>>>>>
>>>>>> still generates a response.
>>>>>>
>>>>>> I tested this with MHD_HTTP_BAD_REQUEST, MHD_HTTP_OK and
>>>>>> MHD_HTTP_NOT_FOUND used for MHD_queue_response().
>>>>>>
>>>>>>
>>>>>> Regards, Tim
>>>>>>
>>>>>
>>>>
>>>
>>
>

signature.asc
Description: OpenPGP digital signature