[MPlayer-dev-eng] [PATCH 2/7] Use Proxy-Authorization instead of Authorization for proxy auth

[Clément Bœsch]

On Tue, Nov 16, 2010 at 10:12:58PM +0100, Reimar Döffinger wrote:
> On Tue, Nov 16, 2010 at 09:53:06PM +0100, Clément Bœsch wrote:
> > On Tue, Nov 16, 2010 at 09:05:24PM +0100, Reimar Döffinger wrote:
> > > On Fri, Nov 12, 2010 at 10:40:57AM +0100, Clément Bœsch wrote:
> > > > @@ -637,13 +637,13 @@ http_add_basic_authentication( HTTP_header_t *http_hdr, const char *username, co
> > > >  
> > > >  	b64_usr_pass[out_len]='\0';
> > > >  
> > > > -	auth = malloc(encoded_len+22);
> > > > +	auth = malloc(encoded_len + strlen(auth_str) + sizeof(": Basic "));
> > > >  	if( auth==NULL ) {
> > > >  		mp_msg(MSGT_NETWORK,MSGL_FATAL,MSGTR_MemAllocFailed);
> > > >  		goto out;
> > > >  	}
> > > >  
> > > > -	sprintf( auth, "Authorization: Basic %s", b64_usr_pass);
> > > > +	sprintf( auth, "%s: Basic %s", auth_str, b64_usr_pass);
> > > 
> > > I think their ok, though I think personally I'd feel slightly
> > > more comfortable if it was something like
> > > > buffer_len = encoded_len + 100; // arbitrary, large enough number
> > > > malloc(buffer_len);
> > > > snprintf(, buffer_len, ....);
> > > 
> > > While hard-coded is not that great, it avoids wasting CPU
> > > time on strlen, and also the duplicated ": Basic " string
> > > is a risk, and this way at least ensure there'll never be
> > > a buffer overflow no matter what (not having to thing about
> > > integer overflows, someone changing only one of those strings,
> > > ...).
> > 
> > The CPU time on strlen in this case is imo totally inappropriate, however
> > I agree with you arguments about the duplicated string, so what about the
> > attached patch?
> 
> Well, while it's very unlikely to be a _new_ issue with this patch,
> to review it properly I'd still have to check that the addition
> will not cause an integer overflow.

Changing encoded_len type from int to size_t may help?

> And the CPU (and honestly more code complexity) is something I just consider
> because, honestly, I can't see much of a point in using a "100 % correct"
> number.

Not sure I get what you mean. Anyway, I still find the allocation
procedure more appropriated, and I just updated the current code.

-- 
Clément B.
Not sent from a jesusPhone.