From: Kirill A. Korinsky <kirill@openbsd.org>
Subject: Re: httpd: add custom HTTP header support #2
To: Rafael Sadowski <rafael@sizeofvoid.org>
Cc: tech@openbsd.org
Date: Mon, 20 Jul 2026 14:13:36 +0200

On Mon, 20 Jul 2026 08:44:39 +0200,
Rafael Sadowski <rafael@sizeofvoid.org> wrote:
> 
> I've already sent this diff to kirill@, and here is the rebase following
> the latest commits. This diff fix small typos/minor issues and impl. the
> error path:
> 
> commit 0da57a19c29af6ea328366dec5f7c699ce763ce8
> Author: Rafael Sadowski <rafael@sizeofvoid.org>
> Date:   Thu Jul 16 15:53:03 2026 +0200
> 
>     add always option for header set and remove
> 
>     This adds support for the "always" keyword in header set and remove
>     directives. refactor the internal header generation to use a kvtree
>     so that custom headers are correctly included in the error/abort path.
> 
> That should cover everything. And now I'm looking for an OK.
> 
>

Two minor things.

1) I think it leaks on reload. server_purge() right now just removes
   &srv->srv_conf from srv_hosts, but doesn't free new headers.

> +int
> +header_name_forbidden(const char *name)
> +{
> +	if (strcasecmp(name, "Content-Length") == 0 ||
> +	    strcasecmp(name, "Transfer-Encoding") == 0 ||
> +	    strcasecmp(name, "Connection") == 0 ||
> +	    strcasecmp(name, "Date") == 0) {
> +		yyerror("header \"%s\" is reserved and cannot be used", name);
> +		return (1);
> +	}
> +
> +	if (strcasecmp(name, "Server") == 0) {
> +		yyerror("header \"Server\" cannot be configured here, "
> +		    "use 'no banner' instead");
> +		return (1);
> +	}
> +	return (0);
> +}
> +

2) Probably we should not allow an empty name here as well.

-- 
wbr, Kirill