* bug#51133: [PATCH 1/1] Tolerate http response line without reason phrase
@ 2021-10-11 7:03 Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2021-10-12 8:01 ` jakub-w
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language @ 2021-10-11 7:03 UTC (permalink / raw)
To: 51133
* 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#51133: [PATCH 1/1] Tolerate http response line without reason phrase
2021-10-11 7:03 bug#51133: [PATCH 1/1] Tolerate http response line without reason phrase Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
@ 2021-10-12 8:01 ` jakub-w
2021-10-12 8:26 ` Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
0 siblings, 1 reply; 6+ messages in thread
From: jakub-w @ 2021-10-12 8:01 UTC (permalink / raw)
To: 51133; +Cc: levenson
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#51133: [PATCH 1/1] Tolerate http response line without reason phrase
2021-10-12 8:01 ` jakub-w
@ 2021-10-12 8:26 ` Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2021-10-12 9:11 ` tomas
2021-10-12 10:03 ` jakub-w
0 siblings, 2 replies; 6+ messages in thread
From: Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language @ 2021-10-12 8:26 UTC (permalink / raw)
To: jakub-w; +Cc: 51133
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#51133: [PATCH 1/1] Tolerate http response line without reason phrase
2021-10-12 8:26 ` Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
@ 2021-10-12 9:11 ` tomas
2021-10-12 10:03 ` jakub-w
1 sibling, 0 replies; 6+ messages in thread
From: tomas @ 2021-10-12 9:11 UTC (permalink / raw)
To: 51133
[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]
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
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#51133: [PATCH 1/1] Tolerate http response line without reason phrase
2021-10-12 8:26 ` Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2021-10-12 9:11 ` tomas
@ 2021-10-12 10:03 ` jakub-w
2021-10-12 14:35 ` Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
1 sibling, 1 reply; 6+ messages in thread
From: jakub-w @ 2021-10-12 10:03 UTC (permalink / raw)
To: 51133; +Cc: levenson
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#51133: [PATCH 1/1] Tolerate http response line without reason phrase
2021-10-12 10:03 ` jakub-w
@ 2021-10-12 14:35 ` Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
0 siblings, 0 replies; 6+ messages in thread
From: Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language @ 2021-10-12 14:35 UTC (permalink / raw)
To: jakub-w; +Cc: 51133
[-- Attachment #1: Type: text/plain, Size: 393 bytes --]
jakub-w@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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-http-Tolerate-http-response-line-without-a-reason-ph.patch --]
[-- Type: text/x-patch, Size: 3717 bytes --]
From b589595db9ab448aa52d002c34d7919a601904e4 Mon Sep 17 00:00:00 2001
From: Alexey Abramov <levenson@mmer.org>
Date: Thu, 7 Oct 2021 13:45:02 +0200
Subject: [PATCH] http: Tolerate http response line without a reason phrase
* 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 tests.
* .dir-locals.el (scheme-mode): Add indentation rule for pass-if-named-exception.
---
.dir-locals.el | 1 +
module/web/http.scm | 20 +++++++++++++-------
test-suite/tests/web-http.test | 8 +++++++-
3 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/.dir-locals.el b/.dir-locals.el
index 90257e7bf..e94ceb723 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -10,6 +10,7 @@
(eval . (put 'let/ec 'scheme-indent-function 1))
(eval . (put 'pass-if 'scheme-indent-function 1))
(eval . (put 'pass-if-exception 'scheme-indent-function 2))
+ (eval . (put 'pass-if-named-exception 'scheme-indent-function 3))
(eval . (put 'pass-if-equal 'scheme-indent-function 2))
(eval . (put 'with-test-prefix 'scheme-indent-function 1))
(eval . (put 'with-test-prefix/c&e 'scheme-indent-function 1))
diff --git a/module/web/http.scm b/module/web/http.scm
index 4276e1744..4053e5271 100644
--- a/module/web/http.scm
+++ b/module/web/http.scm
@@ -1187,14 +1187,20 @@ 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)))))
- (unless (and d0 d1)
+ (d1 (and d0 (or (string-index line char-set:whitespace
+ (skip-whitespace line d0))
+ ;; tolerate responses with empty "reason phrase"
+ (string-length line)))))
+ (cond
+ ((not d0)
+ (bad-response "Bad Response-Line: ~s" line))
+ ((and d0 d1 (string-null? (string-trim (substring line d0 d1))))
(bad-response "Bad Response-Line: ~s" line))
- (values (parse-http-version line 0 d0)
- (parse-non-negative-integer line (skip-whitespace line d0 d1)
- d1)
- (string-trim-both line char-set:whitespace d1))))
+ (else
+ (values (parse-http-version line 0 d0)
+ (parse-non-negative-integer line (skip-whitespace line d0 d1)
+ d1)
+ (string-trim-both line char-set:whitespace d1))))))
(define (write-response-line version code reason-phrase port)
"Write the first line of an HTTP response to PORT."
diff --git a/test-suite/tests/web-http.test b/test-suite/tests/web-http.test
index 63377349c..7d4885722 100644
--- a/test-suite/tests/web-http.test
+++ b/test-suite/tests/web-http.test
@@ -216,7 +216,13 @@
;; Empty reason phrases are valid; see <http://bugs.gnu.org/22273>.
(pass-if-read-response-line "HTTP/1.1 302 "
- (1 . 1) 302 ""))
+ (1 . 1) 302 "")
+ (pass-if-read-response-line "HTTP/1.1 302"
+ (1 . 1) 302 "")
+ (pass-if-named-exception "missing HTTP code" bad-response "Bad Response-Line"
+ (call-with-input-string "HTTP/1.1 \n"
+ (lambda (port)
+ (read-response-line port)))))
(with-test-prefix "write-response-line"
(pass-if-write-response-line "HTTP/1.0 404 Not Found"
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-12 14:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-11 7:03 bug#51133: [PATCH 1/1] Tolerate http response line without reason phrase Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2021-10-12 8:01 ` jakub-w
2021-10-12 8:26 ` Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
2021-10-12 9:11 ` tomas
2021-10-12 10:03 ` jakub-w
2021-10-12 14:35 ` Alexey Abramov via Bug reports for GUILE, GNU's Ubiquitous Extension Language
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).