Subject: [bug#50882] [PATCH] gnu: services: Add darkhttpd service

From: Ludovic Courtès <[email-masked]>
Date: Mon, 25 Oct 2021 14:32:41 +0200

Hi,

jgart <[email-masked]> skribis:

> * gnu/services/web.scm (<darkhttpd-configuration>): New record type.
> (darkhttpd-accounts, darkhttpd-shepherd-service): New procedures.
> (darkhttpd-service-type): New variable.
> * doc/guix.texi (Web Services): Adds documentation for darkhttpd.

Overall LGTM!  Some comments and suggestions below.

> +@cindex darkhttpd
> +@uref{https://unix4lyfe.org/darkhttpd/, darkhttpd} is a web server with a 
> +focus on security and having a small memory footprint.
> +
> +Some security features are the following:
> +
> +@itemize
> +@item Logging accesses, including Referer and User-Agent.
> +@item Can chroot.
> +@item Can drop privileges.
> +@item Impervious to /../ sniffing.
> +@item Times out idle connections.
> +@item Drops overly long requests.
> +@end itemize 

I’d replace the bullet list with a simple sentence like: “Among other
things, it can change root directories, drop privileges, it times out on
idle connections and can drop overly long requests.”

> +@deffn {Scheme Variable} darkhttpd-service-type
> +This is the type of the darkhttpd service, whose value should be a
> +@code{darkhttpd-service-type} object, as in this example:
> +
> +@lisp
> +(service darkhttpd-service-type
> +	 (darkhttpd-configuration

Please don’t use tabs.

> +@end table
> +@end deftp
>  @node Certificate Services

Missing newline before @node.  :-)

> +  (mimetypes               darkhttpd-configuration-mimetypes
> +                           (default #f))
> +  (default-mimetype        darkhttpd-configuration-default-mimetype

Rather ‘mime-type’ (two words).

> +(define darkhttpd-shepherd-service
> +  (match-lambda
> +    (($ <darkhttpd-configuration> package content port address 
> +                                  maximum-connections log-file chroot? 
> +                                  daemonize? index-file do-not-serve-listing?
> +                                  mimetypes default-mimetype 
> +                                  drop-user-priviledges drop-group-priviledges 
> +                                  write-pid-file disable-keep-alive? 
> +                                  forward forward-all 
> +                                  no-server-id? enable-ipv6? 
> +                                  user group)

Rather use ‘match-record’ here, to make sure we’re getting the right
fields.

> +(define darkhttpd-accounts
> +  (match-lambda
> +    (($ <darkhttpd-configuration> _ _ _ _ _ _ _ _ 
> +                                  _ _ _ _ _ _ _ _ 
> +                                  _ _ user group)

In such a case, simply call ‘darkhttpd-configuration-user’ and
‘darkhttpd-configuration-group’; it’s much less error-prone!

> +(define darkhttpd-service-type
> +  (service-type
> +   (name 'guix)
> +   (extensions
> +    (list (service-extension account-service-type
> +                             darkhttpd-accounts)
> +          (service-extension shepherd-root-service-type
> +                             darkhttpd-shepherd-service)))
> +   (default-value (darkhttpd-configuration))))

Please add a ‘description’ field.

Could you also add a system test, under (gnu tests web)?  You can start
by copying the nginx test; it should take around ~20 lines.

TIA!

Ludo’.