Bug 6861 - rfc1738_unescape converts '+' characters to spaces.
rfc1738_unescape converts '+' characters to spaces.
Product: Samba 3.4
Classification: Unclassified
Component: Ntlm_auth Tool
Other Linux
: P3 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
Depends on:
  Show dependency treegraph
Reported: 2009-10-30 15:55 UTC by Jeremy Allison
Modified: 2012-05-23 20:06 UTC (History)
0 users

See Also:
jra: review? (metze)

git-am format patch for 3.4.4. (769 bytes, patch)
2009-10-30 16:18 UTC, Jeremy Allison
jra: review?
metze: review? (abartlet)

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2009-10-30 15:55:48 UTC
It shouldn't do this. From a post to the list by Andrew Bartlett:

I noticed doing some work in master that the current rfc1738_unescape()

        while ((p=strchr(p,'+')))
                *p = ' ';

As far as I can see, this was merged incorrectly from Samba4 when the
string util code was brought back in common in Nov last year.  The
Samba3 code is deliberately missing this loop, with this loop move to
SWAT specificity.

Anyway, the long and the short of it is that ntlm_auth uses
rfc1738_unescape(), and when Squid calls the plaintext ntlm_auth helpers,
it does not replace space characters with +, so this merge (I suspect)
breaks squid.

I'm importing a full rfc1738 decode and encode implementation from
squid, but a patch to 3.4 may wish to just remove those lines.
Comment 1 Jeremy Allison 2009-10-30 16:18:54 UTC
Created attachment 4907 [details]
git-am format patch for 3.4.4.
Comment 2 Jeremy Allison 2009-10-30 16:56:54 UTC
3.2.x is not affected by this.
Comment 3 Jeremy Allison 2009-10-30 16:57:26 UTC
Re-assigning to Karolin for inclusion in 3.4.4.
Comment 4 Stefan Metzmacher 2009-10-31 04:25:51 UTC
is 3.3.x affected?
Comment 5 Jeremy Allison 2009-10-31 12:07:48 UTC
No, 3.3.x is not affected as it looks like the bad code was added in the 3.4 -> Samba4 merge.
Comment 6 Jeremy Allison 2009-11-09 15:20:53 UTC
Getting some reviewers so this can get pushed for 3.4.4.
Comment 7 Stefan Metzmacher 2009-11-10 04:56:35 UTC
Comment on attachment 4907 [details]
git-am format patch for 3.4.4.

Andrew can you ack that one...
Comment 8 Andrew Bartlett 2009-11-10 05:23:19 UTC
I'm happy to ack that this puts things back how they were before the dud merge. 

Comment 9 Karolin Seeger 2009-11-10 05:27:18 UTC
Pushed to v3-4-test.
Closing out bug report.