On Fri, Feb 25, 2022 at 07:47:38PM +0100, Alexander Bluhm wrote:
> On Fri, Feb 25, 2022 at 11:00:22AM +0100, prx wrote:
> > After a few months, I reupload the patch to enable httpd static
> > compression using "location {}" instructions.
> >
> > I use it without any issue on my own website and to serve
> > https://webzine.pufy.cafe.
> > Anyone else tried it?
>
> I just added it to bluhm.genua.de. There I deliver large html
> tables. For http://bluhm.genua.de/regress/results/regress.html it
> reduces bandwith by factor 20.
>
> > +.It Ic gzip_static
> > +Enable static gzip compression.
> > +.Pp
> > +When a file is requested, serves the file with .gz added to its path if it
> > exists.
>
> Mention for what this is useful.
>
> > +#define SERVER_DEFAULT_GZIP_STATIC 0
>
> A define for a default 0 is overkill.
>
> > + int gzip_static;
>
> Better use a flag than an int for a boolean.
>
> > + { "gzip_static", GZIPSTATIC },
>
> We do not have keywords with _ but one with -
>
> > + /* change path to path.gz if necessary. */
> > + if (srv_conf->gzip_static) {
> > + struct http_descriptor *req = clt->clt_descreq;
> > + struct http_descriptor *resp = clt->clt_descresp;
> > + struct stat gzst;
> > + struct kv *r, key;
> > + char gzpath[PATH_MAX];
> > +
> > + /* check Accept-Encoding header */
> > + key.kv_key = "Accept-Encoding";
> > + r = kv_find(&req->http_headers, &key);
> > +
> > + if (r != NULL) {
> > + if (strstr(r->kv_value, "gzip") != NULL) {
> > + /* append ".gz" to path and check existence */
> > + strlcpy(gzpath, path, sizeof(gzpath));
> > + strlcat(gzpath, ".gz", sizeof(gzpath));
> > +
> > + if ((access(gzpath, R_OK) == 0) &&
> > + (stat(gzpath, &gzst) == 0)) {
> > + path = gzpath;
> > + st = &gzst;
>
> Outside of a block you must not use pointer to variables that are
> defined inside a block.
>
> > + kv_add(&resp->http_headers,
> > + "Content-Encoding", "gzip");
> > + }
> > + }
> > + }
> > + }
> > +
> > /* Now open the file, should be readable or we have another problem */
> > if ((fd = open(path, O_RDONLY)) == -1)
> > goto abort;
>
> With all that fixed, diff looks like this and works fine in my setup.
>
> ok?
>
> bluhm
>
> Index: httpd.conf.5
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/httpd/httpd.conf.5,v
> retrieving revision 1.119
> diff -u -p -r1.119 httpd.conf.5
> --- httpd.conf.5 24 Oct 2021 16:01:04 -0000 1.119
> +++ httpd.conf.5 25 Feb 2022 18:41:42 -0000
> @@ -425,6 +425,12 @@ A variable that is set to a comma separa
> features in use
> .Pq omitted when TLS client verification is not in use .
> .El
> +.It Ic gzip-static
> +Enable static gzip compression to save bandwith.
> +.Pp
> +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 hsts Oo Ar option Oc
> Enable HTTP Strict Transport Security.
> Valid options are:
> Index: httpd.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/httpd/httpd.h,v
> retrieving revision 1.158
> diff -u -p -r1.158 httpd.h
> --- httpd.h 24 Oct 2021 16:01:04 -0000 1.158
> +++ httpd.h 25 Feb 2022 18:40:58 -0000
> @@ -396,6 +396,7 @@ SPLAY_HEAD(client_tree, client);
> #define SRVFLAG_DEFAULT_TYPE 0x00800000
> #define SRVFLAG_PATH_REWRITE 0x01000000
> #define SRVFLAG_NO_PATH_REWRITE 0x02000000
> +#define SRVFLAG_GZIP_STATIC 0x04000000
> #define SRVFLAG_LOCATION_FOUND 0x40000000
> #define SRVFLAG_LOCATION_NOT_FOUND 0x80000000
>
> Index: parse.y
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.127
> diff -u -p -r1.127 parse.y
> --- parse.y 24 Oct 2021 16:01:04 -0000 1.127
> +++ parse.y 25 Feb 2022 18:24:30 -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
> +%token ERRDOCS GZIPSTATIC
> %token <v.string> STRING
> %token <v.number> NUMBER
> %type <v.port> port
> @@ -553,6 +553,7 @@ serveroptsl : LISTEN ON STRING opttls po
> | logformat
> | fastcgi
> | authenticate
> + | gzip_static
> | filter
> | LOCATION optfound optmatch STRING {
> struct server *s;
> @@ -1217,6 +1218,14 @@ fcgiport : NUMBER {
> }
> ;
>
> +gzip_static : NO GZIPSTATIC {
> + srv->srv_conf.flags &= ~SRVFLAG_GZIP_STATIC;
> + }
> + | GZIPSTATIC {
> + srv->srv_conf.flags |= SRVFLAG_GZIP_STATIC;
> + }
> + ;
> +
> tcpip : TCP '{' optnl tcpflags_l '}'
> | TCP tcpflags
> ;
> @@ -1441,6 +1450,7 @@ lookup(char *s)
> { "fastcgi", FCGI },
> { "forwarded", FORWARDED },
> { "found", FOUND },
> + { "gzip-static", GZIPSTATIC },
> { "hsts", HSTS },
> { "include", INCLUDE },
> { "index", INDEX },
> Index: server_file.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 server_file.c
> --- server_file.c 29 Apr 2021 18:23:07 -0000 1.70
> +++ server_file.c 25 Feb 2022 18:26:11 -0000
> @@ -223,26 +223,53 @@ server_file_request(struct httpd *env, s
> const char *errstr = NULL;
> int fd = -1, ret, code = 500;
> size_t bufsiz;
> + struct stat gzst;
> + char gzpath[PATH_MAX];
>
> if ((ret = server_file_method(clt)) != 0) {
> code = ret;
> goto abort;
> }
>
> + media = media_find_config(env, srv_conf, path);
> +
> if ((ret = server_file_modified_since(clt->clt_descreq, st)) != -1) {
> /* send the header without a body */
> - media = media_find_config(env, srv_conf, path);
> if ((ret = server_response_http(clt, ret, media, -1,
> MINIMUM(time(NULL), st->st_mtim.tv_sec))) == -1)
> goto fail;
> goto done;
> }
>
> + /* 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 kv *r, key;
> +
> + /* 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 */
> + strlcpy(gzpath, path, sizeof(gzpath));
> + strlcat(gzpath, ".gz", sizeof(gzpath));
> +
Shouldn't we check for truncation on strlcpy and strlcat and goto fail
in that event?
Other than that it looks ok to me.
> + if ((access(gzpath, R_OK) == 0) &&
> + (stat(gzpath, &gzst) == 0)) {
> + path = gzpath;
> + st = &gzst;
> + kv_add(&resp->http_headers,
> + "Content-Encoding", "gzip");
> + }
> + }
> + }
> +
> /* Now open the file, should be readable or we have another problem */
> if ((fd = open(path, O_RDONLY)) == -1)
> goto abort;
>
> - media = media_find_config(env, srv_conf, path);
> ret = server_response_http(clt, 200, media, st->st_size,
> MINIMUM(time(NULL), st->st_mtim.tv_sec));
> switch (ret) {
>
--
Tracey Emery
