[squid-dev] [PATCH] Fix ext_session_acl to handle - when no argument is passed

[Alex Rousskov]

On 03/22/2017 06:44 AM, Amos Jeffries wrote:

> This is a side effect of the change to using logformat tokens. The
> %DATA field is always present now.

My understanding is that, after the logformat changes, Squid started
appending "-" to the helper query when an external ACL has no
parameters. The examples use parameterless ACLs to indicate that no or
default action is to be taken (depending on active/passive mode). We now
need to adjust the helper code to _expect_ that dash.


> +            } else if (!default_action && strcmp(lastdetail, " -") == 0) {
> +                // no action; LOGIN/LOGOUT not supplied
> +                // but truncate the '-' %DATA value given by Squid-4 and later
> +                detail_len = (size_t)(lastdetail-detail);
> +                *lastdetail = '\0';
>              }
> +            // else do nothing

s/truncate/remove/?

It is not clear why we need to remove the dash only when default_action
is zero (i.e., in active mode). The dash is going to be present in both
modes, right? I am not saying the patch is wrong; only that it is not
clear why the code does what it does.

I would expect something along these lines instead (pseudo code):

  // What action was requested (including no/default action)?
  if (... LOGIN ...) action = 1;
  else if (... LOGOUT ...) action = -1;
  else if (... - ...) action = default_action;
  else { ... BH unexpected query format ...; continue; }

  // remove the action field: it is not a part of the db key(?)
  detail_len = (size_t)(lastdetail-detail);
  lastdetail = '\0';

  ... do what the action says ...


HTH,

Alex.