From: Florian Obser <florian@openbsd.org>
Subject: Re: slowcgi: preserve PARAMS order in the CGI environment
To: Rafael Sadowski <rafael@sizeofvoid.org>
Cc: tech@openbsd.org
Date: Mon, 13 Jul 2026 19:25:12 +0200

On 2026-07-13 16:50 +02, Rafael Sadowski <rafael@sizeofvoid.org> wrote:
> I fixed the order in httpd ("tail insert during  read from conf"
> followed by "tail insert" during copying) but still had an incorrect
> order because of slowcgi.

Out of idle curiosity, why is this important?

slist_init(3) has this:

     Singly-linked lists are the simplest of the five data structures and
     support only the above functionality.  Singly-linked lists are ideal for
     applications with large datasets and few or no removals, or for
     implementing a LIFO queue.

     Simple queues and singly-linked tail queues add the following
     functionality:

           1.   Entries can be added at the end of a list.

     However:

           1.   All list insertions must specify the head of the list.
           2.   Each head entry requires two pointers rather than one.
           3.   Code size is about 15% greater and operations run about 20%
                slower than singly-linked lists.

I'm not opposed to the change (it reads OK), but it does not come for
free. Btw. there is a copy of slowcgi.c in usr.sbin/bgplgd...


>
> env vars were stored on an SLIST via SLIST_INSERT_HEAD and later walked
> with SLIST_FOREACH when populating envp[]. Prepending on insert and
> iterating head-to-tail reverses the list, so FastCGI PARAMS delivered as
>
> curl -s http://127.0.0.1:8089/env.cgi
> FCGI_TEST_BB=BB
> FCGI_TEST_E=E
> FCGI_TEST_D=D
> FCGI_TEST_C=C
> FCGI_TEST_B=B
> FCGI_TEST_A=A
>
> even httpd provided them in the correct order:
> ...
> fcgi_add_param: HTTP_HOST[9] => 127.0.0.1:8089[14], total_len: 164
> fcgi_add_param: HTTP_USER_AGENT[15] => curl/8.21.0[11], total_len: 189
> fcgi_add_param: FCGI_TEST_A[11] => A[1], total_len: 217
> fcgi_add_param: FCGI_TEST_B[11] => B[1], total_len: 231
> fcgi_add_param: FCGI_TEST_C[11] => C[1], total_len: 245
> fcgi_add_param: FCGI_TEST_D[11] => D[1], total_len: 259
> fcgi_add_param: FCGI_TEST_E[11] => E[1], total_len: 273
> fcgi_add_param: FCGI_TEST_BB[12] => BB[2], total_len: 287
> fcgi_add_param: REMOTE_ADDR[11] => 127.0.0.1[9], total_len: 303
> fcgi_add_param: REMOTE_PORT[11] => 24112[5], total_len: 325
> ...
>
> Switch the list to STAILQ and insert at the tail so envp[] follows
> the order in which parameters arrived.
>
> OK?
>
> diff --git a/usr.sbin/slowcgi/slowcgi.c b/usr.sbin/slowcgi/slowcgi.c
> index 1663118ee22..b0e511e095a 100644
> --- a/usr.sbin/slowcgi/slowcgi.c
> +++ b/usr.sbin/slowcgi/slowcgi.c
> @@ -83,10 +83,10 @@ struct listener {
>  };
>  
>  struct env_val {
> -	SLIST_ENTRY(env_val)	 entry;
> +	STAILQ_ENTRY(env_val)	 entry;
>  	char			*val;
>  };
> -SLIST_HEAD(env_head, env_val);
> +STAILQ_HEAD(env_head, env_val);
>  
>  struct fcgi_record_header {
>  	uint8_t		version;
> @@ -693,7 +693,7 @@ parse_begin_request(uint8_t *buf, uint16_t n, struct request *c, uint16_t id)
>  	c->request_started = 1;
>  
>  	c->id = id;
> -	SLIST_INIT(&c->env);
> +	STAILQ_INIT(&c->env);
>  	c->env_count = 0;
>  }
>  
> @@ -794,7 +794,7 @@ parse_params(uint8_t *buf, uint16_t n, struct request *c, uint16_t id)
>  		buf += val_len;
>  		n -= val_len;
>  
> -		SLIST_INSERT_HEAD(&c->env, env_entry, entry);
> +		STAILQ_INSERT_TAIL(&c->env, env_entry, entry);
>  		ldebug("env[%d], %s", c->env_count, env_entry->val);
>  		c->env_count++;
>  	}
> @@ -955,7 +955,7 @@ exec_cgi(struct request *c)
>  		argv[1] = NULL;
>  		if ((env = calloc(c->env_count + 1, sizeof(char*))) == NULL)
>  			_exit(1);
> -		SLIST_FOREACH(env_entry, &c->env, entry)
> +		STAILQ_FOREACH(env_entry, &c->env, entry)
>  			env[i++] = env_entry->val;
>  		env[i++] = NULL;
>  		execve(c->script_name, argv, env);
> @@ -1153,9 +1153,9 @@ cleanup_request(struct request *c)
>  		event_del(&c->script_stdin_ev);
>  	}
>  	close(c->fd);
> -	while (!SLIST_EMPTY(&c->env)) {
> -		env_entry = SLIST_FIRST(&c->env);
> -		SLIST_REMOVE_HEAD(&c->env, entry);
> +	while (!STAILQ_EMPTY(&c->env)) {
> +		env_entry = STAILQ_FIRST(&c->env);
> +		STAILQ_REMOVE_HEAD(&c->env, entry);
>  		free(env_entry->val);
>  		free(env_entry);
>  	}
>

-- 
In my defence, I have been left unsupervised.