Re: [PATCH] Bug 1961 pt2: store URI userinfo (aka login) in class URL

On Sun, Apr 27, 2014 at 4:52 PM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> This continues the bug 1961 redesign of URL handling.

Hi,

+ SBuf tmp = checklist->request->url.userInfo();
+ // NOTE: unescape works in-place and may decrease the c-string
length, never increase.
+ rfc1738_unescape(const_cast<char*>(tmp.c_str()));

is troublesome: you are most likely trampling the userinfo data as
rfc1738_unescape is acting on a shared MemBlob (shared by tmp and
userInfo at least) and c_str() doesn't guarantee a COW. The best way
to do this is by defining a SBuf rfc1738_unescape (const SBuf &) which
copies the param if needed.

+ if (colonPos != 0) {

shouldn't this be if (colonPos != SBuf::npos) ?

+ memcpy(user, userName.rawContent(), userName.length());
+ user[userName.length()] = '\0';

you could use
SBuf::size_type upTo = min(userName.length(), sizeof(user));
userName.copy(user, upto);
user[upto]='\0';

Same for
+ memcpy(loginbuf,
request->url.userInfo().rawContent(), len);

+ static char result[MAX_URL*2]; // should be big enough
for a single URI segment
That conditional is worrisome. Is there any risk of the result not
being big enough?

-- 
    Francesco