GNU bug report logs - #51133
[PATCH 1/1] Tolerate http response line without reason phrase

Previous Next

To reply to this bug, email your comments to 51133 AT debbugs.gnu.org.

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

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

From: Alexey Abramov <levenson <at> mmer.org>
To: bug-guile <at> gnu.org
Subject: [PATCH 1/1] Tolerate http response line without reason phrase
Date: Mon, 11 Oct 2021 09:03:55 +0200

* module/web/http.scm (read-response-line): Use the end of the string,
in case a line doesn't have char-set:whitespace at the end.
* test-suite/tests/web-http.test ("read-response-line"): Add test.
---
 module/web/http.scm            | 6 ++++--
 test-suite/tests/web-http.test | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/module/web/http.scm b/module/web/http.scm
index 4276e1744..7443bd6a4 100644
--- a/module/web/http.scm
+++ b/module/web/http.scm
@@ -1187,8 +1187,10 @@ values: the HTTP version, the response code, and the (possibly empty)
 \"reason phrase\"."
   (let* ((line (read-header-line port))
          (d0 (string-index line char-set:whitespace)) ; "delimiter zero"
-         (d1 (and d0 (string-index line char-set:whitespace
-                                   (skip-whitespace line d0)))))
+         (d1 (and d0 (or (string-index line char-set:whitespace
+                                       (skip-whitespace line d0))
+                         ;; tolerate responses with empty "reason phrase"
+                         (string-length line)))))
     (unless (and d0 d1)
       (bad-response "Bad Response-Line: ~s" line))
     (values (parse-http-version line 0 d0)
diff --git a/test-suite/tests/web-http.test b/test-suite/tests/web-http.test
index 63377349c..6d8cd1642 100644
--- a/test-suite/tests/web-http.test
+++ b/test-suite/tests/web-http.test
@@ -216,6 +216,8 @@
 
   ;; Empty reason phrases are valid; see <http://bugs.gnu.org/22273>.
   (pass-if-read-response-line "HTTP/1.1 302 "
+                              (1 . 1) 302 "")
+  (pass-if-read-response-line "HTTP/1.1 302"
                               (1 . 1) 302 ""))
 
 (with-test-prefix "write-response-line"
-- 
2.31.1

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

From: jakub-w <at> riseup.net
To: bug-guile <at> gnu.org
Cc: 51133 <at> debbugs.gnu.org, Alexey Abramov <levenson <at> mmer.org>
Subject: Re: bug#51133: [PATCH 1/1] Tolerate http response line without
 reason phrase
Date: Tue, 12 Oct 2021 10:01:03 +0200

I don't think the reason phrase is optional, even though it can be just
a whitespace.
Even if I'm not mistaken, however, I don't see the reason for Guile not
to be able to parse the status line without a space at the end.

Aside from that consider a string "HTTP/1.1 ", which should be a bad
response because the status code should definitely be obligatory,
however it passes through the (and d0 d1) check after applying this
patch.

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

From: Alexey Abramov <levenson <at> mmer.org>
To: jakub-w <at> riseup.net
Cc: bug-guile <at> gnu.org, 51133 <at> debbugs.gnu.org
Subject: Re: bug#51133: [PATCH 1/1] Tolerate http response line without
 reason phrase
Date: Tue, 12 Oct 2021 10:26:22 +0200

Hi,

I agree that it is not a complient http response, but unfortunately I
came across with some http services (redfish, cimc from Cisco ), where
they don't put a reason phrase. As you can see the difference is that
response line doesn't have a space after the response code, that is why
it raise an exception even though the documentation says 'and the
(possibly empty) "reason phrase"'. 

I would call it as a follow up to f53145d41.

-- 
Alexey

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

From: <tomas <at> tuxteam.de>
To: bug-guile <at> gnu.org
Subject: Re: bug#51133: [PATCH 1/1] Tolerate http response line without
 reason phrase
Date: Tue, 12 Oct 2021 11:11:43 +0200

[Message part 1 (text/plain, inline)]

On Tue, Oct 12, 2021 at 10:26:22AM +0200, Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language wrote:
> Hi,
> 
> I agree that it is not a complient http response,

According to The Book [1] ;-) there should be at least one
space (SP) (as far as I understand this is really a true
honest space, Unicode codepoint 32. It is /not/ part of the
message (aka "reason phrase") , but a separator. The rule
is:

  status-line = HTTP-version SP status-code SP reason-phrase CRLF

The reason phrase itself can contain whatever funny whitespace
it wants:

  reason-phrase  = *( HTAB / SP / VCHAR / obs-text )

and it /can/ be empty.

That said I'd agree that it makes sense to tolerate a missing
SP there. The legal minimum seems thus to be

  HTTP-version SP status-code SP CRLF

>                                          but unfortunately I
> came across with some http services (redfish, cimc from Cisco )

uh-oh. All bets are off, then ;-)


> where they don't put a reason phrase.

That would be OK, but they also eat the mandatory separator space
before the empty reason phrase. Bad folks, bad ;-)

As an onlooker I haven't much to say, but I think you are right
(but not Cisco :)

Cheers
 - t

[signature.asc (application/pgp-signature, inline)]

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

From: jakub-w <at> riseup.net
To: bug-guile <at> gnu.org
Cc: 51133 <at> debbugs.gnu.org, Alexey Abramov <levenson <at> mmer.org>
Subject: Re: bug#51133: [PATCH 1/1] Tolerate http response line without
 reason phrase
Date: Tue, 12 Oct 2021 12:03:09 +0200

You're right but you didn't address my second point.
The fact that with this patch

(call-with-input-string "HTTP/1.1 \n"
  (lambda (port) (read-response-line port)))

passes the check for 'bad-response error inside read-response-line. It
throws 'bad-header-component from non-negative-integer instead because
d1 is always true if d0 is true.

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

From: Alexey Abramov <levenson <at> mmer.org>
To: jakub-w <at> riseup.net
Cc: bug-guile <at> gnu.org, 51133 <at> debbugs.gnu.org
Subject: Re: bug#51133: [PATCH 1/1] Tolerate http response line without
 reason phrase
Date: Tue, 12 Oct 2021 16:35:29 +0200

[Message part 1 (text/plain, inline)]

jakub-w <at> riseup.net writes:

> You're right but you didn't address my second point.
> The fact that with this patch
>
> (call-with-input-string "HTTP/1.1 \n"
>   (lambda (port) (read-response-line port)))

I see, my bad, thanks! Please find a newly attached patch.

I added a test for such a case, but I am not sure about the indentation
though. Please let me know what you think.

-- 
Alexey

[0001-http-Tolerate-http-response-line-without-a-reason-ph.patch (text/x-patch, attachment)]

Previous Next

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