GNU bug report logs - #36350
[2.2.5] ‘read-headers’ blocks, thereby breaking web servers

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 36350 in the body.
You can then email your comments to 36350 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: bug-guile <at> gnu.org
Cc: Mark H Weaver <mhw <at> netris.org>
Subject: [2.2.5] ‘read-headers’ blocks, thereby
 breaking web servers
Date: Mon, 24 Jun 2019 12:32:18 +0200

Hello,

In Guile 2.2.5, if you run:

  ./meta/guile examples/web/hello.scm &
  wget -O - http://localhost:8080

You’ll notice that ‘wget’ hangs (never receives a response) because the
server is actually stuck in a read(2) call that will never complete, in
‘read-headers’.

Reverting 73cde5ed7218a090ecee888870908af5445796f0 solves the problem.

AIUI, before that commit, ‘read-header-line’ would read exactly one
line.  After this change, it calls ‘lookahead-char’, which can block,
and that’s exactly what’s happening here.

I don’t know how HTTP continuation lines work, so I’m not sure what a
correct fix would look like.  Mark, WDYT?

I also noticed that there are no unit tests for (web server), which we
should probably address while we’re at it.  :-)

Thanks,
Ludo’.

Message #8 received at 36350 <at> debbugs.gnu.org (full text, mbox):

From: Dan Frumin <dfrumin <at> cs.ru.nl>
To: 36350 <at> debbugs.gnu.org
Subject: Re: [2.2.5] ‘read-headers’ blocks, thereby breaking web servers
Date: Mon, 24 Jun 2019 14:05:57 +0200

I believe that `(lookahead-char port)` really blocks when the client has finished sending the request and there is no more data from `port` to consume.
If I understand it correctly, then per HTTP/1.1 [1] the request ends with CRLF at the last line, and then comes the message. So I we have read an 
empty string, then we shouldn't proceed with further lookaheads.

Specifically, the following code works out for me:


(define (read-header-line port)
  "Read an HTTP header line, including any continuation lines, and
return the combined string without its final CRLF or LF.  Raise a
'bad-header' exception if the line does not end in CRLF or LF, or if EOF
is reached."
  (format #t "Reading header line now: ")
  (match (%read-line port)
    (((? string? line) . #\newline)
     ;; '%read-line' does not consider #\return a delimiter; so if it's
     ;; there, remove it.  We are more tolerant than the RFC in that we
     ;; tolerate LF-only endings.
     (let ((line (if (string-suffix? "\r" line)
                     (string-drop-right line 1)
                     line)))
       ;; If the next character is a space or tab, then there's at least
       ;; one continuation line.  Read the continuation lines by calling
       ;; 'read-header-line' recursively, and append them to this header
       ;; line, folding the leading spaces and tabs to a single space.
       (if (and (not (string-null? line))
                (space-or-tab? (lookahead-char port)))
           (string-append line " " (string-trim (read-header-line port)
                                                spaces-and-tabs))
           line)))
    ((line . _)                                ;EOF or missing delimiter
     (bad-header 'read-header-line line))))

Moreover, the continuation lines in general have been deprecated: [2].
I have to say I would be in favor of removing support for continuation lines in general.

Best regards,
-Dan


[1]: https://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html
[2]: https://tools.ietf.org/html/rfc7230#section-3.2.4

Message #13 received at 36350 <at> debbugs.gnu.org (full text, mbox):

From: Dan Frumin <dfrumin <at> cs.ru.nl>
To: 36350 <at> debbugs.gnu.org
Subject: Re: [2.2.5] ‘read-headers’ blocks, thereby breaking web servers
Date: Mon, 24 Jun 2019 14:44:58 +0200

By the way, I've just tested the web server in Google Chrome, and it works fine!

I was told that a (potential) reason for that is that Chrome sends TCP FIN to the server, which closes the socket for reading, and then 
`lookahead-char` sees eof.

Best,
Dan

Message #16 received at 36350 <at> debbugs.gnu.org (full text, mbox):

From: Mark H Weaver <mhw <at> netris.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 36350 <at> debbugs.gnu.org
Subject: Re: bug#36350: [2.2.5] ‘read-headers’
 blocks, thereby breaking web servers
Date: Mon, 24 Jun 2019 11:36:41 -0400

Hi Ludovic,

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

>   ./meta/guile examples/web/hello.scm &
>   wget -O - http://localhost:8080
>
> You’ll notice that ‘wget’ hangs (never receives a response) because the
> server is actually stuck in a read(2) call that will never complete, in
> ‘read-headers’.
>
> Reverting 73cde5ed7218a090ecee888870908af5445796f0 solves the problem.
>
> AIUI, before that commit, ‘read-header-line’ would read exactly one
> line.  After this change, it calls ‘lookahead-char’, which can block,
> and that’s exactly what’s happening here.

Gah, indeed!

Also, I see now that there was already some existing code to handle HTTP
continuation lines, before my commit referenced above, although it
neglects to fold the whitespace after the CRLF into a single SP octet.
I'm not sure how I failed to notice that.

The commit should simply be reverted, I think.  Pushed as commit
e1225d013ed8673382d6d8f9300dd6b175c8b820 on the stable-2.2 branch.
I tried leaving the new test in place, but it failed due to the lack of
whitespace folding in the previous code.

I really messed up here, sorry.

> I also noticed that there are no unit tests for (web server), which we
> should probably address while we’re at it.  :-)

Yes, that would be great.  I won't be able to do it anytime soon,
though.

       Mark

Message #19 received at 36350 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mark H Weaver <mhw <at> netris.org>
Cc: 36350 <at> debbugs.gnu.org
Subject: Re: bug#36350: [2.2.5] ‘read-headers’
 blocks, thereby breaking web servers
Date: Mon, 24 Jun 2019 20:56:46 +0200

Hi Mark,

Mark H Weaver <mhw <at> netris.org> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>>   ./meta/guile examples/web/hello.scm &
>>   wget -O - http://localhost:8080
>>
>> You’ll notice that ‘wget’ hangs (never receives a response) because the
>> server is actually stuck in a read(2) call that will never complete, in
>> ‘read-headers’.
>>
>> Reverting 73cde5ed7218a090ecee888870908af5445796f0 solves the problem.
>>
>> AIUI, before that commit, ‘read-header-line’ would read exactly one
>> line.  After this change, it calls ‘lookahead-char’, which can block,
>> and that’s exactly what’s happening here.
>
> Gah, indeed!
>
> Also, I see now that there was already some existing code to handle HTTP
> continuation lines, before my commit referenced above, although it
> neglects to fold the whitespace after the CRLF into a single SP octet.
> I'm not sure how I failed to notice that.
>
> The commit should simply be reverted, I think.  Pushed as commit
> e1225d013ed8673382d6d8f9300dd6b175c8b820 on the stable-2.2 branch.
> I tried leaving the new test in place, but it failed due to the lack of
> whitespace folding in the previous code.

OK, reverting sounds good to me.

> I really messed up here, sorry.

No problem.  Perhaps we should consider releasing 2.0.6 soon and use
that in Guix on ‘core-updates’.

Thanks,
Ludo’.

Message #22 received at 36350 <at> debbugs.gnu.org (full text, mbox):

From: Mark H Weaver <mhw <at> netris.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 36350 <at> debbugs.gnu.org
Subject: Re: bug#36350: [2.2.5] ‘read-headers’
 blocks, thereby breaking web servers
Date: Tue, 25 Jun 2019 03:22:55 -0400

Ludovic Courtès <ludo <at> gnu.org> writes:
> Perhaps we should consider releasing 2.0.6 soon and use that in Guix
> on ‘core-updates’.

Sure, sounds like a good idea.

       Mark

Message #27 received at 36350-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mark H Weaver <mhw <at> netris.org>
Cc: 36350-done <at> debbugs.gnu.org
Subject: Re: bug#36350: [2.2.5] ‘read-headers’
 blocks, thereby breaking web servers
Date: Sun, 30 Jun 2019 21:50:17 +0200

Hi,

Mark H Weaver <mhw <at> netris.org> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>> Perhaps we should consider releasing 2.0.6 soon and use that in Guix
>> on ‘core-updates’.
>
> Sure, sounds like a good idea.

Done in a152a67d3865cc6e7f9d7abd8f17a6e905b8e841.  The test is simple
but would catch regressions like this one.

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.