From: ludo@gnu.org (Ludovic Courtès)
To: Christopher Allan Webber <cwebber@dustycloud.org>
Cc: 24075@debbugs.gnu.org
Subject: bug#24075: tls/https support in Guile (through r6rs binary ports?)
Date: Sun, 06 Nov 2016 22:13:51 +0100 [thread overview]
Message-ID: <87oa1sxqeo.fsf@gnu.org> (raw)
In-Reply-To: <87lgww5x1y.fsf@dustycloud.org> (Christopher Allan Webber's message of "Sun, 06 Nov 2016 11:37:45 -0600")
Christopher Allan Webber <cwebber@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@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’.
next prev parent reply other threads:[~2016-11-06 21:13 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-26 15:55 bug#24075: tls/https support in Guile (through r6rs binary ports?) Christopher Allan Webber
2016-08-04 20:37 ` Andy Wingo
2016-08-21 15:58 ` Christopher Allan Webber
2016-11-05 18:39 ` Christopher Allan Webber
2016-11-05 19:02 ` Ludovic Courtès
2016-11-06 17:37 ` Christopher Allan Webber
2016-11-06 21:13 ` Ludovic Courtès [this message]
2016-11-07 18:14 ` Christopher Allan Webber
2016-11-07 20:13 ` Ludovic Courtès
2016-11-06 18:32 ` Christopher Allan Webber
2016-11-06 21:06 ` Ludovic Courtès
2016-11-07 4:40 ` Christopher Allan Webber
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87oa1sxqeo.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=24075@debbugs.gnu.org \
--cc=cwebber@dustycloud.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).