[PATCH] Authenticating proxy support in mercurial

[Michael S. Tsirkin]

Quoting r. Matt Mackall <mpm at selenic.com>:
> Subject: Re: [PATCH] Authenticating proxy support in mercurial
> 
> On Wed, Jun 08, 2005 at 08:31:04PM +0300, Michael S. Tsirkin wrote:
> > Quoting r. Jake Edge <jake at edge2.net>:
> > > Subject: Re: hg merge: TypeError
> > > >>I'm behind a proxy, could that be the problem, somehow?
> > > 
> > > ... if you are going through a proxy, that would mean that there is some 
> > > other host that you need to contact on the way out to the net ... are 
> > > you unable to do 'telnet selenic.com 80' from your localhost because of 
> > > firewalling or somesuch?  if so, i don't think hg merge is going to work 
> > > without some changes to the code ...
> > 
> > Okay, what I have here is proxy with password authentication.
> 
> Good stab at an interesting problem.
> 
> Things I'd suggest:
> 
> - sync up with the tree, quite a lot has changed since last week!

I did, the patch applies there.

> - config handling should be a property of ui, and not the repo
> - hgpaths should be handled this way too!
> 
> > diff -rup mercurial-0.5b/mercurial/hg.py mercurial-0.5b-proxy/mercurial/hg.py
> > --- mercurial-0.5b/mercurial/hg.py	2005-05-30 18:32:59.000000000 +0300
> > +++ mercurial-0.5b-proxy/mercurial/hg.py	2005-06-08 20:29:25.000000000 +0300
> > @@ -7,6 +7,8 @@
> >  
> >  import sys, struct, sha, socket, os, time, re, urllib2
> >  import urllib
> > +import urllib2
> 
> See the new demandload bits
> 
> > +import ConfigParser
> >  from mercurial import byterange, lock
> >  from mercurial.transaction import *
> >  from mercurial.revlog import *
> > @@ -844,6 +846,57 @@ class remoterepository:
> >      def __init__(self, ui, path):
> >          self.url = path
> >          self.ui = ui
> > +	pf = os.path.join(os.environ["HOME"], ".hgrc")
> 
> Use os.path.expanduser("~/.hgrc")

OK but I think I saw HOME in some other place in code.
Try to grep for it.

> > +	p = ConfigParser.SafeConfigParser()
> > +	p.read(pf)
> 
> Stick this in ui.__init__ and make it retrievable as config(). Then if
> we need to, we can demand load it.
> 
> Tell you what, I like this config file idea enough that I'll implement
> that piece of it myself. And then you can handle the proxy bit.

Good. Thanks.

> > +
> > +	if (p.has_option("http_proxy", "host")):
> > +		host = p.get("http_proxy", "host")
> > +	else:
> > +		host = None
> 
> Use get with a default instead:
> 
>     host = p.get("http_proxy", "host", None)
> 
> Then this section all gets really small.

Right.

> > +
> > +	if (p.has_option("http_proxy", "user")):
> > +		user = p.get("http_proxy", "user")
> > +	else:
> > +		user = None
> > +
> > +	if (p.has_option("http_proxy", "passwd")):
> > +		passwd = p.get("http_proxy", "passwd")
> > +	else:
> > +		passwd = None
> > +
> > +	if (p.has_option("http_proxy", "no")):
> > +		no = p.get("http_proxy", "no")
> > +		no_list = no.split(",")
> 
> Can we have a whitespace-separated list instead?

Sure - commas come from netscape background

> > +	else:
> > +		no_list = []
> > +
> > +	no_proxy = 0
> > +	for h in no_list:
> > +		if h in path:
> > +			no_proxy = 1
> 
> Might want startswith rather than in.
> 
> And maybe filter().

I'm new to python, so I'll have to read up on these constructs :). Links ?

> > +	if ("http://localhost:" in path):
> > +		no_proxy = 1
> > +
> > +	if ((no_proxy == 1) or (host == None)) :
> 
> if no_proxy or not host:
> 
> > +		os.environ["default_proxy"] = ""
> > +		os.environ["http_proxy"] = ""
> > +		os.environ["HTTP_PROXY"] = ""
> 
> This looks suspicious. Is this how urllib2 wants things?

It takes proxy values from the environment and the only
way to turn proxy access off that I found is this.

> > +		proxy_handler = urllib2.BaseHandler()
> > +	else:
> > +		proxy_handler = urllib2.ProxyHandler({"http" : "http://" + host})
> > +
> > +
> > +	if ((user != None) and (passwd != None)) :
> 
> if user and passwd:
> 
> > +		passmgr = urllib2.HTTPPasswordMgrWithDefaultRealm()
> > +		passmgr.add_password(None, host, user, passwd)
> > +		authinfo = urllib2.ProxyBasicAuthHandler(passmgr)
> > +	else:
> > +		authinfo = None
> > +
> > +	opener = urllib2.build_opener(proxy_handler, authinfo)
> > +	urllib2.install_opener(opener)
> >  
> >      def do_cmd(self, cmd, **args):
> >          self.ui.debug("sending %s command\n" % cmd)
> > @@ -851,7 +904,7 @@ class remoterepository:
> >          q.update(args)
> >          qs = urllib.urlencode(q)
> >          cu = "%s?%s" % (self.url, qs)
> > -        return urllib.urlopen(cu)
> > +        return urllib2.urlopen(cu)
> >  
> >      def branches(self, nodes):
> >          n = " ".join(map(hex, nodes))
> > diff -rup mercurial-0.5b/README mercurial-0.5b-proxy/README
> > --- mercurial-0.5b/README	2005-05-29 20:23:56.000000000 +0300
> > +++ mercurial-0.5b-proxy/README	2005-06-08 20:20:20.000000000 +0300
> > @@ -90,3 +90,18 @@ Network support:
> >   foo$ echo "main http://selenic.com/hg/" >> ~/.hgpaths # one pair per line
> >   foo$ hg merge main
> >   foo$ hg co
> > +
> > +Non-transparent proxy support:
> > +
> > + To access a mercurial repository through a proxy,
> > + create a file ~/.hgrc in the following format:
> > +
> > +[http_proxy]
> > +host=myproxy:8080
> > +user=<username>
> > +passwd=<password>
> > +no=<localhost1>,<localhost2>,<localhost3>,...
> > +
> > + "user","passwd" fields are used for authenticating proxies,
> > + "no" is a comma-separated list of local host names for which
> > + proxy must be bypassed.
> 
> This stuff ought to go in doc/hg.1.txt instead. The README is the
> 1-minute introduction.
> 

OK

-- 
MST