GNU bug report logs - #24075
tls/https support in Guile (through r6rs binary ports?)

Previous Next

Full log

View this message in rfc822 format

From: ludo <at> gnu.org (Ludovic Courtès)
To: Christopher Allan Webber <cwebber <at> dustycloud.org>
Cc: Andy Wingo <wingo <at> pobox.com>, 24075 <at> debbugs.gnu.org
Subject: bug#24075: tls/https support in Guile (through r6rs binary ports?)
Date: Sun, 06 Nov 2016 22:13:51 +0100

Christopher Allan Webber <cwebber <at> dustycloud.org> skribis:

> Ludovic Courtès writes:
>
>>> +(define (ensure-gnutls)
>>> +  (if (not (force gnutls-module))
>>> +      (throw 'gnutls-not-available "(gnutls) module not available")))
>>
>> I wonder if this is the right exception, but I can’t think of anything
>> better (there’s no generic “not supported” exception I think; (throw
>> 'system-error … ENOSYS) would do that but it’s too vague.)
>
> I don't know... it's hard for me to tell when to use what exception
> symbol in Guile!
>
> I prefer specific exceptions when a more general exception

Yes, I agree.  I was just wondering out loud whether there might be
another exception.  Anyway, this one’s fine!

>> What about leaving the ‘ensure-gnutls’ call and then simply use the
>> GnuTLS symbols directly and rely on autoloading, as in (guix build
>> download)?
>>
>> --8<---------------cut here---------------start------------->8---
>> ;; Autoload GnuTLS so that this module can be used even when GnuTLS is
>> ;; not available.  At compile time, this yields "possibly unbound
>> ;; variable" warnings, but these are OK: we know that the variables will
>> ;; be bound if we need them, because (guix download) adds GnuTLS as an
>> ;; input in that case.
>>
>> ;; XXX: Use this hack instead of #:autoload to avoid compilation errors.
>> ;; See <http://bugs.gnu.org/12202>.
>> (module-autoload! (current-module)
>>                   '(gnutls) '(make-session connection-end/client))
>> --8<---------------cut here---------------end--------------->8---
>>
>> That would lead more concise and slightly more efficient code, and I
>> think it would still work as expected in the absence of (gnutls).
>>
>> WDYT?
>
> So there was this converstaion on #guile:
>
>   <civodul> mark_weaver: the autoload hack fails gracelessly when GnuTLS is       
>             missing
>   <civodul> that's fine in the context of Guix, but maybe not in a more general   
>             context
>   <paron_remote> oh :)
>   <paron_remote> civodul: what approach would you suggest then?
>   <mark_weaver> civodul: could we make it more graceful?
>   <civodul> yeah maybe with some explicit module hackery
>   <civodul> an explicit resolve-interface + module-ref
>   <civodul> something like that
>   <mark_weaver> sounds doable
>
> So... that's what lead me to change it.
>
> Admittedly I'm not totally clear what was meant by "the autoload hack
> fails gracelessly", and what would be more graceful.  Would it be
> because it's trying to utilize a symbol that's not bound to anything?
>
> Which leads to the next question: if I did the autoload hack, what would
> (ensure-gnutls) look like?

Sorry for the confusing statements.  :-)

I think ‘ensure-gnutls’ would be exactly as in this patch, and the
autoload hack would be exactly as shown above.

Here it would fail “gracefully” in the sense that ‘ensure-gnutls’ would
catch a potential problem early on and raise the ‘gnutls-not-available’
exception (you’d have to double-check that this is indeed the behavior
we get, but I’m quite confident ;-)).

In (guix build download) there’s no such protection so when (gnutls) is
missing, users get an unbound variable right in the middle of the code.

>>> +      (define (read! bv start count)
>>> +        (define read-bv (get-bytevector-n record count))
>>> +        (define read-bv-len (bytevector-length read-bv))
>>> +        (bytevector-copy! read-bv 0 bv 0 read-bv-len)
>>> +        read-bv-len)
>>
>> Beware: ‘get-bytevector-n’ can return the EOF object instead of a
>> number, so you need to check for that.  (Conversely, ‘read!’ needs to
>> return 0 to indicate EOF.)
>
> So that would look like this?
>
>       (define (read! bv start count)
>         (define read-bv (get-bytevector-n record count))
>         (if (eof-object? read-bv)
>             0
>             (let ((read-bv-len (bytevector-length read-bv)))
>               (bytevector-copy! read-bv 0 bv 0 read-bv-len)
>               read-bv-len)))

Exactly.

>>> +  (define (open-socket)
>>> +    (let loop ((addresses addresses))
>>
>> Or just “(define sock …”.
>
> Hm, is that a good idea?  Does this need to happen before or within the
> with-https-proxy?

Oh you’re right, sorry.

> From 91c0a4a728ca4bf2e9468cdc849c350dd3f7380f Mon Sep 17 00:00:00 2001
> From: Christopher Allan Webber <cwebber <at> dustycloud.org>
> Date: Thu, 17 Sep 2015 15:14:54 -0500
> Subject: [PATCH] web: Add https support through gnutls.
>
> Since importing gnutls directly would result in a dependency cycle,
> we load gnutls lazily.
>
> This uses code originally written for Guix by Ludovic
>
> * module/web/client.scm: (%http-receive-buffer-size)
>   (warn-no-gnutls-return-false, gnutls-module, ensure-gnutls)
>   (gnutls-ref, tls-wrap): New variables.
>   (open-socket-for-uri): Wrap in tls when uri scheme is https.
> * doc/ref/web.texi (open-socket-for-uri): Document gnutls usage.

[...]

>  @deffn {Scheme Procedure} open-socket-for-uri uri
> -Return an open input/output port for a connection to URI.
> +Return an open input/output port for a connection to URI.  Guile
> +dynamically loads gnutls for https support; for more information, see
> +@xref{Guile Preparations,
> +how to install the GnuTLS bindings for Guile,, gnutls-guile,
> +GnuTLS-Guile}.

@xref generates a “See” for the beginning of a sentence, so it should
be:

  … support.  @xref{…}, for more information.

Also, “HTTPS” and “GnuTLS”.  :-)

The rest is all good for me, so the only remaining bits are the autoload
thing and maybe the bug you observed?

Thanks!

Ludo’.

Previous Next

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