Re: [9front] [PATCH] webfs: add 'headers' file

[ori@eigenstate.org]

Quoth telephil9@gmail.com:
> Hi,
> 
> Following patch adds a 'headers' file containing the full list of response headers 
> along with their values (each line is Header: value).
> I need this for netsurf which expects response headers prior to processing a request, and the
> current webfs mechanism is not really convenient to work with in this case.

For future patches, writing up what we do and why it should
change would be appreciated.

For this patch: t the moment, the header names are transformed,
and there's no direct mapping from the string to the header
name. Our current mangling and organization leads to a couple
of problems:

1. We can map multiple headers onto the same value:
	foo-bar: baz
	foobar: baz

2. We can collide with webfs files, because headers
   aren't in a subdirectory:

		ctl: foo

	will try to read the ctl file instead of the header,
	which seems like something we don't want.

as a result of 2, there's also no clean way to enumerate
all the headers.

I think netsurf *can* just do the same transform as webfs.

	static char*
	fshdrname(char *s)
	{
		char *k, *w;
	
		for(k=w=s; *k; k++)
			if(isalnum(*k))
				*w++ = tolower(*k);
		*w = 0;
		return s;
	}

and that will work fine, but we'd need to blacklist header
names to avoid collisions.

Looking at the http spec, we can strip fewer characters;
everything in the 'token' type is unproblematic in  9p name,
so checking for a valid token and lowercasing it should
suffice.

For collisions: I don't know if we need to solve it, but
if we do -- I think the better option may be putting
headers into a 'header' subdir, similar to 'parsed'.

The problem there is compatibility: it's going to be a
pain to transition to a more usable scheme. So maybe we
live with the wart, or present both ways?


>  	} else {
>  		for(i=f->level+1; i < nelem(nametab); i++){
> +			if(i == Qheaders && f->client && f->client->qbody){
> +				Key *k;
> +
> +				for(k = f->client->qbody->hdr; k; k = k->next){
> +					f->key = addkey(f->key, k->key, k->val);
> +				}
> +				break;
> +			}

I think Qheaders can reuse the same list as Qheader, no need
to copy it a second time.


> +		for(k = f->key; k != nil; k = k->next){
> +			n += snprint(buf+n, sizeof(buf)-n, "%s: %s\n", k->key, k->val);
> +			if(n >= sizeof(buf)){
> +				n = sizeof(buf)-1;
> +				break;
> +			}

seprint is likely cleaner here, and snprint (unlike
snprintf) does not return more than the number of
bytes it was asked to write.

Also, the buffer here is 1024 characters; it's likely
that we're going to run out with even a modest number
of headers to format. If we go with this design, this
buffer should probably be allocated (and cached, since
we don't want to regenerate it on every Tread; that's
likely a bit expensive, and once we have the body, it
never changes)

finally, we should lowercase the header names, since
they're required to be case insensitive -- this will
make it easier for readers to use them.

> +		}
> +		buf[n] = 0;
> +		goto String;
>  	case Qheader:
>  		snprint(buf, sizeof(buf), "%s", f->key->val);
>  		goto String;
> @@ -703,6 +726,7 @@
>  fsdestroyfid(Fid *fid)
>  {
>  	Webfid *f;
> +	Key *k, *d;
>  
>  	if(f = fid->aux){
>  		fid->aux = nil;
> @@ -714,8 +738,13 @@
>  			}
>  			bufree(f->buq);
>  		}
> -		if(f->key)
> -			free(f->key);
> +		if(f->key){
> +			for(k = f->key; k != nil;){
> +				d = k;
> +				k = k->next;
> +				free(d);
> +			}
> +		}
>  		freeclient(f->client);
>  		free(f);
>  	}
> ===
> 
> Man page:
> ===
> diff -r f020e57da8d6 sys/man/4/webfs
> --- a/sys/man/4/webfs	Thu Dec 17 20:26:38 2020 -0800
> +++ b/sys/man/4/webfs	Sun Jan 10 19:30:48 2021 +0100
> @@ -165,6 +165,11 @@
>  .B parsed
>  directory will change to the final destination.
>  .PP
> +The
> +.B headers
> +file gives the full list of headers along with their values,
> +one per line.
> +.PP
>  The resulting data may be read from
>  .B body
>  as it arrives.
> ===
> 
> --phil
>