[9front] [PATCH] Update hpost to use -r header like hget

[9front] [PATCH] Update hpost to use -r header like hget

[Julien Blanchard]

Hello,
This is a small update to hpost which allows the use of a -r header 
option just like in hget. I had the need when trying to push a file to 
an API which also needed an authorization header.
It's the first time I write some not trivial rc script so maybe there is 
a better way to add this option. After quite some testing it looks like 
it works at least :)

Cheers,
julienxx



diff -r ac35c963f8e2 rc/bin/hpost
--- a/rc/bin/hpost	Sun Dec 13 20:23:03 2020 +0100
+++ b/rc/bin/hpost	Tue Dec 15 17:45:26 2020 +0000
@@ -1,12 +1,13 @@
  #!/bin/rc
  rfork e
  url=()
+header=()
  at=()	# text fields
  af=()	# file fields
  l=()

  fn usage {
-	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -u ] url [ field:value | 
field@file ... ]' >[1=2]
+	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -r ] header [ -u ] url [ 
field:value | field@file ... ]' >[1=2]
  	exit usage
  }

@@ -14,6 +15,7 @@
  	switch($1){
  	case -l;	l=($l $1)
  	case -u;	shift; url=$1
+	case -r;	shift; header=$1
  	case -g;	shift; action=$1; method=mget
  	case -p;	shift; action=$1; method=mpost
  	case -m;	shift; action=$1; method=multi
@@ -95,7 +97,10 @@
  }

  if(! ~ $action ''){
-	hget=(hget $l)
+	if(~ $header '')
+		hget=(hget $l)
+	if not
+		hget=(hget -r $header $l)
  	$method
  	exit
  }
diff -r ac35c963f8e2 sys/man/1/hget
--- a/sys/man/1/hget	Sun Dec 13 20:23:03 2020 +0100
+++ b/sys/man/1/hget	Tue Dec 15 17:45:26 2020 +0000
@@ -38,6 +38,9 @@
  .B -m
  .I action
  ] [
+.B -r
+.I header
+] [
  .B -u
  ]
  .I url

Re: [9front] [PATCH] Update hpost to use -r header like hget

[ori]

Quoth Julien Blanchard <julien@typed-hole.org>:
> Hello,
> This is a small update to hpost which allows the use of a -r header 
> option just like in hget. I had the need when trying to push a file to 
> an API which also needed an authorization header.
> It's the first time I write some not trivial rc script so maybe there is 
> a better way to add this option. After quite some testing it looks like 
> it works at least :)

Thanks for the patch. just a few revisions:

> 
> Cheers,
> julienxx
> 
> 
> 
> diff -r ac35c963f8e2 rc/bin/hpost
> --- a/rc/bin/hpost	Sun Dec 13 20:23:03 2020 +0100
> +++ b/rc/bin/hpost	Tue Dec 15 17:45:26 2020 +0000
> @@ -1,12 +1,13 @@
>   #!/bin/rc
>   rfork e
>   url=()
> +header=()

unrelated to your change, but while we're here:
if you could also set action=()/method=(), that
would prevent them from leaking in through the
environment.

>   at=()	# text fields
>   af=()	# file fields
>   l=()
> 
>   fn usage {
> -	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -u ] url [ field:value | 
> field@file ... ]' >[1=2]
> +	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -r ] header [ -u ] url [ 
> field:value | field@file ... ]' >[1=2]
>   	exit usage
>   }
> 
> @@ -14,6 +15,7 @@
>   	switch($1){
>   	case -l;	l=($l $1)
>   	case -u;	shift; url=$1
> +	case -r;	shift; header=$1

This only supports one header. hget allows repeated
-r flags to add multiple headers:

	hget -r foo -r bar

The change for multiple headers should be as simple
as one change here:

	headers=($headers $1)

> +	if not
> +		hget=(hget -r $header $l)


And one here here:

	-r^$headers

which will distribute -r across all of the
entries in the headers list.

As a reminder: in rc,

	foo_^(x y z)

expands to:

	foo_x foo_y foo_z

Re: [9front] [PATCH] Update hpost to use -r header like hget

[Julien Blanchard]

On 15/12/2020 19:42, ori@eigenstate.org wrote:
> 
> unrelated to your change, but while we're here:
> if you could also set action=()/method=(), that
> would prevent them from leaking in through the
> environment.
> 

Done

> 
> This only supports one header. hget allows repeated
> -r flags to add multiple headers:
> 
> 	hget -r foo -r bar
> 
> The change for multiple headers should be as simple
> as one change here:
> 
> 	headers=($headers $1)
> 
>> +	if not
>> +		hget=(hget -r $header $l)
> 
> 
> And one here here:
> 
> 	-r^$headers
> 
> which will distribute -r across all of the
> entries in the headers list.
> 
> As a reminder: in rc,
> 
> 	foo_^(x y z)
> 
> expands to:
> 
> 	foo_x foo_y foo_z
> 

I had issues with just expanding the headers using -r^headers. It 
generates an hget command like (just an echo before line 98):

hget -rX-Foo: Lol -rAuthorization: Bearer Token -r Content-Type: 
multipart/form-data; boundary=HJBOUNDARY -b https://httpbin.org -P /post

where a valid hget command would be:
hget -r 'X-Foo: Lol' -r 'Authorization: Bearer Token' -r 'Content-Type: 
multipart/form-data; boundary=HJBOUNDARY' -b https://httpbin.org -P /post

so I updated with a for loop but maybe there is a better way?

Thanks for the feedback and the tips!


diff -r ac35c963f8e2 rc/bin/hpost
--- a/rc/bin/hpost	Sun Dec 13 20:23:03 2020 +0100
+++ b/rc/bin/hpost	Wed Dec 16 17:57:47 2020 +0000
@@ -1,12 +1,15 @@
  #!/bin/rc
  rfork e
  url=()
+headers=()
+action=()
+method=()
  at=()	# text fields
  af=()	# file fields
  l=()

  fn usage {
-	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -u ] url [ field:value | 
field@file ... ]' >[1=2]
+	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -r ] headers [ -u ] url 
[ field:value | field@file ... ]' >[1=2]
  	exit usage
  }

@@ -14,6 +17,7 @@
  	switch($1){
  	case -l;	l=($l $1)
  	case -u;	shift; url=$1
+	case -r;	shift; headers=($headers $1)
  	case -g;	shift; action=$1; method=mget
  	case -p;	shift; action=$1; method=mpost
  	case -m;	shift; action=$1; method=multi
@@ -95,7 +99,14 @@
  }

  if(! ~ $action ''){
-	hget=(hget $l)
+	if(~ $headers '')
+		hget=(hget $l)
+	if not
+		h=()
+		for(i in $headers)
+			h=($h -r $i)
+		hget=(hget $h $l)
+
  	$method
  	exit
  }
diff -r ac35c963f8e2 sys/man/1/hget
--- a/sys/man/1/hget	Sun Dec 13 20:23:03 2020 +0100
+++ b/sys/man/1/hget	Wed Dec 16 17:57:47 2020 +0000
@@ -38,6 +38,9 @@
  .B -m
  .I action
  ] [
+.B -r
+.I header
+] [
  .B -u
  ]
  .I url

Re: [9front] [PATCH] Update hpost to use -r header like hget

[sirjofri]

Hello,

I don't know how exactly rc parses arguments when it invokes another 
command, maybe try it with this concatenation?

'-r '^$headers

Notice the space and the quotes that make it effectively a string.

sirjofri

Re: [9front] [PATCH] Update hpost to use -r header like hget

[ori]

Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:
> Hello,
> 
> I don't know how exactly rc parses arguments when it invokes another 
> command, maybe try it with this concatenation?
> 
> '-r '^$headers
> 
> Notice the space and the quotes that make it effectively a string.
> 
> sirjofri
> 

won't work. rc doesn't re-expand arguments; it more or less
only splits words when evaluating `{}, so '-r foo' ends up
as one unit when you do the concatenation:

	cpu% h='-r '^('some header' 'another header')
	cpu% echo $#h
	2
	echo $h(1)
	-r some header

this is the big advantage of rc over unix sh, where you
"have" "to" "quote" "anything" "that" "can" "have"
"spaces", because the shell just reexpands it.

Re: [9front] [PATCH] Update hpost to use -r header like hget

[sirjofri]

16.12.2020 20:50:31 ori@eigenstate.org:
> Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:
>> '-r '^$headers
>
> won't work. rc doesn't re-expand arguments; it more or less
> only splits words when evaluating `{}, so '-r foo' ends up
> as one unit when you do the concatenation: [...]

That's indeed very interesting. I noticed that when playing around with 
it. I also noticed that it worked when encapsulating the whole thing in 
another `{}, but then you probably have issues with quoted arguments with 
spaces ('hello: world').

I think what I did was like this, but I don't know if that makes any 
sense:

h='-r<tab>'^$headers
u=`‘<tab>'{ifs='<tab>'; echo -n $h}

The whole thing is indeed an interesting challenge, especially because rc 
is handling items and lists so special. Spaces don't necessarily separate 
strings, items with spaces are not always quoted, etc. For example, the 
visual string "this is a string" is not always a list with 3 strings. It 
could also be a list with two or only one string.

Btw I really enjoyed your interview I listened to yesterday.

sirjofri

(Sorry ori for double posting this to you. It was meant for the list)

Re: [9front] [PATCH] Update hpost to use -r header like hget

[ori]

Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:
> 
> The whole thing is indeed an interesting challenge, especially because rc 
> is handling items and lists so special. Spaces don't necessarily separate 
> strings, items with spaces are not always quoted, etc. For example, the 
> visual string "this is a string" is not always a list with 3 strings. It 
> could also be a list with two or only one string.

there's no sometimes about it, and there aren't any funny rules.

rc never requotes something behind your back; it parses it once,
and keeps it parsed. evaluating the output of `{} is the place
where it parses it.

All other times, it stays split.

> "this is a string" is not always a list with 3 strings.

no, that's always a string with 4 strings:

	'"this'
	'is'
	'a'
	'string"'

Note that " is not a special character, so it doesn't
affect string splitting. I suspect you meant

	'this is a string'

which is *always* a single string to rc.

Re: [9front] [PATCH] Update hpost to use -r header like hget

[sirjofri]

There were several misunderstandings:

16.12.2020 22:52:11 ori@eigenstate.org:
> Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:
>>
>> The whole thing is indeed an interesting challenge, especially because 
rc
>> is handling items and lists so special. Spaces don't necessarily 
separate
>> strings, items with spaces are not always quoted, etc. For example, 
the
>> visual string "this is a string" is not always a list with 3 strings. 
It
>> could also be a list with two or only one string.
>
> there's no sometimes about it, and there aren't any funny rules.
>
> rc never requotes something behind your back; it parses it once,
> and keeps it parsed. evaluating the output of `{} is the place
> where it parses it.
>
> All other times, it stays split.
>
>> "this is a string" is not always a list with 3 strings.

I used "" to quote it in the english text, not for rc. I maybe should 
have pointed that out.

> no, that's always a string with 4 strings:
>
>   '"this'
>   'is'
>   'a'
>   'string"'

Sure. Four. I should learn counting again...

> Note that " is not a special character, so it doesn't
> affect string splitting. I suspect you meant
>
>   'this is a string'
>
> which is *always* a single string to rc.

Note that I was talking about the 'visual' appearance, like, what you get 
when echoing the list/variable or 'what you see on the screen'. When you 
see the string "this is a string" (without the '"') on the screen inside 
your window as an output there's no way to know how long the list is.

I guess even when capturing the output with `{...} it is still impossible 
to know that, because then rc is reevaluating the string into a list.

Re: [9front] [PATCH] Update hpost to use -r header like hget

[Steve Simon]

maybe we all know this but incase anyone missed it,  there is the “ modifier to variable expansion.

a=(1 2 3 4)

echo $a  -> 4 args, each if 1 char

echo $”a  -> 1 arg, containing a string of 7 chars

-Steve

Re: [9front] [PATCH] Update hpost to use -r header like hget

[Julien Blanchard]

Thanks a lot everyone for the explanations! It allowed me to quite 
simplify the patch.


diff -r ac35c963f8e2 rc/bin/hpost
--- a/rc/bin/hpost	Sun Dec 13 20:23:03 2020 +0100
+++ b/rc/bin/hpost	Thu Dec 17 09:30:35 2020 +0000
@@ -1,12 +1,15 @@
  #!/bin/rc
  rfork e
  url=()
+headers=()
+action=()
+method=()
  at=()	# text fields
  af=()	# file fields
  l=()

  fn usage {
-	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -u ] url [ field:value | 
field@file ... ]' >[1=2]
+	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -r ] headers [ -u ] url 
[ field:value | field@file ... ]' >[1=2]
  	exit usage
  }

@@ -14,6 +17,7 @@
  	switch($1){
  	case -l;	l=($l $1)
  	case -u;	shift; url=$1
+	case -r;	shift; headers=($headers -r $1)
  	case -g;	shift; action=$1; method=mget
  	case -p;	shift; action=$1; method=mpost
  	case -m;	shift; action=$1; method=multi
@@ -94,8 +98,9 @@
  	menc $"f | $hget -r 'Content-Type: multipart/form-data; boundary='$"f 
-b $url -P $action
  }

+
  if(! ~ $action ''){
-	hget=(hget $l)
+	hget=(hget $headers $l)
  	$method
  	exit
  }
diff -r ac35c963f8e2 sys/man/1/hget
--- a/sys/man/1/hget	Sun Dec 13 20:23:03 2020 +0100
+++ b/sys/man/1/hget	Thu Dec 17 09:30:35 2020 +0000
@@ -38,6 +38,9 @@
  .B -m
  .I action
  ] [
+.B -r
+.I header
+] [
  .B -u
  ]
  .I url

Re: [9front] [PATCH] Update hpost to use -r header like hget

[Alex Musolino]

>   fn usage {
> -	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -u ] url [ field:value | 
> field@file ... ]' >[1=2]
> +	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -r ] headers [ -u ] url 
> [ field:value | field@file ... ]' >[1=2]
>   	exit usage
>   }

I think this bit should be:

  fn usage {
-	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -u ] url [ field:value | 
field@file ... ]' >[1=2]
+	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -r header ] [ -u ] url 
[ field:value | field@file ... ]' >[1=2]
  	exit usage
  }

This matches the man page and the usage string from hget(1).  Also, it
wouldn't hurt to amend the man page to mention that the -r option can
be repeated.

--
Cheers,
Alex Musolino

Re: [9front] [PATCH] Update hpost to use -r header like hget

[Julien Blanchard]

On 17/12/2020 11:30, Alex Musolino wrote:
>>    fn usage {
>> -	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -u ] url [ field:value |
>> field@file ... ]' >[1=2]
>> +	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -r ] headers [ -u ] url
>> [ field:value | field@file ... ]' >[1=2]
>>    	exit usage
>>    }
> 
> I think this bit should be:
> 
>    fn usage {
> -	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -u ] url [ field:value |
> field@file ... ]' >[1=2]
> +	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -r header ] [ -u ] url
> [ field:value | field@file ... ]' >[1=2]
>    	exit usage
>    }
> 
> This matches the man page and the usage string from hget(1).  Also, it
> wouldn't hurt to amend the man page to mention that the -r option can
> be repeated.
> 

Good idea, here's the updated diff:


diff -r ac35c963f8e2 rc/bin/hpost
--- a/rc/bin/hpost	Sun Dec 13 20:23:03 2020 +0100
+++ b/rc/bin/hpost	Thu Dec 17 11:21:32 2020 +0000
@@ -1,12 +1,15 @@
  #!/bin/rc
  rfork e
  url=()
+headers=()
+action=()
+method=()
  at=()	# text fields
  af=()	# file fields
  l=()

  fn usage {
-	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -u ] url [ field:value | 
field@file ... ]' >[1=2]
+	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -r ] header [ -u ] url [ 
field:value | field@file ... ]' >[1=2]
  	exit usage
  }

@@ -14,6 +17,7 @@
  	switch($1){
  	case -l;	l=($l $1)
  	case -u;	shift; url=$1
+	case -r;	shift; headers=($headers -r $1)
  	case -g;	shift; action=$1; method=mget
  	case -p;	shift; action=$1; method=mpost
  	case -m;	shift; action=$1; method=multi
@@ -94,8 +98,9 @@
  	menc $"f | $hget -r 'Content-Type: multipart/form-data; boundary='$"f 
-b $url -P $action
  }

+
  if(! ~ $action ''){
-	hget=(hget $l)
+	hget=(hget $headers $l)
  	$method
  	exit
  }
diff -r ac35c963f8e2 sys/man/1/hget
--- a/sys/man/1/hget	Sun Dec 13 20:23:03 2020 +0100
+++ b/sys/man/1/hget	Thu Dec 17 11:21:32 2020 +0000
@@ -38,6 +38,9 @@
  .B -m
  .I action
  ] [
+.B -r
+.I header
+] [
  .B -u
  ]
  .I url
@@ -119,6 +122,9 @@
  .B -r
  sends an arbitrary HTTP
  .IR header .
+The
+.B -r
+flag can be repeated to send multiple headers.
  .PP
  Option
  .B -m

Re: [9front] [PATCH] Update hpost to use -r header like hget

[ori]

Quoth Julien Blanchard <julien@typed-hole.org>:
> 
> Good idea, here's the updated diff:

Committed.

FYI, your mail client line wrapped the diff -- if your
client touches your message inappropriately, you can send
it as an attachment (preferably inline, since it's easier
to quote parts and respond to them)

thanks!

Re: [9front] [PATCH] Update hpost to use -r header like hget

[estevan.cps]

Hello,

> I had issues with just expanding the headers using -r^headers. It 
> generates an hget command like (just an echo before line 98):
> 
> hget -rX-Foo: Lol -rAuthorization: Bearer Token -r Content-Type: 
> multipart/form-data; boundary=HJBOUNDARY -b https://httpbin.org -P /post

The output from that echo is likely misleading, "-rX-Foo: Lol", for example, has a space in it but is passed as a single argument to the program; you'd probably be better off testing that sort of stuff with something that shows arguments separately (unless you're aware of the implications of echoing it and don't care); the following could be helpful:

#!/bin/rc

for(i in `{seq 0 $#*})
	echo $i: $*($i)

> where a valid hget command would be:
> hget -r 'X-Foo: Lol' -r 'Authorization: Bearer Token' -r 'Content-Type: 
> multipart/form-data; boundary=HJBOUNDARY' -b https://httpbin.org -P /post

That's rather inconsistent with, i.e. how arg(2) would parse flags though; (unless I'm overlooking something probably really simple) it probably should handle the argument to the flag whether it is in the same "argv-argument" or not. That can probably be achieved by rewriting hget (and hpost, for that matter) to use something like getflags(8), but that would probably require a patch to getflags to handle "variadic flag arguments" (so multiple -r flags would add the contents to a list instead of overwriting them in turn like it would do now). Could probably sketch something a little bit later (maybe someone has something to add as well?).

Hope that's helpful,

--
Tevo

Re: [9front] [PATCH] Update hpost to use -r header like hget

[magma698hfsp273p9f]

Julien Blanchard <julien@typed-hole.org> writes:

> diff -r ac35c963f8e2 rc/bin/hpost
> --- a/rc/bin/hpost	Sun Dec 13 20:23:03 2020 +0100
> +++ b/rc/bin/hpost	Tue Dec 15 17:45:26 2020 +0000
> @@ -1,12 +1,13 @@
>  #!/bin/rc
>  rfork e
>  url=()
> +header=()
>  at=()	# text fields
>  af=()	# file fields
>  l=()
>
>  fn usage {
> -	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -u ] url [
> field:value | field@file ... ]' >[1=2]
> +	echo 'usage: hpost [ -l ] [ -[gpm] action ] [ -r ] header [ -u
> ] url [ field:value | field@file ... ]' >[1=2]

I think that should be: [ -r header ] ...
rather than: [ -r ] header

>  	exit usage
>  }
>
> @@ -14,6 +15,7 @@
>  	switch($1){
>  	case -l;	l=($l $1)
>  	case -u;	shift; url=$1
> +	case -r;	shift; header=$1
>  	case -g;	shift; action=$1; method=mget
>  	case -p;	shift; action=$1; method=mpost
>  	case -m;	shift; action=$1; method=multi
> @@ -95,7 +97,10 @@
>  }
>
>  if(! ~ $action ''){
> -	hget=(hget $l)
> +	if(~ $header '')
> +		hget=(hget $l)
> +	if not
> +		hget=(hget -r $header $l)
>  	$method
>  	exit
>  }