bug#47157: “Bad Read-Header-Line header: #<eof>” while substituting

[Christopher Baines <mail@cbaines.net>]

[-- Attachment #1: Type: text/plain, Size: 2498 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

> Christopher Baines <mail@cbaines.net> skribis:
>
>>> I think 7b812f7c84c43455cdd68a0e51b6ded018afcc8e and subsequent commits
>>> may have caused this regression.  In particular, in
>>> 20c08a8a45d0f137ead7c05e720456b2aea44402,
>>> ‘call-with-connection-error-handling’ is now used, but that one doesn’t
>>> catch the exceptions mentioned above, in this case ‘bad-header’.
>>
>> I think the behaviour changed unintentionally with [1], however,
>> thinking about the connection reuse in process-substitution compared
>> with http-multiple-get, there's no attempt here to look at if the server
>> has specified whether the connection should be closed.
>>
>> 1: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=f50f5751fff4cfc6d5abba9681054569694b7a5c
>>
>> Just like http-multiple-get, it's probably worth trying to check the
>> headers of the response, look at whether the server has indicated that
>> the connection should be closed, and if so, close the connection,
>> forcing a new one to be established for future requests.
>
> I think that’s not enough because we can’t rely on the server’s state
> intent here.
>
> For example, you have a keep-alive connection that you keep in cache.
> Minutes later, you come back and send a request over that port.  If the
> server dropped the connection in the meantime, that can manifest in any
> of the ways we’ve seen: 'bad-response when attempting to read the
> response, some 'gnutls-error, 'system-error and EPIPE, etc.  There’s no
> way to determine in advance whether the socket is fine.
>
> That’s why the initial approach was to wrap all the call sites were the
> socket was known to be possibly “tainted” in ‘with-cached-connection’.
>
>> I've now actually got around to testing this, I'm no expert at running
>> the substitute script manually without the guix-daemon, but I gave it a
>> go, using a local NGinx instance which just allowed two requests per
>> connection.
>
> I believe in this case ‘port-closed?’ returns true because the
> socket/TLS record port got closed right at the end of the response, so
> it’s the “easy” case; I don’t think it captures the situation I
> described above where an error comes up later while trying to write
> to/read from the port.

Yeah, of course, I think error handling is needed as well, it just
occurred to me when looking at this issue and the relevant code.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]