Index | Thread | Search

From:
Fabien Romano <fabienromano@gmail.com>
Subject:
Re: httpd: do NOT send location fcgiparams twice
To:
Rafael Sadowski <rafael@sizeofvoid.org>, tech@openbsd.org
Date:
Mon, 13 Jul 2026 18:29:09 +0100

Download raw body.

Thread

On 7/13/26 12:55, Rafael Sadowski wrote:
> While working on the custom headers, I'm noticing more and more issues
> in fcgiparams (since I used that as a template).
> 
> server "fcgi-test" {
> 	listen on 127.0.0.1 port 8089
> 	root "/htdocs"
> 
> 	location "*.cgi" {
> 		fastcgi {
> 			socket "/run/slowcgi.sock"
> 			param FCGI_TEST_A "A"
> 			param FCGI_TEST_B "B"
> 			param FCGI_TEST_C "C"
> 			param FCGI_TEST_D "D"
> 			param FCGI_TEST_E "E"
> 		}
> 	}
> }
> 
> curl -s http://127.0.0.1:8089/env.cgi
> FCGI_TEST_E=E
> FCGI_TEST_D=D
> FCGI_TEST_C=C
> FCGI_TEST_B=B
> FCGI_TEST_A=A
> FCGI_TEST_E=E
> FCGI_TEST_D=D
> FCGI_TEST_C=C
> FCGI_TEST_B=B
> FCGI_TEST_A=A
> 
> Yes, the order is reversed. we can fix it in a different commit.
> 
> OK?

config_setserver_fcgiparams() always sends imsg to PROC_SERVER:
	if (proc_composev(ps, PROC_SERVER, IMSG_CFG_FCGI, iov, c) != 0)

But is called inside proc loop:
	for (id = 0; id < PROC_MAX; id++) {
...
		if (id == PROC_SERVER &&
		    (srv->srv_conf.flags & SRVFLAG_LOCATION) == 0) {
...
			/* Configure TLS if necessary. */
			config_setserver_tls(env, srv);
		} else {
...
			/* Configure FCGI parameters if necessary. */
			config_setserver_fcgiparams(env, srv);
		}
	}

The same applies to config_setserver_tls() > config_settls():
	if (proc_composev(ps, PROC_SERVER, IMSG_CFG_TLS, iov, c) != 0)

Thus, I think the correct fix is to move those two outside the loop.
And while there, check the return value.

> commit 603108536a8ef506d76179d9d4007d1038b50eee
> Author: Rafael Sadowski <rafael@sizeofvoid.org>
> Date:   Mon Jul 13 13:33:34 2026 +0200
> 
>     httpd: do NOT send location fcgiparams twice
>     
>     Only emit the params when the current receiver is the server child.
>     
>     When sending a location-config from the parent, the FCGI params IMSG
>     was emitted once per receiver iteration and always targeted the server
>     child.  With both the server and the logger process needing the server
>     config, the server child received IMSG_CFG_FCGI twice per location and
>     appended the same params on top of the existing list. The FastCGI backends
>     then saw every param duplicated.
> 
> diff --git a/config.c b/config.c
> index 146ed2c..6522a4b 100644
> --- a/config.c
> +++ b/config.c
> @@ -281,7 +281,8 @@ config_setserver(struct httpd *env, struct server *srv)
>  			}
>  
>  			/* Configure FCGI parameters if necessary. */
> -			config_setserver_fcgiparams(env, srv);
> +			if (id == PROC_SERVER)
> +				config_setserver_fcgiparams(env, srv);
>  		}
>  	}
>  
> 

Index: config.c
===================================================================
RCS file: /mnt/ext/cvs/src/usr.sbin/httpd/config.c,v
diff -u -p -r1.72 config.c
--- config.c	17 May 2026 10:56:41 -0000	1.72
+++ config.c	13 Jul 2026 17:23:18 -0000
@@ -268,9 +268,6 @@ config_setserver(struct httpd *env, stru
 					return (-1);
 				}
 			}
-
-			/* Configure TLS if necessary. */
-			config_setserver_tls(env, srv);
 		} else {
 			if (proc_composev(ps, id, IMSG_CFG_SERVER,
 			    iov, c) != 0) {
@@ -279,10 +276,17 @@ config_setserver(struct httpd *env, stru
 				    __func__, srv->srv_conf.name);
 				return (-1);
 			}
-
-			/* Configure FCGI parameters if necessary. */
-			config_setserver_fcgiparams(env, srv);
 		}
+	}
+
+	if ((srv->srv_conf.flags & SRVFLAG_LOCATION) == 0) {
+		/* Configure TLS if necessary. */
+		if (config_setserver_tls(env, srv) != 0)
+			return (-1);
+	} else {
+		/* Configure FCGI parameters if necessary. */
+		if (config_setserver_fcgiparams(env, srv) != 0)
+			return (-1);
 	}

 	/* Close server socket early to prevent fd exhaustion in the parent. */