Re: [PATCH] Preserve bare backslashes in AF and TT

On 02/20/2013 11:19 PM, Amos Jeffries wrote:
> On 21/02/2013 6:41 p.m., Alex Rousskov wrote:
>> On 02/20/2013 07:26 PM, Amos Jeffries wrote:
>>> On 21/02/2013 2:47 p.m., Alex Rousskov wrote:
>>>> It looks like NTLM and possibly Negotiate authentication is broken
>>>> in trunk because Squid eats the bare backslash that AF responses use to
>>>> separate authentication domain and user names. With the backslash
>>>> removed, the merged domainuser name never matches, of course.

>>> strwordtok() is a function of our own creation so it is probably best
>>> long term to adjust its API to include a flag for skipping the
>>> slashDecode behaviour instead of creating duplicate code.

>> Please note that the function I added does not duplicate strwordtok()
>> behavior. If anything, it duplicates strtok(), but for good reasons.

> Noted.

>> When two functions cover very different use cases and have rather
>> different semantics (not to mention implementation), I think they both
>> deserve to live.

> I do disagree that two functions are needed and am happy with a flag
> disabling all the special handling in strwordtok().

>>> I like the extra documentation you added about statics and use cases.
>>> Can you write something similar about strwordtok() ?

>> Sure, will do (regardless of whether we add a second tokenizing function
>> or not).

Hi Amos,

    The attached patch dumbs strwordtok() further down when the new
optional decode parameter is false. I also adjusted the comment as promised.

Looking at the final result, I am even more certain that two functions
are better than one function with a parameter in this case. It would be
sad to see other, more performance sensitive code to start using
strwordtok(decode=false) for parsing instead of a simple and efficient
implementation needed for that.

If nobody else wants to weigh in, I will let you decide how to proceed.
Please commit the code that you think should go in now.

Thank you,

Alex.