unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* [Patch] Support HTTP/2 in HTTP client
@ 2020-03-31 14:42 Derek Upham
  2020-03-31 15:00 ` Tristan Colgate
  0 siblings, 1 reply; 5+ messages in thread
From: Derek Upham @ 2020-03-31 14:42 UTC (permalink / raw)
  To: guile-devel

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

Companies like Google now respond to HTTP requests with HTTP 2. 
For example:

  curl --silent --head https://www.google.com

returns the first line

  HTTP/2 200

The Guile HTTP client code expects a “HTTP/x.y” structure, and 
treats this as an error.  This patch recognizes and handles 
“HTTP/2” on input and output.  HTTP version numbers don’t 
proliferate quickly, so the code uses a brute force approach for 
now.


[-- Attachment #2: 0001-Support-HTTP-2-headers-from-web-servers.patch --]
[-- Type: text/x-diff, Size: 4896 bytes --]

From 97d82f131afb40527340fb7126efaf50f7fe59eb Mon Sep 17 00:00:00 2001
From: Derek Upham <sand@blarg.net>
Date: Tue, 4 Jul 2017 08:54:06 -0700
Subject: [PATCH 1/2] Support HTTP/2 headers from web servers.

For example,

  curl --silent --head https://www.google.com

returns the first line

  HTTP/2 200

which does not match the MAJOR.MINOR format expected by the old code.
---
 module/web/http.scm            | 41 +++++++++++++++++++++-------------
 test-suite/tests/web-http.test | 16 +++++++++++--
 2 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/module/web/http.scm b/module/web/http.scm
index de61c9495..c5e559bea 100644
--- a/module/web/http.scm
+++ b/module/web/http.scm
@@ -1059,7 +1059,11 @@ as an ordered alist."
 (define* (parse-http-version str #:optional (start 0) (end (string-length str)))
   "Parse an HTTP version from STR, returning it as a major–minor
 pair. For example, ‘HTTP/1.1’ parses as the pair of integers,
-‘(1 . 1)’."
+‘(1 . 1)’.
+
+The HTTP/2 working group has explicitly dropped the ‘.0’ minor
+version.  For internal consistency, track the the missing minor
+version as zero."
   (let lp ((known *known-versions*))
     (match known
       (((version-str . version-val) . known)
@@ -1067,28 +1071,35 @@ pair. For example, ‘HTTP/1.1’ parses as the pair of integers,
            version-val
            (lp known)))
       (()
-       (let ((dot-idx (string-index str #\. start end)))
-         (unless (and (string-prefix? "HTTP/" str 0 5 start end)
-                      dot-idx
-                      (= dot-idx (string-rindex str #\. start end)))
-           
-           (bad-header-component 'http-version (substring str start end)))
-         (cons (parse-non-negative-integer str (+ start 5) dot-idx)
-               (parse-non-negative-integer str (1+ dot-idx) end)))))))
+       (if (string=? "HTTP/2" (substring str start end))
+           '(2 . 0)
+           (let ((dot-idx (string-index str #\. start end)))
+             (unless (and (string-prefix? "HTTP/" str 0 5 start end)
+                          dot-idx
+                          (= dot-idx (string-rindex str #\. start end)))
+               (bad-header-component 'http-version (substring str start end)))
+             (cons (parse-non-negative-integer str (+ start 5) dot-idx)
+                   (parse-non-negative-integer str (1+ dot-idx) end))))))))
 
 (define (write-http-version val port)
-  "Write the given major-minor version pair to PORT."
-  (put-string port "HTTP/")
-  (put-non-negative-integer port (car val))
-  (put-char port #\.)
-  (put-non-negative-integer port (cdr val)))
+  "Write the given major-minor version pair to PORT.
+
+For consistency with HTTP/2 standards, print version ‘(2 . 0)’ as
+“HTTP/2”."
+  (if (equal? val '(2 . 0))
+      (put-string port "HTTP/2")
+      (begin
+        (put-string port "HTTP/")
+        (put-non-negative-integer port (car val))
+        (put-char port #\.)
+        (put-non-negative-integer port (cdr val)))))
 
 (for-each
  (lambda (v)
    (set! *known-versions*
          (acons v (parse-http-version v 0 (string-length v))
                 *known-versions*)))
- '("HTTP/1.0" "HTTP/1.1"))
+ '("HTTP/1.0" "HTTP/1.1" "HTTP/2"))
 
 
 ;; Request-URI = "*" | absoluteURI | abs_path | authority
diff --git a/test-suite/tests/web-http.test b/test-suite/tests/web-http.test
index 63377349c..843936bb5 100644
--- a/test-suite/tests/web-http.test
+++ b/test-suite/tests/web-http.test
@@ -170,7 +170,13 @@
                              (build-uri-reference
                               #:path "/etc/hosts"
                               #:query "foo=bar")
-                             (1 . 1)))
+                             (1 . 1))
+  (pass-if-read-request-line "HEAD /etc/hosts?foo=bar HTTP/2"
+                             HEAD
+                             (build-uri-reference
+                              #:path "/etc/hosts"
+                              #:query "foo=bar")
+                             (2 . 0)))
 
 (with-test-prefix "write-request-line"
   (pass-if-write-request-line "GET / HTTP/1.1"
@@ -201,7 +207,13 @@
                               (build-uri-reference
                                #:path "/etc/hosts"
                                #:query "foo=bar")
-                              (1 . 1)))
+                              (1 . 1))
+  (pass-if-write-request-line "HEAD /etc/hosts?foo=bar HTTP/2"
+                              HEAD
+                              (build-uri-reference
+                               #:path "/etc/hosts"
+                               #:query "foo=bar")
+                              (2 . 0)))
 
 (with-test-prefix "read-response-line"
   (pass-if-exception "missing CR/LF"
-- 
2.25.1


[-- Attachment #3: Type: text/plain, Size: 50 bytes --]


Derek

-- 
Derek Upham
derek_upham@mailfence.com

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Patch] Support HTTP/2 in HTTP client
  2020-03-31 14:42 [Patch] Support HTTP/2 in HTTP client Derek Upham
@ 2020-03-31 15:00 ` Tristan Colgate
  2020-04-01  1:21   ` Derek Upham
  0 siblings, 1 reply; 5+ messages in thread
From: Tristan Colgate @ 2020-03-31 15:00 UTC (permalink / raw)
  To: Derek Upham; +Cc: guile-devel

http/2 is a substantially different protocol, it will take
considerable effort to support it, guile's client should only be
offering http/1.1 to the server.

On Tue, 31 Mar 2020 at 15:49, Derek Upham <derek_upham@mailfence.com> wrote:
>
> Companies like Google now respond to HTTP requests with HTTP 2.
> For example:
>
>   curl --silent --head https://www.google.com
>
> returns the first line
>
>   HTTP/2 200
>
> The Guile HTTP client code expects a “HTTP/x.y” structure, and
> treats this as an error.  This patch recognizes and handles
> “HTTP/2” on input and output.  HTTP version numbers don’t
> proliferate quickly, so the code uses a brute force approach for
> now.
>
>
> Derek
>
> --
> Derek Upham
> derek_upham@mailfence.com



-- 
Tristan Colgate-McFarlane
----
  "You can get all your daily vitamins from 52 pints of guiness, and a
glass of milk"



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch] Support HTTP/2 in HTTP client
  2020-03-31 15:00 ` Tristan Colgate
@ 2020-04-01  1:21   ` Derek Upham
  2020-04-01  7:24     ` Tristan Colgate
  2020-04-07 14:39     ` Derek Upham
  0 siblings, 2 replies; 5+ messages in thread
From: Derek Upham @ 2020-04-01  1:21 UTC (permalink / raw)
  To: guile-devel

I made this particular tweak because Guile 2.2’s HTTP/1.1 client 
requests were getting back an HTTP/2 response from a podcast 
server, and choking.  (The use of Google’s URL was just an example 
to show the format of the HTTP response header.)   Let’s drop the 
patch for now.  I’ll try disabling it locally and see whether I 
can still reproduce it.  Given the amount of redirects that media 
website backends do, it’s possible that I’ll never come across the 
problem again.

Derek

Tristan Colgate <tcolgate@gmail.com> writes:

> http/2 is a substantially different protocol, it will take
> considerable effort to support it, guile's client should only be
> offering http/1.1 to the server.
>
> On Tue, 31 Mar 2020 at 15:49, Derek Upham 
> <derek_upham@mailfence.com> wrote:
>>
>> Companies like Google now respond to HTTP requests with HTTP 2.
>> For example:
>>
>>   curl --silent --head https://www.google.com
>>
>> returns the first line
>>
>>   HTTP/2 200
>>
>> The Guile HTTP client code expects a “HTTP/x.y” structure, and
>> treats this as an error.  This patch recognizes and handles
>> “HTTP/2” on input and output.  HTTP version numbers don’t
>> proliferate quickly, so the code uses a brute force approach 
>> for
>> now.
>>
>>
>> Derek
>>
>> --
>> Derek Upham
>> derek_upham@mailfence.com


-- 
Derek Upham
derek_upham@mailfence.com



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch] Support HTTP/2 in HTTP client
  2020-04-01  1:21   ` Derek Upham
@ 2020-04-01  7:24     ` Tristan Colgate
  2020-04-07 14:39     ` Derek Upham
  1 sibling, 0 replies; 5+ messages in thread
From: Tristan Colgate @ 2020-04-01  7:24 UTC (permalink / raw)
  To: Derek Upham; +Cc: guile-devel

I'm not sure why a server would give you a http2 response if you
didn't ask for one (or atleast suggest you could accept one), in the
tls ALPN. If it happened on a non-tls request, I'm even more confused.
  Note that curl, without extra cli options, will negotiate http2, so
you need to make sure that guile actually saw that, and not just
assume that it did because curl saw it (you can force the http version
with, e.g. --http1.1)
  If you get a http/2 response, the client, as is, wont be able to
process it anyway, so error'ing is very must the right thing to do.
http/2 isn't just a cosmetic change, it's an entirely different
protocol, not text based, multi request/response streams, an
compression mechanism that uses pre-baked tables based on online
observed data.

On Wed, 1 Apr 2020 at 02:22, Derek Upham <derek_upham@mailfence.com> wrote:
>
> I made this particular tweak because Guile 2.2’s HTTP/1.1 client
> requests were getting back an HTTP/2 response from a podcast
> server, and choking.  (The use of Google’s URL was just an example
> to show the format of the HTTP response header.)   Let’s drop the
> patch for now.  I’ll try disabling it locally and see whether I
> can still reproduce it.  Given the amount of redirects that media
> website backends do, it’s possible that I’ll never come across the
> problem again.
>
> Derek
>
> Tristan Colgate <tcolgate@gmail.com> writes:
>
> > http/2 is a substantially different protocol, it will take
> > considerable effort to support it, guile's client should only be
> > offering http/1.1 to the server.
> >
> > On Tue, 31 Mar 2020 at 15:49, Derek Upham
> > <derek_upham@mailfence.com> wrote:
> >>
> >> Companies like Google now respond to HTTP requests with HTTP 2.
> >> For example:
> >>
> >>   curl --silent --head https://www.google.com
> >>
> >> returns the first line
> >>
> >>   HTTP/2 200
> >>
> >> The Guile HTTP client code expects a “HTTP/x.y” structure, and
> >> treats this as an error.  This patch recognizes and handles
> >> “HTTP/2” on input and output.  HTTP version numbers don’t
> >> proliferate quickly, so the code uses a brute force approach
> >> for
> >> now.
> >>
> >>
> >> Derek
> >>
> >> --
> >> Derek Upham
> >> derek_upham@mailfence.com
>
>
> --
> Derek Upham
> derek_upham@mailfence.com
>


-- 
Tristan Colgate-McFarlane
----
  "You can get all your daily vitamins from 52 pints of guiness, and a
glass of milk"I'm not sure why a server would give you a http2
response if you didn't ask for one (or atleast suggest you could
accept one), in the tls ALPN. If it happened on a non-tls request, I'm
even more confused.
  If you get a http/2 response, the client, as is, wont be able to
process it anyway, so error'ing is very must the right thing to do.
http/2 isn't just a cosmetic change, it's an entirely different
protocol, not text based, multi request/response streams, a

On Wed, 1 Apr 2020 at 02:22, Derek Upham <derek_upham@mailfence.com> wrote:
>
> I made this particular tweak because Guile 2.2’s HTTP/1.1 client
> requests were getting back an HTTP/2 response from a podcast
> server, and choking.  (The use of Google’s URL was just an example
> to show the format of the HTTP response header.)   Let’s drop the
> patch for now.  I’ll try disabling it locally and see whether I
> can still reproduce it.  Given the amount of redirects that media
> website backends do, it’s possible that I’ll never come across the
> problem again.
>
> Derek
>
> Tristan Colgate <tcolgate@gmail.com> writes:
>
> > http/2 is a substantially different protocol, it will take
> > considerable effort to support it, guile's client should only be
> > offering http/1.1 to the server.
> >
> > On Tue, 31 Mar 2020 at 15:49, Derek Upham
> > <derek_upham@mailfence.com> wrote:
> >>
> >> Companies like Google now respond to HTTP requests with HTTP 2.
> >> For example:
> >>
> >>   curl --silent --head https://www.google.com
> >>
> >> returns the first line
> >>
> >>   HTTP/2 200
> >>
> >> The Guile HTTP client code expects a “HTTP/x.y” structure, and
> >> treats this as an error.  This patch recognizes and handles
> >> “HTTP/2” on input and output.  HTTP version numbers don’t
> >> proliferate quickly, so the code uses a brute force approach
> >> for
> >> now.
> >>
> >>
> >> Derek
> >>
> >> --
> >> Derek Upham
> >> derek_upham@mailfence.com
>
>
> --
> Derek Upham
> derek_upham@mailfence.com
>


-- 
Tristan Colgate-McFarlane
----
  "You can get all your daily vitamins from 52 pints of guiness, and a
glass of milk"



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch] Support HTTP/2 in HTTP client
  2020-04-01  1:21   ` Derek Upham
  2020-04-01  7:24     ` Tristan Colgate
@ 2020-04-07 14:39     ` Derek Upham
  1 sibling, 0 replies; 5+ messages in thread
From: Derek Upham @ 2020-04-07 14:39 UTC (permalink / raw)
  To: guile-devel

Okay, I figured it out.  In that particular chunk of code I was 
calling out to Curl to do the heavy lifting: “(run/port (curl 
--silent --head ,source-url))”.  Curl negotiates HTTP/2 and dumps 
the header information:

  HTTP/2 200 
  date: Tue, 07 Apr 2020 14:22:37 GMT
  content-type: audio/mpeg
  content-length: 31411830

The code then passed the response port to the existing 
“read-response” function in the web/response module, to turn this 
text into data objects.

That means this isn’t a “support HTTP/2 connections” change, it’s 
just a “support parsing HTTP/2 responses” change.

However, sometime during all of this I figured out how to get the 
system-installed extensions working with my personal build of 
Guile, so the TLS extension now works and I can cut Curl out of 
this code and just use “http-head”.  We can drop this patch.

Thanks,

Derek

Derek Upham <derek_upham@mailfence.com> writes:

> I made this particular tweak because Guile 2.2’s HTTP/1.1 client 
> requests were getting back an HTTP/2 response from a podcast 
> server, and choking.  (The use of Google’s URL was just an 
> example to show the format of the HTTP response header.)   Let’s 
> drop the 
> patch for now.  I’ll try disabling it locally and see whether I 
> can still reproduce it.  Given the amount of redirects that 
> media 
> website backends do, it’s possible that I’ll never come across 
> the problem again.
>
> Derek
>
> Tristan Colgate <tcolgate@gmail.com> writes:
>
>> http/2 is a substantially different protocol, it will take
>> considerable effort to support it, guile's client should only 
>> be
>> offering http/1.1 to the server.
>>
>> On Tue, 31 Mar 2020 at 15:49, Derek Upham 
>> <derek_upham@mailfence.com> wrote:
>>>
>>> Companies like Google now respond to HTTP requests with HTTP 
>>> 2.
>>> For example:
>>>
>>>   curl --silent --head https://www.google.com
>>>
>>> returns the first line
>>>
>>>   HTTP/2 200
>>>
>>> The Guile HTTP client code expects a “HTTP/x.y” structure, and
>>> treats this as an error.  This patch recognizes and handles
>>> “HTTP/2” on input and output.  HTTP version numbers don’t
>>> proliferate quickly, so the code uses a brute force approach 
>>> for
>>> now.
>>>
>>>
>>> Derek
>>>
>>> --
>>> Derek Upham
>>> derek_upham@mailfence.com


-- 
Derek Upham
derek_upham@mailfence.com



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-04-07 14:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-31 14:42 [Patch] Support HTTP/2 in HTTP client Derek Upham
2020-03-31 15:00 ` Tristan Colgate
2020-04-01  1:21   ` Derek Upham
2020-04-01  7:24     ` Tristan Colgate
2020-04-07 14:39     ` Derek Upham

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).