Re: [PATCH] Fix session helper "crashing too rapidly"

On Tue, 2011-09-20 at 12:09 +1200, Amos Jeffries wrote:
> > Do you mean the -d option to the Squid binary? If so, this doesn't
> > seem
> > to make any difference; it just prints all the log messages to the
> > display as well as the log file.
>
> -d parameter of the helper binary. stderr is piped to cache.log at
> level-0. Thus the need for a separate switch to enable lots of debug
> from the helper.

Sorry if I'm being stupid, but do you mean the actual session helper
binary or something else? If it's the former, then that obviously just
causes an exit due to an invalid option.

> > Done. Also, the following page should be updated:
> >
> > http://wiki.squid-cache.org/ConfigExamples/Portal/Splash
> >
> > I'm happy to do it myself, if you can give me wiki edit rights?
>
> Sure. Just need to know what username you login with.

Great stuff. AndrewBeverley

> -h is reserved for display of "help" / usage() texts.
>

Good point.

> -T would probably be better for an absolute timeout.
>

Changed as suggested.

> ext_session_acl.8:
> * " (unless -h is specified).". Please make a separate sentence.
> Something like "Also supports fixed length timeouts for regular splash
> page displays."

Done.

> (if you can think of any other useful scenario than regular splash
> pages that can be mentioned too.)

Well I've mentioned the possibility of forcing a user to
re-authenticate, but I'm not sure that I'm correct in saying that. Do
you think that is possible?

> * rather than calling the new mode "Hard timeout". It's a "Periodic"
> or "Fixed length" timeout ... something a bit more descriptive like
> that.

Done.

> ** Needs to explain how it interacts with -a mode. ie I would expect
> LOGOUT or timeout to terminate the session. Explicit LOGIN required for
> a new one to start.

Done.

> ext_session_acl.cc: (modulo the -h ==> -T change requested)
> * in getopt() format syntax ':' indicates a value is received for the
> option. What you coded is "t:b:a?h?".
> * I think you should make -t and -T both optional arguments which are
> interchangeable. ("t:T:b:a?")

Done.

Please find updated patch attached.

Andy