GNU bug report logs - #28262
[PATCH] Handle the same HTTP redirects everywhere.

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 28262 in the body.
You can then email your comments to 28262 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: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: guix-patches <at> gnu.org
Subject: [PATCH] Handle the same HTTP redirects everywhere.
Date: Mon, 28 Aug 2017 15:46:10 +0200

* guix/download.scm (http-fetch): Complete the hard-coded list of HTTP
redirect status codes.
* guix/http-client.scm (http-fetch): Likewise.
* guix/scripts/lint.scm (probe-uri): Likewise.
---

Guix,

There are three (that I know of) hard-coded lists of HTTP redirect status
codes in Guix. All were different, and all were incomplete.

This patch doesn't address the duplication, but does add all missing
codes. Specifically the newer HTTP/1.1 codes, including 303 ‘See Other’.
It's not strictly a plain redirect, but used as such in the wild[1], and
treating it as such is probably enough for our purposes.

This allows at least lightdm-gtk-greeter to be built again. Why its
sources waren't mirrored to begin with I do not know, nor did I check.

Kind regards,

T G-R

[1]: https://launchpad.net/lightdm-gtk-greeter/2.0/2.0.2/+download/lightdm-gtk-greeter-2.0.2.tar.gz

 guix/build/download.scm | 5 ++++-
 guix/http-client.scm    | 6 +++++-
 guix/scripts/lint.scm   | 7 ++++++-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/guix/build/download.scm b/guix/build/download.scm
index 6ef623334..bcf22663b 100644
--- a/guix/build/download.scm
+++ b/guix/build/download.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw <at> netris.org>
 ;;; Copyright © 2015 Steve Sprang <scs <at> stevesprang.com>
+;;; Copyright © 2017 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -763,7 +764,9 @@ certificates; otherwise simply ignore them."
          file))
       ((301                                       ; moved permanently
         302                                       ; found (redirection)
-        307)                                      ; temporary redirection
+        303                                       ; see other
+        307                                       ; temporary redirection
+        308)                                      ; permanent redirection
        (let ((uri (resolve-uri-reference (response-location resp) uri)))
          (format #t "following redirection to `~a'...~%"
                  (uri->string uri))
diff --git a/guix/http-client.scm b/guix/http-client.scm
index 3c5441c38..8db332093 100644
--- a/guix/http-client.scm
+++ b/guix/http-client.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw <at> netris.org>
 ;;; Copyright © 2012, 2015 Free Software Foundation, Inc.
+;;; Copyright © 2017 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -259,7 +260,10 @@ Raise an '&http-get-error' condition if downloading fails."
           ((200)
            (values data (response-content-length resp)))
           ((301                                   ; moved permanently
-            302)                                  ; found (redirection)
+            302                                   ; found (redirection)
+            303                                   ; see also
+            307                                   ; temporary redirect
+            308)                                  ; permanent redirect
            (let ((uri (resolve-uri-reference (response-location resp) uri)))
              (close-port port)
              (format #t (G_ "following redirection to `~a'...~%")
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index aceafc674..b27732d39 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2016 Danny Milosavljevic <dannym+a <at> scratchpost.org>
 ;;; Copyright © 2016 Hartmut Goebel <h.goebel <at> crazy-compilers.com>
 ;;; Copyright © 2017 Alex Kost <alezost <at> gmail.com>
+;;; Copyright © 2017 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -411,7 +412,11 @@ for connections to complete; when TIMEOUT is #f, wait as long as needed."
                    (close-connection port))))
 
              (case (response-code response)
-               ((301 302 307)
+               ((301                    ; moved permanently
+                 302                    ; found (redirection)
+                 303                    ; see also
+                 307                    ; temporary redirect
+                 308)                   ; permanent redirect
                 (let ((location (response-location response)))
                   (if (or (not location) (member location visited))
                       (values 'http-response response)
-- 
2.13.1

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

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: 28262 <at> debbugs.gnu.org
Subject: Re: [bug#28262] [PATCH] Handle the same HTTP redirects everywhere.
Date: Mon, 28 Aug 2017 15:52:06 +0200

Tobias Geerinckx-Rice wrote on 28/08/17 at 15:46:
> diff --git a/guix/http-client.scm b/guix/http-client.scm
> index 3c5441c38..8db332093 100644
> --- a/guix/http-client.scm
> +++ b/guix/http-client.scm
> @@ -259,7 +260,10 @@ Raise an '&http-get-error' condition if downloading fails."
>            ((200)
>             (values data (response-content-length resp)))
>            ((301                                   ; moved permanently
> -            302)                                  ; found (redirection)
> +            302                                   ; found (redirection)
> +            303                                   ; see also
> +            307                                   ; temporary redirect
> +            308)                                  ; permanent redirect

s/redirect$/redirection/

> diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
> index aceafc674..b27732d39 100644
> --- a/guix/scripts/lint.scm
> +++ b/guix/scripts/lint.scm
> @@ -411,7 +412,11 @@ for connections to complete; when TIMEOUT is #f, wait as long as needed."
>                     (close-connection port))))
>  
>               (case (response-code response)
> -               ((301 302 307)
> +               ((301                    ; moved permanently
> +                 302                    ; found (redirection)
> +                 303                    ; see also
> +                 307                    ; temporary redirect
> +                 308)                   ; permanent redirect
>                  (let ((location (response-location response)))
>                    (if (or (not location) (member location visited))
>                        (values 'http-response response)

Dittums.

Kind regards,

T G-R

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 28262 <at> debbugs.gnu.org
Subject: Re: [bug#28262] [PATCH] Handle the same HTTP redirects everywhere.
Date: Thu, 31 Aug 2017 15:10:12 +0200

Hey Tobias,

Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:

> * guix/download.scm (http-fetch): Complete the hard-coded list of HTTP
> redirect status codes.

Actually guix/build/download.scm.

> * guix/http-client.scm (http-fetch): Likewise.
> * guix/scripts/lint.scm (probe-uri): Likewise.
> ---
>
> Guix,
>
> There are three (that I know of) hard-coded lists of HTTP redirect status
> codes in Guix. All were different, and all were incomplete.
>
> This patch doesn't address the duplication, but does add all missing
> codes. Specifically the newer HTTP/1.1 codes, including 303 ‘See Other’.
> It's not strictly a plain redirect, but used as such in the wild[1], and
> treating it as such is probably enough for our purposes.
>
> This allows at least lightdm-gtk-greeter to be built again. Why its
> sources waren't mirrored to begin with I do not know, nor did I check.

Good catch, go for it!

As a followup, we should look into merging the two ‘http-fetch’
procedures.  I don’t think the initial motivation for having two
separate implementations still holds.

Thanks,
Ludo’.

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

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: ludo <at> gnu.org
Cc: 28262-done <at> debbugs.gnu.org
Subject: Re: [bug#28262] [PATCH] Handle the same HTTP redirects everywhere.
Date: Tue, 5 Sep 2017 18:21:36 +0200

Ludo',

Ludovic Courtès wrote on 31/08/17 at 15:10:
> Actually guix/build/download.scm.

Thanks!

> As a followup, we should look into merging the two ‘http-fetch’
> procedures.  I don’t think the initial motivation for having two
> separate implementations still holds.

I'll push this fix and close this bug, but if you have the time I'd love
to know more about that initial motivation. The two have grown quite apart.

> Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:
>> Specifically the newer HTTP/1.1 codes

I wrote that? Great. Now I feel old.

Kind regards,

T G-R

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 28262-done <at> debbugs.gnu.org
Subject: Re: [bug#28262] [PATCH] Handle the same HTTP redirects everywhere.
Date: Thu, 07 Sep 2017 11:43:07 +0200

Hi,

Tobias Geerinckx-Rice <me <at> tobias.gr> skribis:

>> As a followup, we should look into merging the two ‘http-fetch’
>> procedures.  I don’t think the initial motivation for having two
>> separate implementations still holds.
>
> I'll push this fix and close this bug, but if you have the time I'd love
> to know more about that initial motivation. The two have grown quite apart.

Initially (guix http-client) existed simply to paper over API changes in
Guile’s (web client) and to work around bugs in older Guile version.
This is because (guix http-client) is used on the “host” side, where we
support(ed) older releases of Guile 2.0.

(guix build download) didn’t really have this constraint because the
Guile used on the “build” side was known, and known to be recent enough
in most cases.  So all it did was to add a higher-level API above what
Guile provides, which follows redirects, etc.

Since 36626c556ed75219bce196ac93d148f6b9af984c we require Guile >=
2.0.9, so some of the bugs/workarounds we had no longer apply and the
separation probably no longer makes sense.

HTH!

Ludo’.

Previous Next

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