Re: httpd: don't send the fastcgi param struct over imsg and tweaks

On Fri, 17 Jul 2026 22:02:13 +0200,
Rafael Sadowski <[email protected]> wrote:
> 
>

Mostly good, a few minor findings:

> -	if (proc_composev(ps, PROC_SERVER, IMSG_CFG_FCGI, iov, c) != 0) {
> -		log_warn("%s: failed to compose IMSG_CFG_FCGI imsg for "
> -		    "`%s'", __func__, srv_conf->name);
> -		free(iov);
> -		return (-1);
> +		fpmsg.id = srv_conf->id;
> +		fpmsg.namelen = strlen(fp->name);
> +		fpmsg.vallen = strlen(fp->value);
> +
> +		iov[0].iov_base = &fpmsg;
> +		iov[0].iov_len = sizeof(fpmsg);
> +		iov[1].iov_base = fp->name;
> +		iov[1].iov_len = fpmsg.namelen;
> +		iov[2].iov_base = fp->value;
> +		iov[2].iov_len = fpmsg.vallen;
> +
> +		if (proc_composev(ps, PROC_SERVER, IMSG_CFG_FCGI, iov, 3)
> +		    != 0) {
> +			log_warn("%s: failed to compose IMSG_CFG_FCGI "
> +			    "for `%s'", __func__, srv_conf->name);
> +			return (-1);
> +		}

Here tricky part: if not first proc_composev() call fails, early messages
are alread commited.

Before that refactoring here was none or all contract, now it none,
something or all.

> @@ -813,29 +814,33 @@ fcgiflags	: SOCKET STRING {
>  			if ((param = calloc(1, sizeof(*param))) == NULL)
>  				fatal("out of memory");
>  
> -			if (strlcpy(param->name, $2, sizeof(param->name)) >=
> -			    sizeof(param->name)) {
> -				yyerror("fastcgi_param name truncated");
> +			if (strlen($2) > HTTPD_FCGI_NAME_MAX - 1) {
> +				yyerror("fastcgi param name too long (max %d)",
> +				    HTTPD_FCGI_NAME_MAX - 1);
>  				free($2);
>  				free($3);
> -				free(param);
>  				YYERROR;
>  			}
> -			if (strlcpy(param->value, $3, sizeof(param->value)) >=
> -			    sizeof(param->value)) {
> -				yyerror("fastcgi_param value truncated");
> +
> +			if (strlen($3) > HTTPD_FCGI_VAL_MAX - 1) {
> +				yyerror("fastcgi param value too long (max %d)",
> +				    HTTPD_FCGI_VAL_MAX - 1);
>  				free($2);
>  				free($3);
> -				free(param);
>  				YYERROR;
>  			}
> +
> +			if ((param->name = strdup($2)) == NULL ||
> +			    (param->value = strdup($3)) == NULL)
> +				fatal("out of memory");
> +
>  			free($2);
>  			free($3);
>  
>  			DPRINTF("[%s,%s,%d]: adding param \"%s\" value \"%s\"",
>  			    srv_conf->location, srv_conf->name, srv_conf->id,
>  			    param->name, param->value);
> -			TAILQ_INSERT_HEAD(&srv_conf->fcgiparams, param, entry);
> +			TAILQ_INSERT_TAIL(&srv_conf->fcgiparams, param, entry);
>  		}
>  		| STRIP NUMBER			{
>  			if ($2 < 0 || $2 > INT_MAX) {

I think here you're leaking param on not sucessfull exit.

-- 
wbr, Kirill