[PATCH] lint: Check if HTTPS version of HTTP URL exists.

  • Open
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • Xinglu Chen
Owner
unassigned
Submitted by
Xinglu Chen
Severity
normal
X
X
Xinglu Chen wrote on 28 Sep 2021 21:09
(address . guix-patches@gnu.org)
e2047d5738d30969bc766ef85ea65715954a6927.1632855961.git.public@yoctocell.xyz
* guix/lint.scm (check-if-https-uri-exists?): New procedure.
(check-home-page, check-source): Use it.
---
I don’t really know how to test this while making it future-proof, any
suggestions?

guix/lint.scm | 41 ++++++++++++++++++++++++++++++++++++-----
1 file changed, 36 insertions(+), 5 deletions(-)

Toggle diff (71 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 527fda165a..246a5ab9c8 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -875,17 +875,44 @@ (define (validate-uri uri package field)
(else
(error "internal linter error" status)))))
+(define (check-if-https-uri-exists? uri field package)
+ "Given a URI that uses HTTP, check whether a HTTPS version exists."
+ (guard (c ((http-get-error? c)
+ #f))
+ (catch #t
+ (lambda ()
+ (let* ((url (uri->string uri))
+ (https-url (string-append
+ "https" (string-drop url (string-length "http"))))
+ (https-uri (string->uri https-url)))
+ (when (http-fetch/cached https-uri)
+ (make-warning package
+ (G_ "HTTPS version is available for: ~a")
+ (list url)
+ #:field field))))
+ (match-lambda*
+ ((or ('gnutls-error _ ...) ('tls-certificate-error _ ...))
+ #f)
+ (args
+ (apply throw args))))))
+
(define (check-home-page package)
"Emit a warning if PACKAGE has an invalid 'home-page' field, or if that
'home-page' is not reachable."
- (let ((uri (and=> (package-home-page package) string->uri)))
+ (let* ((home-page (package-home-page package))
+ (uri (and=> home-page string->uri)))
(cond
((uri? uri)
(match (validate-uri uri package 'home-page)
((and (? lint-warning? warning) warning)
(list warning))
- (_ '())))
- ((not (package-home-page package))
+ (_ (if (eq? (uri-scheme uri) 'http)
+ (match (check-if-https-uri-exists? uri 'home-page package)
+ ((? lint-warning? warning)
+ (list warning))
+ (_ '()))
+ '()))))
+ ((not home-page)
(if (or (string-contains (package-name package) "bootstrap")
(string=? (package-name package) "ld-wrapper"))
'()
@@ -1079,8 +1106,12 @@ (define (warnings-for-uris uris)
((uri rest ...)
(match (validate-uri uri package 'source)
(#t
- ;; We found a working URL, so stop right away.
- '())
+ (if (eq? (uri-scheme uri) 'http)
+ (match (check-if-https-uri-exists? uri 'source package)
+ ((? lint-warning? warning)
+ (list warning))
+ (_ '()))
+ '()))
(#f
;; Unsupported URL or other error, skip.
(loop rest warnings))

base-commit: 009f0fc3dde0c2162c6df02fc4790a9f1d909e99
--
2.33.0
L
L
Ludovic Courtès wrote on 2 Oct 2021 17:15
(name . Xinglu Chen)(address . public@yoctocell.xyz)(address . 50874@debbugs.gnu.org)
877devwj95.fsf@gnu.org
Hi,

Xinglu Chen <public@yoctocell.xyz> skribis:

Toggle quote (3 lines)
> * guix/lint.scm (check-if-https-uri-exists?): New procedure.
> (check-home-page, check-source): Use it.

Applied, thanks!

Toggle quote (3 lines)
> I don’t really know how to test this while making it future-proof, any
> suggestions?

I don’t know either, since we don’t have an easy way to spin up an HTTPS
server. I think it’s okay to leave it as is, for lack of a better idea.

However, this version of the patch leads to test failures in
tests/lint.scm (“Connection refused”).

Ludo’.
X
X
Xinglu Chen wrote on 9 Oct 2021 11:57
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 50874@debbugs.gnu.org)
87v926ldvv.fsf@yoctocell.xyz
On Sat, Oct 02 2021, Ludovic Courtès wrote:

Toggle quote (9 lines)
> Hi,
>
> Xinglu Chen <public@yoctocell.xyz> skribis:
>
>> * guix/lint.scm (check-if-https-uri-exists?): New procedure.
>> (check-home-page, check-source): Use it.
>
> Applied, thanks!

Was it? I don’t see it in the log.

Toggle quote (9 lines)
>> I don’t really know how to test this while making it future-proof, any
>> suggestions?
>
> I don’t know either, since we don’t have an easy way to spin up an HTTPS
> server. I think it’s okay to leave it as is, for lack of a better idea.
>
> However, this version of the patch leads to test failures in
> tests/lint.scm (“Connection refused”).

Thanks for catching this; I will look into it. Good that you didn’t
apply it then. :-)
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEAVhh4yyK5+SEykIzrPUJmaL7XHkFAmFhZ3QVHHB1YmxpY0B5
b2N0b2NlbGwueHl6AAoJEKz1CZmi+1x5G8MQAIRb9hCAiGe8IUTXo5W5DEIGpnNJ
hsb2Cb6Leq28CsamOC1RL+020pyo5mdUSXR0aiCjpQB4xo7h4hUt7t05+x0ArmwK
lliynomsDadwcKrFIdzHYQhznCdEqo+Kj48UZz70XpZfbO6JgnYYK+L52Kt1BL0a
1HkIeFhugbY3zzmihN9oAOGlZgwlQpdURAUYO8Asf7CC5GlkO2PWQOBmhKC6U6oB
k2YkNXiBmXoDW7cHtFnXhVqtTPYIsliF1uidSKrfeo2YvFTFw4dwJ6OGLic2IGFP
H2E7FZFpYKFhzPrMJiwxShuDaA1bW5nRCv+2Y2nB/uU/kOjKhULT/fyhgctiNabq
VJuqK71ntk/1jjhM2N9IdPDJFluJ+rd0jQ6yVUVDNuGaP69qjtxSKIoAcdCXSokA
XGU3cWTN7Y6u6dSiNuwj7s7g7nupz7BRaZ4YUZ+ykcbCxlOUjDIwpQjO5kv4QwAU
T+XpNY6azoIBEJ4GTc6nokCNivB9NeaZJ2LfxBU2r1culMonMHgLAJAFAgp7Oz2G
nKSNyeVN2azDCO9aJbhViX6EpKulXii4DgNdwxsRXGv2Ox3QSQDvUXU97Zias8AI
djcTFoz1Wp5AX/3FYzDnXpibJkbiEaIbdk8BVpw5QGY02r0wsbFVBYnsfdEO4rNj
MKx5/JTnxNZ2YilN
=R/nz
-----END PGP SIGNATURE-----

X
X
Xinglu Chen wrote on 9 Oct 2021 12:11
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 50874@debbugs.gnu.org)
87sfxald81.fsf@yoctocell.xyz
On Sat, Oct 09 2021, Xinglu Chen wrote:

Toggle quote (25 lines)
> On Sat, Oct 02 2021, Ludovic Courtès wrote:
>
>> Hi,
>>
>> Xinglu Chen <public@yoctocell.xyz> skribis:
>>
>>> * guix/lint.scm (check-if-https-uri-exists?): New procedure.
>>> (check-home-page, check-source): Use it.
>>
>> Applied, thanks!
>
> Was it? I don’t see it in the log.
>
>>> I don’t really know how to test this while making it future-proof, any
>>> suggestions?
>>
>> I don’t know either, since we don’t have an easy way to spin up an HTTPS
>> server. I think it’s okay to leave it as is, for lack of a better idea.
>>
>> However, this version of the patch leads to test failures in
>> tests/lint.scm (“Connection refused”).
>
> Thanks for catching this; I will look into it. Good that you didn’t
> apply it then. :-)

Hm, I am not able to reproduce this. All the tests in ‘tests/lint.scm’
pass for me.
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEAVhh4yyK5+SEykIzrPUJmaL7XHkFAmFhas4VHHB1YmxpY0B5
b2N0b2NlbGwueHl6AAoJEKz1CZmi+1x5+GkQAI37Ey0Ovxp++dfjLaWxU/eeyDmc
zTnrje2wbPQ0dPAn0Qj1aSoRQM3NleTGR56T2IcPpjLihwgajJkoAjSfLthngFM1
uGNnIGrOqfS5kh6I9RsjH0vjVgfhYCJAIdeu/NZKzCxsrsjIJ4NXbO2yOWiPsyMq
baANhzScD9TXBnIBbq3JypJrlf0P/e5qtigzt5eSRdmipcAlag3nWnc4bLdYokU1
X8uZXwuLpQhrRTZ81LbXepCHNMR+YIZCHRkapblCmX/jk8VxiPSfs3LMlpUK8WCs
hJ5QqLg6xn6Fm5SvgmyXeQx/MGzW5qoScO3SfB6SuGYIBo6U+euIollH4NWo48EP
ZrqO+eJ5kO3om+TBHWBkE3Q6OGLtRqYiJsckLVnw0w4R/UzrHOiq1qnsYfpexcO2
HWEnJ1BYC4ZDexClZdNcM1/tedcMlng6s+Mh029DU6xN8RR4coNrz45+p1LICwj0
gTerIXFJkyQz25nnJoUKM2UAKcU/dT+hpPkbsPEvz/Ox1FAtqdZfv8hglTjb5trd
Wjik4exQxGXuKrjNfykhbRIJfNtMUSHoek9fCaKseJQa6gFsqv/Hjdr2XYKGCPZm
t0TaXF2KrFiY0XiAlTVhyg4qDzRxUSQAEqSmUTf0DQ3f1rOl67Pvbh556O4+jQF4
f2RmsxJnKZAV1BeJ
=vokx
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 10 Oct 2021 15:12
(name . Xinglu Chen)(address . public@yoctocell.xyz)(address . 50874@debbugs.gnu.org)
87y271vx9w.fsf_-_@gnu.org
Hi!

Xinglu Chen <public@yoctocell.xyz> skribis:

Toggle quote (13 lines)
> On Sat, Oct 02 2021, Ludovic Courtès wrote:
>
>> Hi,
>>
>> Xinglu Chen <public@yoctocell.xyz> skribis:
>>
>>> * guix/lint.scm (check-if-https-uri-exists?): New procedure.
>>> (check-home-page, check-source): Use it.
>>
>> Applied, thanks!
>
> Was it? I don’t see it in the log.

Actually no. :-) I initially applied it, started replying, noticed
there was a problem, and then sent that inconsistent reply.

Toggle quote (12 lines)
>>> I don’t really know how to test this while making it future-proof, any
>>> suggestions?
>>
>> I don’t know either, since we don’t have an easy way to spin up an HTTPS
>> server. I think it’s okay to leave it as is, for lack of a better idea.
>>
>> However, this version of the patch leads to test failures in
>> tests/lint.scm (“Connection refused”).
>
> Thanks for catching this; I will look into it. Good that you didn’t
> apply it then. :-)

Here’s what I see (among others):

Toggle snippet (51 lines)
test-name: home-page: 200
location: /home/ludo/src/guix/tests/lint.scm:650
source:
+ (test-equal
+ "home-page: 200"
+ '()
+ (with-http-server
+ `((200 ,%long-string))
+ (let ((pkg (package
+ (inherit (dummy-package "x"))
+ (home-page (%local-url)))))
+ (check-home-page pkg))))
expected-value: ()
actual-value: #f
actual-error:
+ (system-error
+ connect*
+ "~A"
+ ("Connection refused")
+ (111))
result: FAIL

[…]

test-name: source: 200
location: /home/ludo/src/guix/tests/lint.scm:917
source:
+ (test-equal
+ "source: 200"
+ '()
+ (with-http-server
+ `((200 ,%long-string))
+ (let ((pkg (package
+ (inherit (dummy-package "x"))
+ (source
+ (origin
+ (method url-fetch)
+ (uri (%local-url))
+ (sha256 %null-sha256))))))
+ (check-source pkg))))
expected-value: ()
actual-value: #f
actual-error:
+ (system-error
+ connect*
+ "~A"
+ ("Connection refused")
+ (111))
result: FAIL

I believe that’s because, in addition to (%local-url), ‘check-home-page’
& co. now try to connect on port 443 of the same host, and there’s
nothing listening to that port on my machine.

Ludo’.
L
L
Ludovic Courtès wrote on 23 Nov 2021 09:09
control message for bug #50874
(address . control@debbugs.gnu.org)
871r37mgxr.fsf@gnu.org
tags 50874 + moreinfo
quit
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 50874@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 50874
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch