Re: [PATCH] NA - token = fatalf

On 13/02/2013 2:37 p.m., Alex Rousskov wrote:
> On 02/12/2013 03:33 PM, Henrik Nordström wrote:
>> tis 2013-02-12 klockan 14:41 -0700 skrev Alex Rousskov:
>>> Could somebody with better authentication and helper knowledge clarify
>>> whether the token field is indeed required for Nagotiate ERR and NA
>>> responses? If not, can we just remove the above quoted fatalf() blob and
>>> make the following line conditional on the token presence?
>> Squid-2 negotiate expects
>>
>> NA<SPACE>blob<SPACE>message<NEWLINE>
>>
>> but do not require any of them to be present.
> Is the attached fix on the right track? It makes the "token" part of the
> helper response optional and, hence, removes the fatalf() message. No
> other changes were intended, but this trunk patch is untested.
>
>
> Thank you,
>
> Alex.
>

See my other message..

Yes your patch looks reasonable design for removal of the *specific*
fatal() on the ERR/NA switch case, but only because the normal handling
of that case is auth failure and cleanup. The other fatal()'s on success
should be removed as well and cannot ignore the token= or user= absence.

** Like Henrik said the message= portion may be empty or missing. If
token is missing it is much more likely that message is also missing,
otherwise on NA the first word of the message would very likely be
wrongly mapped as the token. So the messageNote dereference will need to
be protected as well in this change.

Amos
Received on Wed Feb 13 2013 - 02:31:54 MST