On Tue, Jan 03, 2023 at 10:02:26AM +0100, Omar Polo wrote:
> On 2023/01/03 00:16:40 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > You're right, I added this for symmetry to request timeout but there
> > is probably no good reason to expose this to users. I've dropped the
> > user-facing parts in the new patch below. We can always put them back
> > in if they ever turn out to be needed (e.g. because someone runs yellow
> > pages and passwd lookups take forever for some reason).
> 
> have to admit I haven't thought about YP at all.  I don't have any
> experience with it.  Since some functions are now routed thru the
> internet the timeout makes even more sense.
> 
> still no idea if the knob would be useful, i'll defer the decision to
> people actually using it :)

I hope nobody is routing YP via the internet, but who knows...
Usually, networked login systems run on a local network and are quite fast.
I believe 5 seconds is very generous.

> > @@ -135,6 +139,16 @@ main		: UNIX_SOCKET STRING {
> >  		| NUMBER { $$ = $1; }
> >  		;
> >  
> > +timeout		: NUMBER {
> > +			if ($1 < 0) {
> 
> should zero be disallowed too?

Not when parsing a raw timeout value. We might at some point add a timeout
config knob where zero would mean "forever". But the request timeout value
should indeed be > 0, see diff below. ok?

> > +				yyerror("invalid timeout: %lld", $1);
> > +				YYERROR;
> > +			}
> > +			$$.tv_sec = $1;
> > +			$$.tv_usec = 0;
> > +		}
> > +		;
> > +
> 

diff /home/stsp/src/got
commit - 40b85cca5d86ebef3a353efd464af989c3ebf18b
path + /home/stsp/src/got
blob - 580f43381ccf163eac2c738800e7efd25c7562e4
file + gotd/parse.y
--- gotd/parse.y
+++ gotd/parse.y
@@ -194,6 +194,10 @@ conflags	: REQUEST TIMEOUT timeout		{
 		;
 
 conflags	: REQUEST TIMEOUT timeout		{
+			if ($3.tv_sec <= 0) {
+				yyerror("invalid timeout: %lld", $3.tv_sec);
+				YYERROR;
+			}
 			memcpy(&gotd->request_timeout, &$3,
 			    sizeof(gotd->request_timeout));
 		}