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

On 02/20/2013 07:26 PM, Amos Jeffries wrote:
> On 21/02/2013 2:47 p.m., Alex Rousskov wrote:
>> Hello,
>>
>> 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.
>>
>> The attached patch fixes the problem in my tests. Is there a better way
>> to do this?

> 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.

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

> Example patch attached. Re-formatting is avoided to show clearly the
> small amount of change needed.

Are we sure that valid user names cannot contain quotes or other symbols
that strwordtok() may want to process specially? Today and tomorrow? It
seems reasonable to have two tokenizing APIs, one that handles
shell-like escaping/quoting (and possibly variable substitution in the
future) and another that just extracts words based on whitespace. Their
use cases are very different.

If you disagree that two functions are better than one here, how about
applying the "slashDecode" flag that you want to add to _all_ special
strwordtok() powers, naming it simply "decode"?

After all, if backslashes are not treated specially, not everything can
be successfully quoted anyway. In other words, if input format does not
support an escape mechanism such as backslashes it cannot fully support
quotes either.

> 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).

Thank you,

Alex.
Received on Thu Feb 21 2013 - 05:41:19 MST