all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#44906: Substitute requests fail if URL has trailing slash
@ 2020-11-27 21:19 Hartmut Goebel
  2020-11-27 23:37 ` zimoun
  0 siblings, 1 reply; 6+ messages in thread
From: Hartmut Goebel @ 2020-11-27 21:19 UTC (permalink / raw)
  To: 44906

If the substitute-URL ends with a slash, api requests fail.

Expected behavior:

Substitute-URLs with and without trailing slash should behave the same.
This is especially true for substitute-URLs with empty path
("http://server") and path "/" (http://server/") - for which the RFCs
explicitly state to be equivalent.

According to RFC 7230, sec 2.7.3 "http and https URI Normalization and
Comparison" [1]:

   […] an empty
   path component is equivalent to an absolute path of "/", so the
   normal form is to provide a path of "/" instead.

[1] https://tools.ietf.org/html/rfc7230#section-2.7.3


How to reproduce:

no trailing slash:

$ guix weather --substitute-urls="https://ci.guix.gnu.org" gcc-toolchain
…
https://ci.guix.gnu.org
  100.0% substitutes available (3 out of 3)
…

Trailing slash:

$ guix weather --substitute-urls="https://ci.guix.gnu.org/" gcc-toolchain
…
https://ci.guix.gnu.org/
  0.0% substitutes available (0 out of 3)
…
  'https://ci.guix.gnu.org//api/queue?nr=1000' returned 400 ("Bad Request")

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |





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

* bug#44906: Substitute requests fail if URL has trailing slash
  2020-11-27 21:19 bug#44906: Substitute requests fail if URL has trailing slash Hartmut Goebel
@ 2020-11-27 23:37 ` zimoun
  2020-11-28  9:47   ` Hartmut Goebel
  0 siblings, 1 reply; 6+ messages in thread
From: zimoun @ 2020-11-27 23:37 UTC (permalink / raw)
  To: Hartmut Goebel, 44906

Dear,

Thank you for the report.


Tweaking the function such as:

--8<---------------cut here---------------start------------->8---
(define (narinfo-request cache-url path)
  "Return an HTTP request for the narinfo of PATH at CACHE-URL."
  (let ((url (string-append cache-url "/" (store-path-hash-part path)
                            ".narinfo"))
        (headers '((User-Agent . "GNU Guile"))))
    (format #t "~%Narinfo request: ~a~%~%" url)
    (build-request (string->uri url) #:method 'GET #:headers headers)))
--8<---------------cut here---------------end--------------->8---

and removing the cache adequately, then running:

--8<---------------cut here---------------start------------->8---
./pre-inst-env guix weather \
     --substitute-urls="https://ci.guix.gnu.org/ https://ci.guix.gnu.org" \
     hello
computing 1 package derivations for x86_64-linux...

looking for 1 store items on https://ci.guix.gnu.org/...

Narinfo request: https://ci.guix.gnu.org//a462kby1q51ndvxdv3b6p0rsixxrgx1h.narinfo

updating substitutes from 'https://ci.guix.gnu.org/'... 100.0%
https://ci.guix.gnu.org/
  0.0% substitutes available (0 out of 1)

[...]

  'https://ci.guix.gnu.org//api/queue?nr=1000' returned 400 ("Bad Request")

looking for 1 store items on https://ci.guix.gnu.org...

Narinfo request: https://ci.guix.gnu.org/a462kby1q51ndvxdv3b6p0rsixxrgx1h.narinfo

updating substitutes from 'https://ci.guix.gnu.org'... 100.0%
https://ci.guix.gnu.org
  100.0% substitutes available (1 out of 1)

[...]

  at least 1,000 queued builds

[...]

  build rate: 36.89 builds per hour

[...]
--8<---------------cut here---------------end--------------->8---


On Fri, 27 Nov 2020 at 22:19, Hartmut Goebel <h.goebel@crazy-compilers.com> wrote:

> According to RFC 7230, sec 2.7.3 "http and https URI Normalization and
> Comparison" [1]:
>
>    […] an empty
>    path component is equivalent to an absolute path of "/", so the
>    normal form is to provide a path of "/" instead.
>
> [1] https://tools.ietf.org/html/rfc7230#section-2.7.3

Now, the question is where should the fix go?  “guix publish” exposing
the narinfos or “guix weather“?  Or both?


From my understanding, one fix should go to ‘guix publish’ exposing the
narinfos since:

   https://ci.guix.gnu.org//a462kby1q51ndvxdv3b6p0rsixxrgx1h.narinfo

should be a valid URL and return the narinfo file.  However, taking this
road, it means that the cache folder will not be the same:

~/.cache/guix/substitute/x2wcz6gz3evwlqcrz3fqstmezkfcfnpfb5kfyxbz7kjikc7upkiq/
~/.cache/guix/substitute/4refhwxbjmeua2kwg2nmzhv4dg4d3dorpjefq7kiciw2pfhaf26a/

https://ci.guix.gnu.org/ resp. https://ci.guix.gnu.org  Therefore, ‘guix
weather’ should be fixed too.


WDYT?

All the best,
simon




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

* bug#44906: Substitute requests fail if URL has trailing slash
  2020-11-27 23:37 ` zimoun
@ 2020-11-28  9:47   ` Hartmut Goebel
  2020-12-03 17:01     ` Ludovic Courtès
  0 siblings, 1 reply; 6+ messages in thread
From: Hartmut Goebel @ 2020-11-28  9:47 UTC (permalink / raw)
  To: zimoun, 44906

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

Am 28.11.20 um 00:37 schrieb zimoun:
> Now, the question is where should the fix go?  “guix publish” exposing
> the narinfos or “guix weather“?  Or both?

I propose fixing all places where string-append is used to join URLs, 
since joining URLs is not the same as string concatenation. We might 
restrict our algorithm to only joining a path. 
<https://tools.ietf.org/html/rfc3986#section-5.2.2> shows the complete 
algorithm, where this is the relevant part for only joining a path 
(R.path) to a base URL's path (T.path).

                if (R.path starts-with "/") then
                   T.path = remove_dot_segments(R.path);
                else
                   T.path = merge(Base.path, R.path);
                   T.path = remove_dot_segments(T.path);

(Side-node: guile module (web uri) 
<https://www.gnu.org/software/guile/manual/html_node/URIs.html> seems to 
lack respective, easy to use functions.)

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |


[-- Attachment #2: Type: text/html, Size: 1918 bytes --]

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

* bug#44906: Substitute requests fail if URL has trailing slash
  2020-11-28  9:47   ` Hartmut Goebel
@ 2020-12-03 17:01     ` Ludovic Courtès
  2020-12-04  4:15       ` Mark H Weaver
  0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2020-12-03 17:01 UTC (permalink / raw)
  To: Hartmut Goebel; +Cc: 44906

Hi,

Hartmut Goebel <h.goebel@crazy-compilers.com> skribis:

> I propose fixing all places where string-append is used to join URLs,
> since joining URLs is not the same as string concatenation. We might 
> restrict our algorithm to only joining a
> path. <https://tools.ietf.org/html/rfc3986#section-5.2.2> shows the
> complete algorithm, where this is the relevant part for only joining a
> path (R.path) to a base URL's path (T.path).
>
>                if (R.path starts-with "/") then
>                   T.path = remove_dot_segments(R.path);
>                else
>                   T.path = merge(Base.path, R.path);
>                   T.path = remove_dot_segments(T.path);

To begin with, we could define ‘url-append’ in (guix http-client), say,
and use it in (guix scripts substitute).

Eventually it would be nice to have that in (web uri).

Thoughts?

Ludo’.




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

* bug#44906: Substitute requests fail if URL has trailing slash
  2020-12-03 17:01     ` Ludovic Courtès
@ 2020-12-04  4:15       ` Mark H Weaver
  2021-07-09  8:38         ` bug#44906: [PATCH 0/3] Properly construct URLs if base-url " Hartmut Goebel
  0 siblings, 1 reply; 6+ messages in thread
From: Mark H Weaver @ 2020-12-04  4:15 UTC (permalink / raw)
  To: Ludovic Courtès, Hartmut Goebel; +Cc: 44906

Hi,

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

> Hartmut Goebel <h.goebel@crazy-compilers.com> skribis:
>
>> I propose fixing all places where string-append is used to join URLs,
>> since joining URLs is not the same as string concatenation. We might 
>> restrict our algorithm to only joining a
>> path. <https://tools.ietf.org/html/rfc3986#section-5.2.2> shows the
>> complete algorithm, where this is the relevant part for only joining a
>> path (R.path) to a base URL's path (T.path).
>>
>>                if (R.path starts-with "/") then
>>                   T.path = remove_dot_segments(R.path);
>>                else
>>                   T.path = merge(Base.path, R.path);
>>                   T.path = remove_dot_segments(T.path);
>
> To begin with, we could define ‘url-append’ in (guix http-client), say,
> and use it in (guix scripts substitute).
>
> Eventually it would be nice to have that in (web uri).

Note that 'resolve-uri-reference' in (guix build download) implements
the algorithm specified in RFC 3986 section 5.2.2, for purposes of
supporting HTTP redirects.  Perhaps some of that code will be useful.

     Regards,
       Mark




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

* bug#44906: [PATCH 0/3] Properly construct URLs if base-url has trailing slash
  2020-12-04  4:15       ` Mark H Weaver
@ 2021-07-09  8:38         ` Hartmut Goebel
  0 siblings, 0 replies; 6+ messages in thread
From: Hartmut Goebel @ 2021-07-09  8:38 UTC (permalink / raw)
  To: 44906, Mark H Weaver, zimoun, Ludovic Courtès

Here is now an attempt to solve the issue. It had to be fixed in
guix/substitutes.scm and guix/ci.scm only. In guix/scripts/publish.scm I did
not spot any place where wrong URLs are constructed.

Many thanks to Mark for pointing to 'resolve-uri-reference'.

Regarding CI: I did some tests, so these should work. Anyhow, I did not find a
tests-suite for fully testing this part.

Hartmut Goebel (3):
  substitute: Fix handling of short option "-h".
  substitutes: Properly construct URLs.
  ci: Properly construct URLs.

 guix/ci.scm                 | 79 +++++++++++++++++++++----------------
 guix/scripts/substitute.scm |  2 +-
 guix/substitutes.scm        | 13 +++---
 3 files changed, 55 insertions(+), 39 deletions(-)

-- 
2.30.2





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

end of thread, other threads:[~2021-07-09  8:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-27 21:19 bug#44906: Substitute requests fail if URL has trailing slash Hartmut Goebel
2020-11-27 23:37 ` zimoun
2020-11-28  9:47   ` Hartmut Goebel
2020-12-03 17:01     ` Ludovic Courtès
2020-12-04  4:15       ` Mark H Weaver
2021-07-09  8:38         ` bug#44906: [PATCH 0/3] Properly construct URLs if base-url " Hartmut Goebel

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.