Re: [libmicrohttpd] 0.9.67 regression

[Christian Grothoff]

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