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

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

[telephil9]

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.

===
diff -r f020e57da8d6 sys/src/cmd/webfs/fs.c
--- a/sys/src/cmd/webfs/fs.c	Thu Dec 17 20:26:38 2020 -0800
+++ b/sys/src/cmd/webfs/fs.c	Sun Jan 10 19:24:24 2021 +0100
@@ -42,6 +42,7 @@
 			Qctl,
 			Qbody,
 			Qpost,
+			Qheaders,
 			Qparsed,
 				Qurl,
 				Qurlschm,
@@ -63,6 +64,7 @@
 			"ctl",
 			"body",
 			"postbody",
+			"headers",
 			"parsed",
 				"url",
 				"scheme",
@@ -293,6 +295,14 @@
 		}
 	} 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;
+			}
 			if(nametab[i]){
 				if(strcmp(name, nametab[i]) == 0)
 					break;
@@ -479,8 +489,10 @@
 static void
 fsread(Req *r)
 {
-	char buf[1024];
+	char buf[2048];
 	Webfid *f;
+	Key *k;
+	int n;
 
 	f = r->fid->aux;
 	switch(f->level){
@@ -505,6 +517,17 @@
 	case Qctl:
 		snprint(buf, sizeof(buf), "%d\n", CLIENTID(f->client));
 		goto String;
+	case Qheaders:
+		n = 0;
+		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;
+			}
+		}
+		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

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

[ori]

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
> 

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

[ori]

Quoth ori@eigenstate.org:
> 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?
> 

One other thought that I just had: What if we picked
a few of the mandatory or very common headers (contenttype,
etc) and put them explicitly in the request dir, and
moved the rest to a less-mangled headers subdir?

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

[Lyndon Nerenberg]

> One other thought that I just had: What if we picked
> a few of the mandatory or very common headers (contenttype,
> etc) and put them explicitly in the request dir, and
> moved the rest to a less-mangled headers subdir?

If we're going to have a headers/... directory do we really need
to duplicate some of those files elsewhere?  For consistency's
sake I'd just keep 'em all in the one directory and call it a day.

--lyndon

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

[ori]

Quoth Lyndon Nerenberg <lyndon@orthanc.ca>:
> > One other thought that I just had: What if we picked
> > a few of the mandatory or very common headers (contenttype,
> > etc) and put them explicitly in the request dir, and
> > moved the rest to a less-mangled headers subdir?
> 
> If we're going to have a headers/... directory do we really need
> to duplicate some of those files elsewhere?  For consistency's
> sake I'd just keep 'em all in the one directory and call it a day.
> 
> --lyndon
> 

We don't need to -- but how many scripts and programs are we
willing to break, and how badly?

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

[Steve Simon]

I have a memory that webfs already creates header files for a limited set of headers: contenttype, contentencoding, length (from memory). why not just export headers we need so there can never be a clash?

-Steve

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

[Lyndon Nerenberg]

ori@eigenstate.org writes:

> We don't need to -- but how many scripts and programs are we
> willing to break, and how badly?

Well ... fixing things in the base is easy enough.  For the others,
bind(1) is a viable workaround until they get updated.

I don't like breaking things gratuitously, but I'll do it if it
means a cleaner/more consistent interface.

All just IMO of course.

--lyndon

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

[hiro]

i agree with lyndon, having two similar interfaces is even worse than
having none.
esp. for unimportant stuff like webfs, http servers will be much less
stable than webfs anyway.

On 1/10/21, Lyndon Nerenberg <lyndon@orthanc.ca> wrote:
> ori@eigenstate.org writes:
>
>> We don't need to -- but how many scripts and programs are we
>> willing to break, and how badly?
>
> Well ... fixing things in the base is easy enough.  For the others,
> bind(1) is a viable workaround until they get updated.
>
> I don't like breaking things gratuitously, but I'll do it if it
> means a cleaner/more consistent interface.
>
> All just IMO of course.
>
> --lyndon
>

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

[ori]

Quoth Steve Simon <steve@quintile.net>:
> I have a memory that webfs already creates header files for a limited set of headers: contenttype, contentencoding, length (from memory). why not just export headers we need so there can never be a clash?

How does webfs know which headers we need in advance? lots of software
adds magic or custom headers these days.

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

[cinap_lenrek]

it is *NOT* selective. the old webfs had only some headers exposed.
the 9fonrt webfs generalizes this, mangles the header name to fit
the old naming scheme by making it lowercase and remove dashes.
all the headers can be read.

theres no list in webfs that filters headers. they all appear
after we receive the headers and the open appears.

--
cinap