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

From: Kinkie <gkinkie_at_gmail.com>
Date: Sun, 27 Apr 2014 17:51:32 +0200

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
Received on Sun Apr 27 2014 - 15:51:43 MDT

This archive was generated by hypermail 2.2.0 : Mon Apr 28 2014 - 12:00:18 MDT