[MPlayer-dev-eng] [PATCH 3/7] Use authorization field to pass server authentication through proxy

[Clément Bœsch]

On Fri, Nov 19, 2010 at 02:57:28PM +0100, Clément Bœsch wrote:
> On Fri, Nov 19, 2010 at 07:54:06AM +0100, Reimar Döffinger wrote:
> > On Fri, Nov 19, 2010 at 12:11:58AM +0100, Clément Bœsch wrote:
> > > On Thu, Nov 18, 2010 at 10:32:50PM +0100, Reimar Döffinger wrote:
> > > > On Thu, Nov 18, 2010 at 09:34:24PM +0100, Clément Bœsch wrote:
> > > > > diff --git a/stream/network.c b/stream/network.c
> > > > > index 02ddcc8..6b7117c 100644
> > > > > --- a/stream/network.c
> > > > > +++ b/stream/network.c
> > > > > @@ -209,7 +209,12 @@ http_send_request( URL_t *url, off_t pos ) {
> > > > >  			mp_msg(MSGT_NETWORK, MSGL_ERR, "Invalid URL '%s' to proxify\n", url->file+1);
> > > > >  			goto err_out;
> > > > >  		}
> > > > > -		http_set_uri( http_hdr, server_url->url );
> > > > > +		snprintf(str, sizeof(str), "%s://%s:%d%s",
> > > > > +				server_url->protocol,
> > > > > +				server_url->hostname,
> > > > > +				server_url->port ? server_url->port : 80,
> > > > > +				server_url->file);
> > > > > +		http_set_uri(http_hdr, str);
> > > > 
> > > > What's your opinion on removing user/password from ->url in
> > > > url_new instead?
> > > > I think it is quite ugly, but I guess I am fine with it if
> > > > you think it's the best solution.
> > > > I do feel a bit uncomfortable about using fixed-size, on-stack
> > > > destination, URLs can get quite gigantic...
> > > 
> > > Indeed, fixed-size for URLs is a problem. But I'm not sure about some
> > > issues that can be caused by the modification of the url field. The first
> > > one that come to my mind is the http_proxy url build in
> > > stream/network.c:178 for example. This will need modification, and it may
> > > be not the only case.
> > > 
> > > However, it's still possible to add a field say "noauth_url". This can
> > > also be used when writing the stream URL on stdout (and avoid displaying
> > > the authentication). This solution looks like harmless.
> > > 
> > > Patch attached with this solution.
> > 
> > Seems ok if you can do a good solution with that.
> > Though could you use snprintf, just to be paranoid?
> 
> Yep sure, patch updated.
> 
> > Actually, I think there is an actual bug, the string
> > there will become longer if url was
> > a://b/c it will become a://b:80/c.
> 
> Yes indeed, this is fixed.
> 
> > Also :80 is wrong for e.g. ftp.
> 
> It's not present anymore: port is not specified if not set now, just like
> previous patch.
> 
> I also added the missing free, so it should be ok.
> 

OK to apply? It's the last feature to make proxy authentication possible.

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