Index | Thread | Search

From:
Lloyd <ng2d68@proton.me>
Subject:
Re: add support to httpd for serving static brotli encoded files
To:
tech@openbsd.org
Date:
Tue, 24 Feb 2026 03:31:38 +0000

Download raw body.

Thread
SRVFLAG_BITS in httpd.h will need to be updated (used for debugging).

Now that I look at it, the values seem to be out of order in -current.

Might as well fix that while you're in there.

Regards
Lloyd

Adam Mullins wrote:

> Whoops. I sent that a little early. I also did not justify it 72
> characters. Sorry about that.
> 
> I also meant to add that I commented more heavily than the rest of the
> httpd files. I don't expect these comments to survive review, but I
> wanted to outline my thought process for the reviewer.
> 
> The unified diff is included below.
> 
> On 2/23/26 9:31 PM, Adam Mullins wrote:
>  > Hello. This is my first time contributing to a free software
>  > project. Feel free to let me know if I have made any mistakes or
>  > missed anything. Sorry for the long email, but I wanted to cover
>  > everything.
>  >
>  > Currently httpd has the ability to preferentially serve gzip encoded
>  > static files, but not brotli encoded files. This patch adds the
>  > brotli-static option to httpd, allowing it to serve .br files to
>  > clients. Brotli offers 10-20% better compression than gzip for common
>  > web file types.
>  >
>  > With this patch, when brotli-static is set in httpd.conf, httpd will
>  > serve a *.br file if the client has signaled it will accept it, and
>  > the connection is inside of TLS, and the *.br version is no older than
>  > the *.gz and unencoded file. Failing that, it will try to serve the
>  > gzip version under the same constraints, minus the TLS
>  > requirement. Finally, it defaults to serving the unencoded file.
>  >
>  > Changes:
>  > - Update httpd.conf.5 to explain the new option.
>  > - Add rules and logic for parsing httpd.conf to parse.y.
>  > - Add SRVFLAG_BROTLI_STATIC to httpd.h.
>  > - Migrate encoded file selection logic out of
>  >    server_file.c:server_file_access() to a helper
>  >   static function, and adds logic to select *.br files if they exist.
>  >
>  > I was uncertain about a couple of things.
>  >
>  > First, I #define'd SRVFLAG_BROTLI_STATIC to 0x10000000. This particular
>  > number was skipped by the other flags (ie, the flags went from
>  > 0x08... to 0x40...). I'm not sure if this constant is being avoided for
>  > some reason, but everything seems to be working okay. Also, with this
>  > addition we are out of possible 32 bit combinations unless I am
>  > missing something.
>  >
>  > Second, I tried a few approaches to the selection logic flow, but I
>  > couldn't get it any more compact. It was taking up a lot of space in the
>  > original function, so I migrated it to a helper. I only noticed a few
>  > others like this in the project, so I'm not sure if this idiomatic. Also,
>  > while my additions check the return of libc calls like fstat() and
>  > snprintf() for errors, it _does not_ check that all passed in pointers
>  > are non-NULL. This seems reasonable to me given that it is a file-local
>  > helper, called from only one function, and that caller likewise does not
>  > check its arguments (ie server_file_access does not verify
>  > `struct client *clt` is non-NULL before dereferencing it).
> 
> cvs server: Diffing .
> Index: httpd.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
> diff -u -p -u -r1.129 httpd.conf.5
> --- httpd.conf.5        18 Jan 2026 16:38:02 -0000      1.129
> +++ httpd.conf.5        24 Feb 2026 02:41:19 -0000
> @@ -464,6 +464,15 @@ Enable static gzip compression to save b
>   If gzip encoding is accepted and if the requested file exists with
>   an additional .gz suffix, use the compressed file instead and deliver
>   it with content encoding gzip.
> +.It Ic brotli-static
> +Enable static brotli compression to save bandwidth.
> +.Pp
> +If brotli encoding is accepted, the requested file exists with a .br
> suffix,
> +and the client is connected with TLS, use the compressed file instead and
> +deliver it with content encoding br.
> +.Pp
> +If both brotli-static and gzip-static are enabled, brotli is preferred
> +if the above conditions are satisfied.
>   .It Ic hsts Oo Ar option Oc
>   Enable HTTP Strict Transport Security.
>   Valid options are:
> Index: httpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> diff -u -p -u -r1.167 httpd.h
> --- httpd.h     28 Nov 2025 16:10:00 -0000      1.167
> +++ httpd.h     24 Feb 2026 02:41:19 -0000
> @@ -390,6 +390,7 @@ SPLAY_HEAD(client_tree, client);
>   #define SRVFLAG_NO_PATH_REWRITE        0x02000000
>   #define SRVFLAG_GZIP_STATIC    0x04000000
>   #define SRVFLAG_NO_BANNER      0x08000000
> +#define SRVFLAG_BROTLI_STATIC  0x10000000
>   #define SRVFLAG_LOCATION_FOUND 0x40000000
>   #define SRVFLAG_LOCATION_NOT_FOUND 0x80000000
> 
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> diff -u -p -u -r1.130 parse.y
> --- parse.y     28 Nov 2025 16:10:00 -0000      1.130
> +++ parse.y     24 Feb 2026 02:41:19 -0000
> @@ -141,7 +141,7 @@ typedef struct {
>   %token TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD
> REQUEST
>   %token ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
>   %token CA CLIENT CRL OPTIONAL PARAM FORWARDED FOUND NOT
> -%token ERRDOCS GZIPSTATIC BANNER
> +%token ERRDOCS GZIPSTATIC BANNER BROTLISTATIC
>   %token <v.string>      STRING
>   %token  <v.number>     NUMBER
>   %type  <v.port>        port
> @@ -561,6 +561,7 @@ serveroptsl : LISTEN ON STRING opttls po
>                  | fastcgi
>                  | authenticate
>                  | gzip_static
> +               | brotli_static
>                  | filter
>                  | LOCATION optfound optmatch STRING     {
>                          struct server           *s;
> @@ -1249,6 +1250,14 @@ gzip_static      : NO GZIPSTATIC         {
>                  }
>                  ;
> 
> +brotli_static  : NO BROTLISTATIC       {
> +                       srv->srv_conf.flags &= ~SRVFLAG_BROTLI_STATIC;
> +               }
> +               | BROTLISTATIC          {
> +                       srv->srv_conf.flags |= SRVFLAG_BROTLI_STATIC;
> +               }
> +               ;
> +
>   tcpip          : TCP '{' optnl tcpflags_l '}'
>                  | TCP tcpflags
>                  ;
> @@ -1455,6 +1464,7 @@ lookup(char *s)
>                  { "block",              BLOCK },
>                  { "body",               BODY },
>                  { "buffer",             BUFFER },
> +               { "brotli-static",      BROTLISTATIC },
>                  { "ca",                 CA },
>                  { "certificate",        CERTIFICATE },
>                  { "chroot",             CHROOT },
> Index: server_file.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> diff -u -p -u -r1.80 server_file.c
> --- server_file.c       29 Apr 2024 16:17:46 -0000      1.80
> +++ server_file.c       24 Feb 2026 02:41:19 -0000
> @@ -55,6 +55,8 @@ int            server_file_method(struct client *
>   int             parse_range_spec(char *, size_t, struct range *);
>   int             parse_ranges(struct client *, char *, size_t);
>   static int      select_visible(const struct dirent *);
> +static int      server_file_encoded_path(const struct client *, const
> char *,
> +                    int *, struct stat *);
> 
>   int
>   server_file_access(struct httpd *env, struct client *clt,
> @@ -168,44 +170,10 @@ server_file_access(struct httpd *env, st
>                              fd, &st, r->kv_value));
>          }
> 
> -       /* change path to path.gz if necessary. */
> -       if (srv_conf->flags & SRVFLAG_GZIP_STATIC) {
> -               struct http_descriptor  *req = clt->clt_descreq;
> -               struct http_descriptor  *resp = clt->clt_descresp;
> -               struct stat              gzst;
> -               int                      gzfd;
> -               char                     gzpath[PATH_MAX];
> -
> -               /* check Accept-Encoding header */
> -               key.kv_key = "Accept-Encoding";
> -               r = kv_find(&req->http_headers, &key);
> -
> -               if (r != NULL && strstr(r->kv_value, "gzip") != NULL) {
> -                       /* append ".gz" to path and check existence */
> -                       ret = snprintf(gzpath, sizeof(gzpath), "%s.gz",
> path);
> -                       if (ret < 0 || (size_t)ret >= sizeof(gzpath)) {
> -                               close(fd);
> -                               return (500);
> -                       }
> -
> -                       if ((gzfd = open(gzpath, O_RDONLY)) != -1) {
> -                               /* .gz must be a file, and not older */
> -                               if (fstat(gzfd, &gzst) != -1 &&
> -                                   S_ISREG(gzst.st_mode) &&
> -                                   timespeccmp(&gzst.st_mtim, &st.st_mtim,
> -                                   >=)) {
> -  kv_add(&resp->http_headers,
> -                                           "Content-Encoding", "gzip");
> -                                       /* Use original file timestamp */
> -                                       gzst.st_mtim = st.st_mtim;
> -                                       st = gzst;
> -                                       close(fd);
> -                                       fd = gzfd;
> -                               } else {
> -                                       close(gzfd);
> -                               }
> -                       }
> -               }
> +       /* Send encoded file if client and server agree. */
> +       if((ret = server_file_encoded_path(clt, path, &fd, &st)) != 0) {
> +               close(fd);
> +               return (ret);
>          }
> 
>          return (server_file_request(env, clt, media, fd, &st));
> @@ -823,4 +791,124 @@ parse_range_spec(char *str, size_t size,
>                  return (0);
> 
>          return (1);
> +}
> +
> +/*
> + * If brotli-static is enabled,
> + *   and the connection is inside of TLS,
> + *   and the file isn't older than the html
> + *   and (if gzip-static is enabled, and
> + *       the brotli isn't older than the gzip),
> + *   then send the brotli version.
> + * Else if gzip-static is enabled,
> + *   and the file exists
> + *   and it's newer than the html version,
> + *   then send the gzip version.
> + * Otherwise send the regular file.
> + */
> +static int
> +server_file_encoded_path(const struct client *clt, const char *path,
> +    int *fd, struct stat *st)
> +{
> +       struct kv               *r, key;
> +       struct server_config    *srv_conf = clt->clt_srv_conf;
> +       int                      ret;
> +       struct http_descriptor  *req = clt->clt_descreq;
> +       struct http_descriptor  *resp = clt->clt_descresp;
> +       struct stat              brst;
> +       int                      brfd = -1;
> +       char                     brpath[PATH_MAX];
> +       struct stat              gzst;
> +       int                      gzfd = -1;
> +       char                     gzpath[PATH_MAX];
> +
> +       /* did client send Accept-Encoding header? */
> +       key.kv_key = "Accept-Encoding";
> +       r = kv_find(&req->http_headers, &key);
> +
> +       /* if brotli-static is set, client accepts brotli, and
> +        * connection is inside tls, set brst & brfd for later use. */
> +       if ((srv_conf->flags & SRVFLAG_BROTLI_STATIC) &&
> +           r != NULL &&
> +           strstr(r->kv_value, "br") != NULL &&
> +           clt->clt_tls_ctx != NULL) {
> +               /* append .br... */
> +               ret = snprintf(brpath, sizeof(brpath), "%s.br", path);
> +               if (ret < 0 || (size_t)ret >= sizeof(brpath)) {
> +                       return (500);
> +               }
> +               /* ...and check existence. */
> +               if ((brfd = open(brpath, O_RDONLY)) != -1) {
> +                       if(fstat(brfd, &brst) == -1) {
> +                               /* couldn't stat file, bail */
> +                               close(brfd);
> +                               return (500);
> +                       }
> +               }
> +       }
> +
> +       /* if client accepts gzip */
> +       if ((srv_conf->flags & SRVFLAG_GZIP_STATIC) &&
> +           r != NULL && strstr(r->kv_value, "gzip") != NULL) {
> +               /* append .gz... */
> +               ret = snprintf(gzpath, sizeof(gzpath), "%s.gz", path);
> +               if (ret < 0 || (size_t)ret >= sizeof(gzpath)) {
> +                       /* brfd might be open here */
> +                       if (brfd != -1) close(brfd);
> +                       return (500);
> +               }
> +               /* ...and check existence. */
> +               if ((gzfd = open(gzpath, O_RDONLY)) != -1) {
> +                       if(fstat(gzfd, &gzst) == -1) {
> +                               /* brfd might be open here */
> +                               if (brfd != -1) close(brfd);
> +                               close(gzfd);
> +                               return (500);
> +                       }
> +               }
> +       }
> +
> +       if (brfd != -1 &&
> +           S_ISREG(brst.st_mode) &&
> +           timespeccmp(&brst.st_mtim, &st->st_mtim, >=) &&
> +           (gzfd == -1 || timespeccmp(&brst.st_mtim, &gzst.st_mtim, >=)))
> +       {
> +               /* if .br is a valid option, is a regular file, not
> older than
> +                  the base file, and not older than the gzip version (if it
> +                  exists), then serve the .br file. */
> +               kv_add(&resp->http_headers,
> +                      "Content-Encoding", "br");
> +               /* Use original file timestamp */
> +               brst.st_mtim = st->st_mtim;
> +               /* Point fd at the br version. */
> +               *st = brst;
> +               close(*fd);
> +               *fd = brfd;
> +               brfd = -1;
> +       } else if (gzfd != -1 &&
> +                  S_ISREG(gzst.st_mode) &&
> +                  timespeccmp(&gzst.st_mtim, &st->st_mtim, >=))
> +       {
> +               /* otherwise, if .gz is a valid option, is a regular
> +                  file, and not older than the base file, serve it.*/
> +               kv_add(&resp->http_headers,
> +                      "Content-Encoding", "gzip");
> +               /* Use original file timestamp */
> +               gzst.st_mtim = st->st_mtim;
> +               /* Point fd at the gz version. */
> +               *st = gzst;
> +               close(*fd);
> +               *fd = gzfd;
> +               gzfd = -1;
> +       }
> +
> +       /*
> +        *  It's possible to reach this point with fd, brfd, and gzfd
> +        *  all open and valid; eg, if *.br and *.gz both exist, but
> +        *  are older than the uncompressed version.
> +        */
> +       if (brfd != -1) close(brfd);
> +       if (gzfd != -1) close(gzfd);
> +
> +       return (0);
>   }
> 
>