Re: [PATCH] Allow specifying the socket style for open-socket-for-uri.

[Maxime Devos <maximedevos@telenet.be>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2558 bytes --]



Op 20-07-2023 om 16:08 schreef Christopher Baines:
> Since this allows specifying additional behaviours for the socket
> through using SOCK_CLOEXEC and/or SOCK_NONBLOCK (when bitwise or'ed with
> SOCK_STREAM).
> 
> Note that Guile/guile-gnutls currently doesn't support performing the
> TLS handshake on a non-blocking socket, so this currently won't work.

I would think that it currently works for _http_. (https is another 
matter of course.)

> * module/web/client.scm (open-socket-for-uri): Allow specifying the
> socket style.
> ---
>   module/web/client.scm | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/module/web/client.scm b/module/web/client.scm
> index 6c54c5021..a5405d17f 100644
> --- a/module/web/client.scm
> +++ b/module/web/client.scm
> @@ -317,9 +317,11 @@ host name without trailing dot."
>     (read-response port))
>   
>   (define* (open-socket-for-uri uri-or-string
> -                              #:key (verify-certificate? #t))
> +                              #:key (verify-certificate? #t)
> +                              (socket-style SOCK_STREAM))
>     "Return an open input/output port for a connection to URI-OR-STRING.
> -When VERIFY-CERTIFICATE? is true, verify HTTPS server certificates."
> +When VERIFY-CERTIFICATE? is true, verify HTTPS server certificates.
> +SOCKET-STYLE is passed as the second argument to the socket procedure."

I think it should mention that socket-style doesn't work yet for https.

>     (define uri
>       (ensure-uri-reference uri-or-string))
>     (define https?
> @@ -346,7 +348,9 @@ When VERIFY-CERTIFICATE? is true, verify HTTPS server certificates."
>         (let* ((ai (car addresses))
>                (s  (with-fluids ((%default-port-encoding #f))
>                      ;; Restrict ourselves to TCP.
> -                   (socket (addrinfo:fam ai) SOCK_STREAM IPPROTO_IP))))
> +                   (socket (addrinfo:fam ai)
> +                           socket-style
> +                           IPPROTO_IP))))
>           (catch 'system-error
>             (lambda ()
>               (connect s (addrinfo:addr ai))

I don't think that #:socket-style SOCK_SEQPACKET makes much sense, so I 
would propose having a #:socket-flags argument instead.  Also, 
describing the new argument in terms of how it is used in the 
implementation doesn't seem great to me -- I would instead propose doing 
what the documentation 'accept' does with its 'flags' argument.

Best regards,
Maxime.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]