9front - general discussion about 9front
 help / color / mirror / Atom feed
* [9front] [PATCH] webfs: add 'headers' file
@ 2021-01-10 18:32 telephil9
  2021-01-10 19:28 ` ori
  0 siblings, 1 reply; 10+ messages in thread
From: telephil9 @ 2021-01-10 18:32 UTC (permalink / raw)
  To: 9front

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [9front] [PATCH] webfs: add 'headers' file
  2021-01-10 18:32 [9front] [PATCH] webfs: add 'headers' file telephil9
@ 2021-01-10 19:28 ` ori
  2021-01-10 19:51   ` ori
  0 siblings, 1 reply; 10+ messages in thread
From: ori @ 2021-01-10 19:28 UTC (permalink / raw)
  To: 9front

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
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [9front] [PATCH] webfs: add 'headers' file
  2021-01-10 19:28 ` ori
@ 2021-01-10 19:51   ` ori
  2021-01-10 20:30     ` Lyndon Nerenberg
  0 siblings, 1 reply; 10+ messages in thread
From: ori @ 2021-01-10 19:51 UTC (permalink / raw)
  To: 9front

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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [9front] [PATCH] webfs: add 'headers' file
  2021-01-10 19:51   ` ori
@ 2021-01-10 20:30     ` Lyndon Nerenberg
  2021-01-10 21:11       ` ori
  2021-01-10 21:16       ` Steve Simon
  0 siblings, 2 replies; 10+ messages in thread
From: Lyndon Nerenberg @ 2021-01-10 20:30 UTC (permalink / raw)
  To: 9front

> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [9front] [PATCH] webfs: add 'headers' file
  2021-01-10 20:30     ` Lyndon Nerenberg
@ 2021-01-10 21:11       ` ori
  2021-01-10 22:29         ` Lyndon Nerenberg
  2021-01-10 21:16       ` Steve Simon
  1 sibling, 1 reply; 10+ messages in thread
From: ori @ 2021-01-10 21:11 UTC (permalink / raw)
  To: 9front

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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [9front] [PATCH] webfs: add 'headers' file
  2021-01-10 20:30     ` Lyndon Nerenberg
  2021-01-10 21:11       ` ori
@ 2021-01-10 21:16       ` Steve Simon
  2021-01-11 17:13         ` ori
  1 sibling, 1 reply; 10+ messages in thread
From: Steve Simon @ 2021-01-10 21:16 UTC (permalink / raw)
  To: 9front

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [9front] [PATCH] webfs: add 'headers' file
  2021-01-10 21:11       ` ori
@ 2021-01-10 22:29         ` Lyndon Nerenberg
  2021-01-11  0:50           ` hiro
  0 siblings, 1 reply; 10+ messages in thread
From: Lyndon Nerenberg @ 2021-01-10 22:29 UTC (permalink / raw)
  To: 9front

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [9front] [PATCH] webfs: add 'headers' file
  2021-01-10 22:29         ` Lyndon Nerenberg
@ 2021-01-11  0:50           ` hiro
  0 siblings, 0 replies; 10+ messages in thread
From: hiro @ 2021-01-11  0:50 UTC (permalink / raw)
  To: 9front

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
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [9front] [PATCH] webfs: add 'headers' file
  2021-01-10 21:16       ` Steve Simon
@ 2021-01-11 17:13         ` ori
  2021-01-11 21:53           ` cinap_lenrek
  0 siblings, 1 reply; 10+ messages in thread
From: ori @ 2021-01-11 17:13 UTC (permalink / raw)
  To: 9front

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [9front] [PATCH] webfs: add 'headers' file
  2021-01-11 17:13         ` ori
@ 2021-01-11 21:53           ` cinap_lenrek
  0 siblings, 0 replies; 10+ messages in thread
From: cinap_lenrek @ 2021-01-11 21:53 UTC (permalink / raw)
  To: 9front

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-01-11 22:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-10 18:32 [9front] [PATCH] webfs: add 'headers' file telephil9
2021-01-10 19:28 ` ori
2021-01-10 19:51   ` ori
2021-01-10 20:30     ` Lyndon Nerenberg
2021-01-10 21:11       ` ori
2021-01-10 22:29         ` Lyndon Nerenberg
2021-01-11  0:50           ` hiro
2021-01-10 21:16       ` Steve Simon
2021-01-11 17:13         ` ori
2021-01-11 21:53           ` cinap_lenrek

9front - general discussion about 9front

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/9front

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 9front 9front/ http://inbox.vuxu.org/9front \
		9front@9front.org
	public-inbox-index 9front

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.9front


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git