| Submitter | Stéphane Klein |
|---|---|
| Date | Nov. 25, 2013, 4:22 p.m. |
| Message ID | <52937932.1060007@bearstech.com> |
| Download | mbox | patch |
| Permalink | /patch/3120/ |
| State | Superseded |
| Headers | show |
Comments
On Mon, Nov 25, 2013 at 05:22:10PM +0100, Stéphane Klein wrote: > # HG changeset patch > # User Stéphane Klein <contact@stephane-klein.info> > # Date 1385396292 -3600 > # Mon Nov 25 17:18:12 2013 +0100 > # Node ID ec1afe2354a840eb47a2286308fb5a5eeed91eba > # Parent 1c46b18b0e1c47fa4cecf21b78c083a54ae9903f > Keep authentication information after the first fail HTTP access (issue3567) Patch looks reasonable, though the first line of the commit message probably wants to be: url: cache HTTP Basic authentication headers and reuse them on later requests (issue3567) (or something similar) Don't resend the patch for that. I'd like someone else to look at this quickly and just verify that I'm not insane that it's reasonable, since it deals with sending authn information. > > Context : mercurial access to repository server with http access, and this > server is protected by basic auth. > > Before patch : > > * mercurial try an anonymous access to server, server return 401 response and > mercurial resend request with login / password information > > After patch : > > * mercurial try an anonymous access to server, server return > 401 response. For all next requests, mercurial keep in memory this > information (this server need basic auth information). > > This patch reduce the number of http access against mercurial server. > > Example, before patch : > > 10.10.168.170 - - [25/Oct/2013:15:44:51 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:44:52 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:00 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:01 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:03 +0200] "GET /hg/testagt?cmd=batch > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:04 +0200] "GET /hg/testagt?cmd=batch > HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:06 +0200] "GET /hg/testagt?cmd=getbundle > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:07 +0200] "GET /hg/testagt?cmd=getbundle > HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:09 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:10 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 - "-" "mercurial/proto-1.0" > > Example after patch : > > 10.10.168.170 - - [28/Oct/2013:11:49:14 +0100] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:15 +0100] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:17 +0100] "GET /hg/testagt?cmd=batch > HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:19 +0100] "GET /hg/testagt?cmd=getbundle > HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:22 +0100] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:24 +0100] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 - "-" "mercurial/proto-1.0" > > In this last example, you can see only one 401 response. > > diff -r 1c46b18b0e1c -r ec1afe2354a8 mercurial/url.py > --- a/mercurial/url.py Fri Nov 22 17:26:58 2013 -0600 > +++ b/mercurial/url.py Mon Nov 25 17:18:12 2013 +0100 > @@ -8,6 +8,7 @@ > # GNU General Public License version 2 or any later version. > > import urllib, urllib2, httplib, os, socket, cStringIO > +import base64 > from i18n import _ > import keepalive, util, sslutil > import httpconnection as httpconnectionmod > @@ -418,9 +419,22 @@ > > class httpbasicauthhandler(urllib2.HTTPBasicAuthHandler): > def __init__(self, *args, **kwargs): > + self.auth = None > urllib2.HTTPBasicAuthHandler.__init__(self, *args, **kwargs) > self.retried_req = None > > + def http_request(self, request): > + if self.auth: > + request.add_unredirected_header(self.auth_header, self.auth) > + > + return request > + > + def https_request(self, request): > + if self.auth: > + request.add_unredirected_header(self.auth_header, self.auth) > + > + return request > + > def reset_retry_count(self): > # Python 2.6.5 will call this on 401 or 407 errors and thus loop > # forever. We disable reset_retry_count completely and reset in > @@ -435,6 +449,19 @@ > return urllib2.HTTPBasicAuthHandler.http_error_auth_reqed( > self, auth_header, host, req, headers) > > + def retry_http_basic_auth(self, host, req, realm): > + user, pw = self.passwd.find_user_password(realm, host) > + if pw is not None: > + raw = "%s:%s" % (user, pw) > + auth = 'Basic %s' % base64.b64encode(raw).strip() > + if req.headers.get(self.auth_header, None) == auth: > + return None > + self.auth = auth > + req.add_unredirected_header(self.auth_header, auth) > + return self.parent.open(req, timeout=req.timeout) > + else: > + return None > + > handlerfuncs = [] > > def opener(ui, authinfo=None): > > -- > Stéphane Klein <sklein@bearstech.com> > GSM : 06 61 48 76 04 > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
Le 27/11/13 15:56, Augie Fackler a écrit : > Don't resend the patch for that. I'd like someone else to look at this > quickly and just verify that I'm not insane that it's reasonable, > since it deals with sending authn information. Some news about this code review ?
On Tue, 2013-12-03 at 16:53 +0100, Stéphane Klein wrote: > Le 27/11/13 15:56, Augie Fackler a écrit : > > Don't resend the patch for that. I'd like someone else to look at this > > quickly and just verify that I'm not insane that it's reasonable, > > since it deals with sending authn information. > > Some news about this code review ? The news is always available here: http://mercurial.selenic.com/wiki/ContributingChanges#Flow_control http://www.selenic.com/inbox The current news is that I have 245 messages up to 36 days old to review. As your patch is ~20 days old, you're somewhere near the middle of the backlog, which is not processed in strict FIFO order.
On Mon, Nov 25, 2013 at 05:22:10PM +0100, Stéphane Klein wrote: > # HG changeset patch > # User Stéphane Klein <contact@stephane-klein.info> > # Date 1385396292 -3600 > # Mon Nov 25 17:18:12 2013 +0100 > # Node ID ec1afe2354a840eb47a2286308fb5a5eeed91eba > # Parent 1c46b18b0e1c47fa4cecf21b78c083a54ae9903f > Keep authentication information after the first fail HTTP access (issue3567) Hearing no objections after a couple of weeks, I'm queueing this. It seems reasonable. > > Context : mercurial access to repository server with http access, and this > server is protected by basic auth. > > Before patch : > > * mercurial try an anonymous access to server, server return 401 response and > mercurial resend request with login / password information > > After patch : > > * mercurial try an anonymous access to server, server return > 401 response. For all next requests, mercurial keep in memory this > information (this server need basic auth information). > > This patch reduce the number of http access against mercurial server. > > Example, before patch : > > 10.10.168.170 - - [25/Oct/2013:15:44:51 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:44:52 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:00 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:01 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:03 +0200] "GET /hg/testagt?cmd=batch > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:04 +0200] "GET /hg/testagt?cmd=batch > HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:06 +0200] "GET /hg/testagt?cmd=getbundle > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:07 +0200] "GET /hg/testagt?cmd=getbundle > HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:09 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:10 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 - "-" "mercurial/proto-1.0" > > Example after patch : > > 10.10.168.170 - - [28/Oct/2013:11:49:14 +0100] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:15 +0100] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:17 +0100] "GET /hg/testagt?cmd=batch > HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:19 +0100] "GET /hg/testagt?cmd=getbundle > HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:22 +0100] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:24 +0100] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 - "-" "mercurial/proto-1.0" > > In this last example, you can see only one 401 response. > > diff -r 1c46b18b0e1c -r ec1afe2354a8 mercurial/url.py > --- a/mercurial/url.py Fri Nov 22 17:26:58 2013 -0600 > +++ b/mercurial/url.py Mon Nov 25 17:18:12 2013 +0100 > @@ -8,6 +8,7 @@ > # GNU General Public License version 2 or any later version. > > import urllib, urllib2, httplib, os, socket, cStringIO > +import base64 > from i18n import _ > import keepalive, util, sslutil > import httpconnection as httpconnectionmod > @@ -418,9 +419,22 @@ > > class httpbasicauthhandler(urllib2.HTTPBasicAuthHandler): > def __init__(self, *args, **kwargs): > + self.auth = None > urllib2.HTTPBasicAuthHandler.__init__(self, *args, **kwargs) > self.retried_req = None > > + def http_request(self, request): > + if self.auth: > + request.add_unredirected_header(self.auth_header, self.auth) > + > + return request > + > + def https_request(self, request): > + if self.auth: > + request.add_unredirected_header(self.auth_header, self.auth) > + > + return request > + > def reset_retry_count(self): > # Python 2.6.5 will call this on 401 or 407 errors and thus loop > # forever. We disable reset_retry_count completely and reset in > @@ -435,6 +449,19 @@ > return urllib2.HTTPBasicAuthHandler.http_error_auth_reqed( > self, auth_header, host, req, headers) > > + def retry_http_basic_auth(self, host, req, realm): > + user, pw = self.passwd.find_user_password(realm, host) > + if pw is not None: > + raw = "%s:%s" % (user, pw) > + auth = 'Basic %s' % base64.b64encode(raw).strip() > + if req.headers.get(self.auth_header, None) == auth: > + return None > + self.auth = auth > + req.add_unredirected_header(self.auth_header, auth) > + return self.parent.open(req, timeout=req.timeout) > + else: > + return None > + > handlerfuncs = [] > > def opener(ui, authinfo=None): > > -- > Stéphane Klein <sklein@bearstech.com> > GSM : 06 61 48 76 04 > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On Tue, Dec 10, 2013 at 04:22:39PM -0500, Augie Fackler wrote: > On Mon, Nov 25, 2013 at 05:22:10PM +0100, Stéphane Klein wrote: > > # HG changeset patch > > # User Stéphane Klein <contact@stephane-klein.info> > > # Date 1385396292 -3600 > > # Mon Nov 25 17:18:12 2013 +0100 > > # Node ID ec1afe2354a840eb47a2286308fb5a5eeed91eba > > # Parent 1c46b18b0e1c47fa4cecf21b78c083a54ae9903f > > Keep authentication information after the first fail HTTP access (issue3567) > > Hearing no objections after a couple of weeks, I'm queueing this. It > seems reasonable. (but, I should mention, with some tweaks to the log message to make it more compliant with our format and a minor English tweak) > > > > > Context : mercurial access to repository server with http access, and this > > server is protected by basic auth. > > > > Before patch : > > > > * mercurial try an anonymous access to server, server return 401 response and > > mercurial resend request with login / password information > > > > After patch : > > > > * mercurial try an anonymous access to server, server return > > 401 response. For all next requests, mercurial keep in memory this > > information (this server need basic auth information). > > > > This patch reduce the number of http access against mercurial server. > > > > Example, before patch : > > > > 10.10.168.170 - - [25/Oct/2013:15:44:51 +0200] "GET /hg/testagt?cmd=capabilities > > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:44:52 +0200] "GET /hg/testagt?cmd=capabilities > > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:45:00 +0200] "GET /hg/testagt?cmd=capabilities > > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:45:01 +0200] "GET /hg/testagt?cmd=capabilities > > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:45:03 +0200] "GET /hg/testagt?cmd=batch > > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:45:04 +0200] "GET /hg/testagt?cmd=batch > > HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:45:06 +0200] "GET /hg/testagt?cmd=getbundle > > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:45:07 +0200] "GET /hg/testagt?cmd=getbundle > > HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:45:09 +0200] "GET /hg/testagt?cmd=listkeys > > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:45:10 +0200] "GET /hg/testagt?cmd=listkeys > > HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys > > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys > > HTTP/1.1" 200 - "-" "mercurial/proto-1.0" > > > > Example after patch : > > > > 10.10.168.170 - - [28/Oct/2013:11:49:14 +0100] "GET /hg/testagt?cmd=capabilities > > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [28/Oct/2013:11:49:15 +0100] "GET /hg/testagt?cmd=capabilities > > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [28/Oct/2013:11:49:17 +0100] "GET /hg/testagt?cmd=batch > > HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [28/Oct/2013:11:49:19 +0100] "GET /hg/testagt?cmd=getbundle > > HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [28/Oct/2013:11:49:22 +0100] "GET /hg/testagt?cmd=listkeys > > HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" > > 10.10.168.170 - - [28/Oct/2013:11:49:24 +0100] "GET /hg/testagt?cmd=listkeys > > HTTP/1.1" 200 - "-" "mercurial/proto-1.0" > > > > In this last example, you can see only one 401 response. > > > > diff -r 1c46b18b0e1c -r ec1afe2354a8 mercurial/url.py > > --- a/mercurial/url.py Fri Nov 22 17:26:58 2013 -0600 > > +++ b/mercurial/url.py Mon Nov 25 17:18:12 2013 +0100 > > @@ -8,6 +8,7 @@ > > # GNU General Public License version 2 or any later version. > > > > import urllib, urllib2, httplib, os, socket, cStringIO > > +import base64 > > from i18n import _ > > import keepalive, util, sslutil > > import httpconnection as httpconnectionmod > > @@ -418,9 +419,22 @@ > > > > class httpbasicauthhandler(urllib2.HTTPBasicAuthHandler): > > def __init__(self, *args, **kwargs): > > + self.auth = None > > urllib2.HTTPBasicAuthHandler.__init__(self, *args, **kwargs) > > self.retried_req = None > > > > + def http_request(self, request): > > + if self.auth: > > + request.add_unredirected_header(self.auth_header, self.auth) > > + > > + return request > > + > > + def https_request(self, request): > > + if self.auth: > > + request.add_unredirected_header(self.auth_header, self.auth) > > + > > + return request > > + > > def reset_retry_count(self): > > # Python 2.6.5 will call this on 401 or 407 errors and thus loop > > # forever. We disable reset_retry_count completely and reset in > > @@ -435,6 +449,19 @@ > > return urllib2.HTTPBasicAuthHandler.http_error_auth_reqed( > > self, auth_header, host, req, headers) > > > > + def retry_http_basic_auth(self, host, req, realm): > > + user, pw = self.passwd.find_user_password(realm, host) > > + if pw is not None: > > + raw = "%s:%s" % (user, pw) > > + auth = 'Basic %s' % base64.b64encode(raw).strip() > > + if req.headers.get(self.auth_header, None) == auth: > > + return None > > + self.auth = auth > > + req.add_unredirected_header(self.auth_header, auth) > > + return self.parent.open(req, timeout=req.timeout) > > + else: > > + return None > > + > > handlerfuncs = [] > > > > def opener(ui, authinfo=None): > > > > -- > > Stéphane Klein <sklein@bearstech.com> > > GSM : 06 61 48 76 04 > > > > _______________________________________________ > > Mercurial-devel mailing list > > Mercurial-devel@selenic.com > > http://selenic.com/mailman/listinfo/mercurial-devel > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
Aaaand I just backed it out, thanks to terrible breakage on Python 2.4. Sigh. Patch works great on 2.6 and 2.7, disaster on 2.4. On Dec 13, 2013, at 12:48 PM, Augie Fackler <raf@durin42.com> wrote: > On Tue, Dec 10, 2013 at 04:22:39PM -0500, Augie Fackler wrote: >> On Mon, Nov 25, 2013 at 05:22:10PM +0100, Stéphane Klein wrote: >>> # HG changeset patch >>> # User Stéphane Klein <contact@stephane-klein.info> >>> # Date 1385396292 -3600 >>> # Mon Nov 25 17:18:12 2013 +0100 >>> # Node ID ec1afe2354a840eb47a2286308fb5a5eeed91eba >>> # Parent 1c46b18b0e1c47fa4cecf21b78c083a54ae9903f >>> Keep authentication information after the first fail HTTP access (issue3567) >> >> Hearing no objections after a couple of weeks, I'm queueing this. It >> seems reasonable. > > (but, I should mention, with some tweaks to the log message to make it > more compliant with our format and a minor English tweak) > >> >>> >>> Context : mercurial access to repository server with http access, and this >>> server is protected by basic auth. >>> >>> Before patch : >>> >>> * mercurial try an anonymous access to server, server return 401 response and >>> mercurial resend request with login / password information >>> >>> After patch : >>> >>> * mercurial try an anonymous access to server, server return >>> 401 response. For all next requests, mercurial keep in memory this >>> information (this server need basic auth information). >>> >>> This patch reduce the number of http access against mercurial server. >>> >>> Example, before patch : >>> >>> 10.10.168.170 - - [25/Oct/2013:15:44:51 +0200] "GET /hg/testagt?cmd=capabilities >>> HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:44:52 +0200] "GET /hg/testagt?cmd=capabilities >>> HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:45:00 +0200] "GET /hg/testagt?cmd=capabilities >>> HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:45:01 +0200] "GET /hg/testagt?cmd=capabilities >>> HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:45:03 +0200] "GET /hg/testagt?cmd=batch >>> HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:45:04 +0200] "GET /hg/testagt?cmd=batch >>> HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:45:06 +0200] "GET /hg/testagt?cmd=getbundle >>> HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:45:07 +0200] "GET /hg/testagt?cmd=getbundle >>> HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:45:09 +0200] "GET /hg/testagt?cmd=listkeys >>> HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:45:10 +0200] "GET /hg/testagt?cmd=listkeys >>> HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys >>> HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys >>> HTTP/1.1" 200 - "-" "mercurial/proto-1.0" >>> >>> Example after patch : >>> >>> 10.10.168.170 - - [28/Oct/2013:11:49:14 +0100] "GET /hg/testagt?cmd=capabilities >>> HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [28/Oct/2013:11:49:15 +0100] "GET /hg/testagt?cmd=capabilities >>> HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [28/Oct/2013:11:49:17 +0100] "GET /hg/testagt?cmd=batch >>> HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [28/Oct/2013:11:49:19 +0100] "GET /hg/testagt?cmd=getbundle >>> HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [28/Oct/2013:11:49:22 +0100] "GET /hg/testagt?cmd=listkeys >>> HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" >>> 10.10.168.170 - - [28/Oct/2013:11:49:24 +0100] "GET /hg/testagt?cmd=listkeys >>> HTTP/1.1" 200 - "-" "mercurial/proto-1.0" >>> >>> In this last example, you can see only one 401 response. >>> >>> diff -r 1c46b18b0e1c -r ec1afe2354a8 mercurial/url.py >>> --- a/mercurial/url.py Fri Nov 22 17:26:58 2013 -0600 >>> +++ b/mercurial/url.py Mon Nov 25 17:18:12 2013 +0100 >>> @@ -8,6 +8,7 @@ >>> # GNU General Public License version 2 or any later version. >>> >>> import urllib, urllib2, httplib, os, socket, cStringIO >>> +import base64 >>> from i18n import _ >>> import keepalive, util, sslutil >>> import httpconnection as httpconnectionmod >>> @@ -418,9 +419,22 @@ >>> >>> class httpbasicauthhandler(urllib2.HTTPBasicAuthHandler): >>> def __init__(self, *args, **kwargs): >>> + self.auth = None >>> urllib2.HTTPBasicAuthHandler.__init__(self, *args, **kwargs) >>> self.retried_req = None >>> >>> + def http_request(self, request): >>> + if self.auth: >>> + request.add_unredirected_header(self.auth_header, self.auth) >>> + >>> + return request >>> + >>> + def https_request(self, request): >>> + if self.auth: >>> + request.add_unredirected_header(self.auth_header, self.auth) >>> + >>> + return request >>> + >>> def reset_retry_count(self): >>> # Python 2.6.5 will call this on 401 or 407 errors and thus loop >>> # forever. We disable reset_retry_count completely and reset in >>> @@ -435,6 +449,19 @@ >>> return urllib2.HTTPBasicAuthHandler.http_error_auth_reqed( >>> self, auth_header, host, req, headers) >>> >>> + def retry_http_basic_auth(self, host, req, realm): >>> + user, pw = self.passwd.find_user_password(realm, host) >>> + if pw is not None: >>> + raw = "%s:%s" % (user, pw) >>> + auth = 'Basic %s' % base64.b64encode(raw).strip() >>> + if req.headers.get(self.auth_header, None) == auth: >>> + return None >>> + self.auth = auth >>> + req.add_unredirected_header(self.auth_header, auth) >>> + return self.parent.open(req, timeout=req.timeout) >>> + else: >>> + return None >>> + >>> handlerfuncs = [] >>> >>> def opener(ui, authinfo=None): >>> >>> -- >>> Stéphane Klein <sklein@bearstech.com> >>> GSM : 06 61 48 76 04 >>> >>> _______________________________________________ >>> Mercurial-devel mailing list >>> Mercurial-devel@selenic.com >>> http://selenic.com/mailman/listinfo/mercurial-devel >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@selenic.com >> http://selenic.com/mailman/listinfo/mercurial-devel > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Le 14/12/13 18:49, Augie Fackler a écrit : > Aaaand I just backed it out, thanks to terrible breakage on Python 2.4. > > Sigh. Patch works great on 2.6 and 2.7, disaster on 2.4. I'm going to work on this task. - -- Stéphane Klein <sklein@bearstech.com> GSM : 06 61 48 76 04 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (Darwin) Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlKzDMkACgkQU9O4HJsr4YzZpgEAwlcseHAn1g9ftMQ2Uf5S4kBm MjvCFNMj9HDalOtYqhQA/06yoOv0N4oUyQ2ddGgXIgciEZ1r2sbdpZ7S6sHFMHzr =M97G -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Le 14/12/13 18:49, Augie Fackler a écrit : > Aaaand I just backed it out, thanks to terrible breakage on Python 2.4. > > Sigh. Patch works great on 2.6 and 2.7, disaster on 2.4. This is a new version with python 2.4 bugfix. # HG changeset patch # User Stéphane Klein <contact@stephane-klein.info> # Date 1387547765 -3600 # Fri Dec 20 14:56:05 2013 +0100 # Node ID ec0ec32a305f6626ccd9b79e4874ffbcd2e5ecd8 # Parent 04036798ebed0c6d7062517bb49b308a15e4345e http: reuse authentication info after the first failed request (issue3567) diff -r 04036798ebed -r ec0ec32a305f mercurial/url.py - --- a/mercurial/url.py Fri Nov 15 23:18:08 2013 -0500 +++ b/mercurial/url.py Fri Dec 20 14:56:05 2013 +0100 @@ -8,6 +8,7 @@ # GNU General Public License version 2 or any later version. import urllib, urllib2, httplib, os, socket, cStringIO +import base64 from i18n import _ import keepalive, util, sslutil import httpconnection as httpconnectionmod @@ -418,9 +419,22 @@ class httpbasicauthhandler(urllib2.HTTPBasicAuthHandler): def __init__(self, *args, **kwargs): + self.auth = None urllib2.HTTPBasicAuthHandler.__init__(self, *args, **kwargs) self.retried_req = None + def http_request(self, request): + if self.auth: + request.add_unredirected_header(self.auth_header, self.auth) + + return request + + def https_request(self, request): + if self.auth: + request.add_unredirected_header(self.auth_header, self.auth) + + return request + def reset_retry_count(self): # Python 2.6.5 will call this on 401 or 407 errors and thus loop # forever. We disable reset_retry_count completely and reset in @@ -435,6 +449,19 @@ return urllib2.HTTPBasicAuthHandler.http_error_auth_reqed( self, auth_header, host, req, headers) + def retry_http_basic_auth(self, host, req, realm): + user, pw = self.passwd.find_user_password(realm, host) + if pw is not None: + raw = "%s:%s" % (user, pw) + auth = 'Basic %s' % base64.b64encode(raw).strip() + if req.headers.get(self.auth_header, None) == auth: + return None + self.auth = auth + req.add_unredirected_header(self.auth_header, auth) + return self.parent.open(req) + else: + return None + handlerfuncs = [] def opener(ui, authinfo=None): - -- Stéphane Klein <sklein@bearstech.com> GSM : 06 61 48 76 04 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (Darwin) Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlK0TaoACgkQU9O4HJsr4Ywv3QEAr3jIgqCBKKhAGTiqqbtA8Ty0 GYkzJCy3W78eR5I/cWQBAKDSfadY+NnQtRM/Sg1ywVUre8shdsZxPtSTzBiwUGi3 =He/J -----END PGP SIGNATURE-----
On Fri, 20 Dec 2013 18:01:14 +0400, Stéphane Klein <sklein@bearstech.com> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > Le 14/12/13 18:49, Augie Fackler a écrit : >> Aaaand I just backed it out, thanks to terrible breakage on Python 2.4. >> >> Sigh. Patch works great on 2.6 and 2.7, disaster on 2.4. > > This is a new version with python 2.4 bugfix. > > # HG changeset patch > # User Stéphane Klein <contact@stephane-klein.info> > # Date 1387547765 -3600 > # Fri Dec 20 14:56:05 2013 +0100 > # Node ID ec0ec32a305f6626ccd9b79e4874ffbcd2e5ecd8 > # Parent 04036798ebed0c6d7062517bb49b308a15e4345e > http: reuse authentication info after the first failed request > (issue3567) I'm sure there were a lot more lines of description here in the first version. And I believe those were useful lines nobody objected to. > diff -r 04036798ebed -r ec0ec32a305f mercurial/url.py > - --- a/mercurial/url.py Fri Nov 15 23:18:08 2013 -0500 > +++ b/mercurial/url.py Fri Dec 20 14:56:05 2013 +0100 > @@ -8,6 +8,7 @@ > # GNU General Public License version 2 or any later version. > > import urllib, urllib2, httplib, os, socket, cStringIO > +import base64 > from i18n import _ > import keepalive, util, sslutil > import httpconnection as httpconnectionmod > @@ -418,9 +419,22 @@ > > class httpbasicauthhandler(urllib2.HTTPBasicAuthHandler): > def __init__(self, *args, **kwargs): > + self.auth = None > urllib2.HTTPBasicAuthHandler.__init__(self, *args, **kwargs) > self.retried_req = None > > + def http_request(self, request): > + if self.auth: > + request.add_unredirected_header(self.auth_header, self.auth) > + > + return request > + > + def https_request(self, request): > + if self.auth: > + request.add_unredirected_header(self.auth_header, self.auth) > + > + return request > + > def reset_retry_count(self): > # Python 2.6.5 will call this on 401 or 407 errors and thus loop > # forever. We disable reset_retry_count completely and reset in > @@ -435,6 +449,19 @@ > return urllib2.HTTPBasicAuthHandler.http_error_auth_reqed( > self, auth_header, host, req, headers) > > + def retry_http_basic_auth(self, host, req, realm): > + user, pw = self.passwd.find_user_password(realm, host) > + if pw is not None: > + raw = "%s:%s" % (user, pw) > + auth = 'Basic %s' % base64.b64encode(raw).strip() > + if req.headers.get(self.auth_header, None) == auth: > + return None > + self.auth = auth > + req.add_unredirected_header(self.auth_header, auth) > + return self.parent.open(req) > + else: > + return None > + > handlerfuncs = [] > > def opener(ui, authinfo=None): > > > - -- > Stéphane Klein <sklein@bearstech.com> > GSM : 06 61 48 76 04 > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.12 (Darwin) > Comment: GPGTools - http://gpgtools.org > Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ > > iF4EAREIAAYFAlK0TaoACgkQU9O4HJsr4Ywv3QEAr3jIgqCBKKhAGTiqqbtA8Ty0 > GYkzJCy3W78eR5I/cWQBAKDSfadY+NnQtRM/Sg1ywVUre8shdsZxPtSTzBiwUGi3 > =He/J > -----END PGP SIGNATURE----- > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Le 20/12/13 15:20, Nikolaj Sjujskij a écrit : > On Fri, 20 Dec 2013 18:01:14 +0400, Stéphane Klein <sklein@bearstech.com> > wrote: > > Le 14/12/13 18:49, Augie Fackler a écrit : >>>> Aaaand I just backed it out, thanks to terrible breakage on Python >>>> 2.4. >>>> >>>> Sigh. Patch works great on 2.6 and 2.7, disaster on 2.4. > > This is a new version with python 2.4 bugfix. > > # HG changeset patch # User Stéphane Klein <contact@stephane-klein.info> # > Date 1387547765 -3600 # Fri Dec 20 14:56:05 2013 +0100 # Node ID > ec0ec32a305f6626ccd9b79e4874ffbcd2e5ecd8 # Parent > 04036798ebed0c6d7062517bb49b308a15e4345e http: reuse authentication info > after the first failed request (issue3567) >> I'm sure there were a lot more lines of description here in the first >> version. And I believe those were useful lines nobody objected to. > I need to resend the full commit message in my patch ? - -- Stéphane Klein <sklein@bearstech.com> GSM : 06 61 48 76 04 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (Darwin) Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlK0VJ8ACgkQU9O4HJsr4YyG6QEAm5IPDZeBmMZ9MUqV4qDOOWkh 7GYBRTJFAybeUfSh2sMA/jpU9+gE4PzbjeGsAMPhqXv4CuaEeughmgVID6Da0HQd =9b9q -----END PGP SIGNATURE-----
On Dec 20, 2013, at 9:30 AM, Stéphane Klein <sklein@bearstech.com> wrote:
> I need to resend the full commit message in my patch ?
yes please
On Fri, 20 Dec 2013 18:30:55 +0400, Stéphane Klein <sklein@bearstech.com> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > Le 20/12/13 15:20, Nikolaj Sjujskij a écrit : >> On Fri, 20 Dec 2013 18:01:14 +0400, Stéphane Klein >> <sklein@bearstech.com> >> wrote: >> >> Le 14/12/13 18:49, Augie Fackler a écrit : >>>>> Aaaand I just backed it out, thanks to terrible breakage on Python >>>>> 2.4. >>>>> >>>>> Sigh. Patch works great on 2.6 and 2.7, disaster on 2.4. >> >> This is a new version with python 2.4 bugfix. >> >> # HG changeset patch # User Stéphane Klein >> <contact@stephane-klein.info> # >> Date 1387547765 -3600 # Fri Dec 20 14:56:05 2013 +0100 # Node ID >> ec0ec32a305f6626ccd9b79e4874ffbcd2e5ecd8 # Parent >> 04036798ebed0c6d7062517bb49b308a15e4345e http: reuse authentication info >> after the first failed request (issue3567) >>> I'm sure there were a lot more lines of description here in the first >>> version. And I believe those were useful lines nobody objected to. >> > > I need to resend the full commit message in my patch ? Well, that commit message was a part of patch you've been working on. I wasn't sure why you have stripped it, surely you don't expect "applyer" to cut'n'paste description from earlier version to craft a changeset himself. Basically you were expected to resend the same patch with issues fixed (Python 2.4 support, the first line/summary). And nobody suggested stripping that description (because it's a good and robust one). So I pointed out its absence in case I've missed something :) Thanks for contributing!
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
# HG changeset patch
# User Stéphane Klein <contact@stephane-klein.info>
# Date 1388413356 -3600
# Mon Dec 30 15:22:36 2013 +0100
# Node ID 9063db5432d6c6c75491e759861a373bc493822a
# Parent 4274eda143cb1025be1130ffdaaf62370a2a6961
Keep authentication information after the first fail HTTP access (issue3567)
Context : mercurial access to repository server with http access, and this
server is protected by basic auth.
Before patch :
* mercurial try an anonymous access to server, server return 401 response and
mercurial resend request with login / password information
After patch :
* mercurial try an anonymous access to server, server return
401 response. For all next requests, mercurial keep in memory this
information (this server need basic auth information).
This patch reduce the number of http access against mercurial server.
Example, before patch :
10.10.168.170 - - [25/Oct/2013:15:44:51 +0200] "GET /hg/testagt?cmd=capabilities
HTTP/1.1" 401 260 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:44:52 +0200] "GET /hg/testagt?cmd=capabilities
HTTP/1.1" 200 147 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:45:00 +0200] "GET /hg/testagt?cmd=capabilities
HTTP/1.1" 401 260 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:45:01 +0200] "GET /hg/testagt?cmd=capabilities
HTTP/1.1" 200 147 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:45:03 +0200] "GET /hg/testagt?cmd=batch
HTTP/1.1" 401 260 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:45:04 +0200] "GET /hg/testagt?cmd=batch
HTTP/1.1" 200 42 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:45:06 +0200] "GET /hg/testagt?cmd=getbundle
HTTP/1.1" 401 260 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:45:07 +0200] "GET /hg/testagt?cmd=getbundle
HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:45:09 +0200] "GET /hg/testagt?cmd=listkeys
HTTP/1.1" 401 260 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:45:10 +0200] "GET /hg/testagt?cmd=listkeys
HTTP/1.1" 200 15 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys
HTTP/1.1" 401 260 "-" "mercurial/proto-1.0"
10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys
HTTP/1.1" 200 - "-" "mercurial/proto-1.0"
Example after patch :
10.10.168.170 - - [28/Oct/2013:11:49:14 +0100] "GET /hg/testagt?cmd=capabilities
HTTP/1.1" 401 260 "-" "mercurial/proto-1.0"
10.10.168.170 - - [28/Oct/2013:11:49:15 +0100] "GET /hg/testagt?cmd=capabilities
HTTP/1.1" 200 147 "-" "mercurial/proto-1.0"
10.10.168.170 - - [28/Oct/2013:11:49:17 +0100] "GET /hg/testagt?cmd=batch
HTTP/1.1" 200 42 "-" "mercurial/proto-1.0"
10.10.168.170 - - [28/Oct/2013:11:49:19 +0100] "GET /hg/testagt?cmd=getbundle
HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0"
10.10.168.170 - - [28/Oct/2013:11:49:22 +0100] "GET /hg/testagt?cmd=listkeys
HTTP/1.1" 200 15 "-" "mercurial/proto-1.0"
10.10.168.170 - - [28/Oct/2013:11:49:24 +0100] "GET /hg/testagt?cmd=listkeys
HTTP/1.1" 200 - "-" "mercurial/proto-1.0"
In this last example, you can see only one 401 response.
diff -r 4274eda143cb -r 9063db5432d6 mercurial/url.py
- --- a/mercurial/url.py Mon Sep 16 01:08:29 2013 -0700
+++ b/mercurial/url.py Mon Dec 30 15:22:36 2013 +0100
@@ -8,6 +8,7 @@
# GNU General Public License version 2 or any later version.
import urllib, urllib2, httplib, os, socket, cStringIO
+import base64
from i18n import _
import keepalive, util, sslutil
import httpconnection as httpconnectionmod
@@ -418,9 +419,22 @@
class httpbasicauthhandler(urllib2.HTTPBasicAuthHandler):
def __init__(self, *args, **kwargs):
+ self.auth = None
urllib2.HTTPBasicAuthHandler.__init__(self, *args, **kwargs)
self.retried_req = None
+ def http_request(self, request):
+ if self.auth:
+ request.add_unredirected_header(self.auth_header, self.auth)
+
+ return request
+
+ def https_request(self, request):
+ if self.auth:
+ request.add_unredirected_header(self.auth_header, self.auth)
+
+ return request
+
def reset_retry_count(self):
# Python 2.6.5 will call this on 401 or 407 errors and thus loop
# forever. We disable reset_retry_count completely and reset in
@@ -435,6 +449,19 @@
return urllib2.HTTPBasicAuthHandler.http_error_auth_reqed(
self, auth_header, host, req, headers)
+ def retry_http_basic_auth(self, host, req, realm):
+ user, pw = self.passwd.find_user_password(realm, req.get_full_url())
+ if pw is not None:
+ raw = "%s:%s" % (user, pw)
+ auth = 'Basic %s' % base64.b64encode(raw).strip()
+ if req.headers.get(self.auth_header, None) == auth:
+ return None
+ self.auth = auth
+ req.add_header(self.auth_header, auth)
+ return self.parent.open(req)
+ else:
+ return None
+
handlerfuncs = []
def opener(ui, authinfo=None):
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iF4EAREIAAYFAlLBgr0ACgkQU9O4HJsr4YyHrgD8D53D5ch3lXNMBY6u06MMCUL3
Di2nnU+ozINRP4hS56UA/RKm7kf4eUzcXBkUaI/wxYB0SL4Z5h3tNrEsYSk9SQyZ
=AFUv
-----END PGP SIGNATURE-----
On Mon, Dec 30, 2013 at 03:27:09PM +0100, Stéphane Klein wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > # HG changeset patch > # User Stéphane Klein <contact@stephane-klein.info> > # Date 1388413356 -3600 > # Mon Dec 30 15:22:36 2013 +0100 > # Node ID 9063db5432d6c6c75491e759861a373bc493822a > # Parent 4274eda143cb1025be1130ffdaaf62370a2a6961 > Keep authentication information after the first fail HTTP access (issue3567) > Looks reasonable here. Can someone run the testsuite for this on 2.4 and 2.7 and verify it works? I don't have ready access to anything with Python 2.4 anymore. > > Context : mercurial access to repository server with http access, and this > server is protected by basic auth. > > Before patch : > > * mercurial try an anonymous access to server, server return 401 response and > mercurial resend request with login / password information > > After patch : > > * mercurial try an anonymous access to server, server return > 401 response. For all next requests, mercurial keep in memory this > information (this server need basic auth information). > > This patch reduce the number of http access against mercurial server. > > Example, before patch : > > 10.10.168.170 - - [25/Oct/2013:15:44:51 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:44:52 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:00 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:01 +0200] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:03 +0200] "GET /hg/testagt?cmd=batch > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:04 +0200] "GET /hg/testagt?cmd=batch > HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:06 +0200] "GET /hg/testagt?cmd=getbundle > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:07 +0200] "GET /hg/testagt?cmd=getbundle > HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:09 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:10 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 - "-" "mercurial/proto-1.0" > > Example after patch : > > 10.10.168.170 - - [28/Oct/2013:11:49:14 +0100] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:15 +0100] "GET /hg/testagt?cmd=capabilities > HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:17 +0100] "GET /hg/testagt?cmd=batch > HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:19 +0100] "GET /hg/testagt?cmd=getbundle > HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:22 +0100] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" > 10.10.168.170 - - [28/Oct/2013:11:49:24 +0100] "GET /hg/testagt?cmd=listkeys > HTTP/1.1" 200 - "-" "mercurial/proto-1.0" > > In this last example, you can see only one 401 response. > > diff -r 4274eda143cb -r 9063db5432d6 mercurial/url.py > - --- a/mercurial/url.py Mon Sep 16 01:08:29 2013 -0700 > +++ b/mercurial/url.py Mon Dec 30 15:22:36 2013 +0100 > @@ -8,6 +8,7 @@ > # GNU General Public License version 2 or any later version. > > import urllib, urllib2, httplib, os, socket, cStringIO > +import base64 > from i18n import _ > import keepalive, util, sslutil > import httpconnection as httpconnectionmod > @@ -418,9 +419,22 @@ > > class httpbasicauthhandler(urllib2.HTTPBasicAuthHandler): > def __init__(self, *args, **kwargs): > + self.auth = None > urllib2.HTTPBasicAuthHandler.__init__(self, *args, **kwargs) > self.retried_req = None > > + def http_request(self, request): > + if self.auth: > + request.add_unredirected_header(self.auth_header, self.auth) > + > + return request > + > + def https_request(self, request): > + if self.auth: > + request.add_unredirected_header(self.auth_header, self.auth) > + > + return request > + > def reset_retry_count(self): > # Python 2.6.5 will call this on 401 or 407 errors and thus loop > # forever. We disable reset_retry_count completely and reset in > @@ -435,6 +449,19 @@ > return urllib2.HTTPBasicAuthHandler.http_error_auth_reqed( > self, auth_header, host, req, headers) > > + def retry_http_basic_auth(self, host, req, realm): > + user, pw = self.passwd.find_user_password(realm, req.get_full_url()) > + if pw is not None: > + raw = "%s:%s" % (user, pw) > + auth = 'Basic %s' % base64.b64encode(raw).strip() > + if req.headers.get(self.auth_header, None) == auth: > + return None > + self.auth = auth > + req.add_header(self.auth_header, auth) > + return self.parent.open(req) > + else: > + return None > + > handlerfuncs = [] > > def opener(ui, authinfo=None): > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.12 (Darwin) > Comment: GPGTools - http://gpgtools.org > Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ > > iF4EAREIAAYFAlLBgr0ACgkQU9O4HJsr4YyHrgD8D53D5ch3lXNMBY6u06MMCUL3 > Di2nnU+ozINRP4hS56UA/RKm7kf4eUzcXBkUaI/wxYB0SL4Z5h3tNrEsYSk9SQyZ > =AFUv > -----END PGP SIGNATURE----- > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Le 02/01/14 00:20, Augie Fackler a écrit : > >> Looks reasonable here. Can someone run the testsuite for this on 2.4 and >> 2.7 and verify it works? I don't have ready access to anything with >> Python 2.4 anymore. > > Someone to execute this test on Python 2.4 ? Best regards, Stephane - -- Stéphane Klein <sklein@bearstech.com> GSM : 06 61 48 76 04 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (Darwin) Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLX80EACgkQU9O4HJsr4YzstQD/ZBFYCTWUOAS7ntwd3sdr1RYc glKxgfuuaMgqmPZLZNcA/1SuN/tpf70vyRljiQgeQE3UmsCZ9eQ63FMBaT3b/vWv =5159 -----END PGP SIGNATURE-----
On 01/02/2014 12:20 AM, Augie Fackler wrote: > On Mon, Dec 30, 2013 at 03:27:09PM +0100, Stéphane Klein wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA256 >> >> # HG changeset patch >> # User Stéphane Klein <contact@stephane-klein.info> >> # Date 1388413356 -3600 >> # Mon Dec 30 15:22:36 2013 +0100 >> # Node ID 9063db5432d6c6c75491e759861a373bc493822a >> # Parent 4274eda143cb1025be1130ffdaaf62370a2a6961 >> Keep authentication information after the first fail HTTP access (issue3567) >> > Looks reasonable here. Can someone run the testsuite for this on 2.4 > and 2.7 and verify it works? I don't have ready access to anything > with Python 2.4 anymore. Tested ... and it failed. First meta failure is that the patch didn't show any test coverage. test-http.t and python 2.4 failed with this patch ... but it has been failing for years as mentioned on http://bz.selenic.com/show_bug.cgi?id=2739#c17 . python 2.4.3 showed similar failures. They can be fixed by using req.get_full_url() instead of host, like in http://hg.python.org/cpython/rev/ac02f655b31e . I haven't investigated why it fixes a problem - and why Python apparently backed the change out again. But testing with test-http.t with extra test coverage of this change and on 2.4 2.4.3 2.4.6 2.5 2.5.6 2.6 2.6.1 2.6.2 2.6.9 2.7 2.7.6 shows that it works. I just sent a series that includes the updated version of this patch and the prerequisites. With these changes the patch looks good to me. It fixes an annoying problem that has been around forever. It is a hack, but that is how we usually make urllib2 usable. It also takes a nice step in the direction of including a full urllib2 so we can fix issues without problems with patching of different stdlib versions. ;-) /Mads
Le 20/01/14 01:52, Mads Kiilerich a écrit : > With these changes the patch looks good to me. It fixes an annoying problem that > has been around forever. Do you mean that you have fixed my patch or do I need to resend my patch with this change ? in other word : shall I do something to help to "push this patch" ? Best regards, Stephane
On 01/20/2014 04:57 PM, Stéphane Klein wrote: > Le 20/01/14 01:52, Mads Kiilerich a écrit : >> With these changes the patch looks good to me. It fixes an annoying problem that >> has been around forever. > Do you mean that you have fixed my patch or do I need to resend my patch with > this change ? in other word : shall I do something to help to "push this patch" ? I do think that http://selenic.com/pipermail/mercurial-devel/2014-January/055766.html should be enough. But if you disagree or would like to improve on it then please take it back or start from your latest version or something. You can probably help pushing this patch forward by doing some history digging in the Python repo and see why http://hg.python.org/cpython/rev/ac02f655b31e was backed out again. It would be nice to have a compelling argument why my change is good or bad. http://bz.selenic.com/show_bug.cgi?id=2739 can perhaps also provide some background information. /Mads
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Le 07/02/14 03:09, Mads Kiilerich a écrit : > On 01/20/2014 05:15 PM, Mads Kiilerich wrote: >> On 01/20/2014 04:57 PM, Stéphane Klein wrote: >>> Le 20/01/14 01:52, Mads Kiilerich a écrit : >>>> With these changes the patch looks good to me. It fixes an annoying >>>> problem that has been around forever. >>> Do you mean that you have fixed my patch or do I need to resend my >>> patch with this change ? in other word : shall I do something to help >>> to "push this patch" ? >> >> I do think that >> http://selenic.com/pipermail/mercurial-devel/2014-January/055766.html >> should be enough. But if you disagree or would like to improve on it then >> please take it back or start from your latest version or something. >> >> You can probably help pushing this patch forward by doing some history >> digging in the Python repo and see why >> http://hg.python.org/cpython/rev/ac02f655b31e was backed out again. It >> would be nice to have a compelling argument why my change is good or bad. >> http://bz.selenic.com/show_bug.cgi?id=2739 can perhaps also provide some >> background information. > > Your patch didn't make it for 2.9. :-( > > My test that will show that it works is now in 'default'. > > Please resend for 'default' with corresponding test update ... or aim at > 'stable' again if you feel adventurous. > > /Mads > # HG changeset patch # User Stéphane Klein <contact@stephane-klein.info> # Date 1392643459 -3600 # Mon Feb 17 14:24:19 2014 +0100 # Node ID e64f75d26305e44f3136db6d147385f94f043134 # Parent 0e2877f8605dcaf4fdf2ab7e0046f1f6f80161dd http: reuse authentication info after the first failed request (issue3567) Context : mercurial access to repository server with http access, and this server is protected by basic auth. Before patch : * mercurial try an anonymous access to server, server return 401 response and mercurial resend request with login / password information After patch : * mercurial try an anonymous access to server, server return 401 response. For all next requests, mercurial keep in memory this information (this server need basic auth information). This patch reduce the number of http access against mercurial server. Example, before patch : 10.10.168.170 - - [25/Oct/2013:15:44:51 +0200] "GET /hg/testagt?cmd=capabilities HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:44:52 +0200] "GET /hg/testagt?cmd=capabilities HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:45:00 +0200] "GET /hg/testagt?cmd=capabilities HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:45:01 +0200] "GET /hg/testagt?cmd=capabilities HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:45:03 +0200] "GET /hg/testagt?cmd=batch HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:45:04 +0200] "GET /hg/testagt?cmd=batch HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:45:06 +0200] "GET /hg/testagt?cmd=getbundle HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:45:07 +0200] "GET /hg/testagt?cmd=getbundle HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:45:09 +0200] "GET /hg/testagt?cmd=listkeys HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:45:10 +0200] "GET /hg/testagt?cmd=listkeys HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" 10.10.168.170 - - [25/Oct/2013:15:45:12 +0200] "GET /hg/testagt?cmd=listkeys HTTP/1.1" 200 - "-" "mercurial/proto-1.0" Example after patch : 10.10.168.170 - - [28/Oct/2013:11:49:14 +0100] "GET /hg/testagt?cmd=capabilities HTTP/1.1" 401 260 "-" "mercurial/proto-1.0" 10.10.168.170 - - [28/Oct/2013:11:49:15 +0100] "GET /hg/testagt?cmd=capabilities HTTP/1.1" 200 147 "-" "mercurial/proto-1.0" 10.10.168.170 - - [28/Oct/2013:11:49:17 +0100] "GET /hg/testagt?cmd=batch HTTP/1.1" 200 42 "-" "mercurial/proto-1.0" 10.10.168.170 - - [28/Oct/2013:11:49:19 +0100] "GET /hg/testagt?cmd=getbundle HTTP/1.1" 200 61184 "-" "mercurial/proto-1.0" 10.10.168.170 - - [28/Oct/2013:11:49:22 +0100] "GET /hg/testagt?cmd=listkeys HTTP/1.1" 200 15 "-" "mercurial/proto-1.0" 10.10.168.170 - - [28/Oct/2013:11:49:24 +0100] "GET /hg/testagt?cmd=listkeys HTTP/1.1" 200 - "-" "mercurial/proto-1.0" In this last example, you can see only one 401 response. diff -r 0e2877f8605d -r e64f75d26305 mercurial/url.py - --- a/mercurial/url.py Sat Feb 15 22:09:32 2014 -0600 +++ b/mercurial/url.py Mon Feb 17 14:24:19 2014 +0100 @@ -8,6 +8,7 @@ # GNU General Public License version 2 or any later version. import urllib, urllib2, httplib, os, socket, cStringIO +import base64 from i18n import _ import keepalive, util, sslutil import httpconnection as httpconnectionmod @@ -422,9 +423,22 @@ class httpbasicauthhandler(urllib2.HTTPBasicAuthHandler): def __init__(self, *args, **kwargs): + self.auth = None urllib2.HTTPBasicAuthHandler.__init__(self, *args, **kwargs) self.retried_req = None + def http_request(self, request): + if self.auth: + request.add_unredirected_header(self.auth_header, self.auth) + + return request + + def https_request(self, request): + if self.auth: + request.add_unredirected_header(self.auth_header, self.auth) + + return request + def reset_retry_count(self): # Python 2.6.5 will call this on 401 or 407 errors and thus loop # forever. We disable reset_retry_count completely and reset in @@ -439,6 +453,19 @@ return urllib2.HTTPBasicAuthHandler.http_error_auth_reqed( self, auth_header, host, req, headers) + def retry_http_basic_auth(self, host, req, realm): + user, pw = self.passwd.find_user_password(realm, req.get_full_url()) + if pw is not None: + raw = "%s:%s" % (user, pw) + auth = 'Basic %s' % base64.b64encode(raw).strip() + if req.headers.get(self.auth_header, None) == auth: + return None + self.auth = auth + req.add_header(self.auth_header, auth) + return self.parent.open(req) + else: + return None + handlerfuncs = [] - -- Stéphane Klein <sklein@bearstech.com> GSM : 06 61 48 76 04 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (Darwin) Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlMCDlkACgkQU9O4HJsr4YzYpgEAi0XiApZuBBEUbhic5VjKmoTD 9VvrU+2CeF3/Fqh5HFIBAMdTZ8uay1qMei84+jOZmEPIdlqiJU7U1i+TVZgW2BvQ =Gxkk -----END PGP SIGNATURE-----
On 02/17/2014 02:27 PM, Stéphane Klein wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > Le 07/02/14 03:09, Mads Kiilerich a écrit : >> On 01/20/2014 05:15 PM, Mads Kiilerich wrote: >> Your patch didn't make it for 2.9. :-( My test that will show that it >> works is now in 'default'. Please resend for 'default' with >> corresponding test update ... or aim at 'stable' again if you feel >> adventurous. > > # HG changeset patch > # User Stéphane Klein <contact@stephane-klein.info> > # Date 1392643459 -3600 > # Mon Feb 17 14:24:19 2014 +0100 > # Node ID e64f75d26305e44f3136db6d147385f94f043134 > # Parent 0e2877f8605dcaf4fdf2ab7e0046f1f6f80161dd > http: reuse authentication info after the first failed request (issue3567) Meanwhile we have gotten better test coverage of this in 'default'. The patch should include the necessary changes to the test so it still pass (http://mercurial.selenic.com/wiki/WritingTests ). That will prove that a part of what is described in the commit message actually is true (and perhaps don't have to be described in that much detail.) A comment in the commit message telling which Python versions it has been tested on would also be nice. (contrib/Makefile.python might be useful.) /Mads
Patch
diff -r 1c46b18b0e1c -r ec1afe2354a8 mercurial/url.py --- a/mercurial/url.py Fri Nov 22 17:26:58 2013 -0600 +++ b/mercurial/url.py Mon Nov 25 17:18:12 2013 +0100 @@ -8,6 +8,7 @@ # GNU General Public License version 2 or any later version. import urllib, urllib2, httplib, os, socket, cStringIO +import base64 from i18n import _ import keepalive, util, sslutil import httpconnection as httpconnectionmod @@ -418,9 +419,22 @@ class httpbasicauthhandler(urllib2.HTTPBasicAuthHandler): def __init__(self, *args, **kwargs): + self.auth = None urllib2.HTTPBasicAuthHandler.__init__(self, *args, **kwargs) self.retried_req = None + def http_request(self, request): + if self.auth: + request.add_unredirected_header(self.auth_header, self.auth) + + return request + + def https_request(self, request): + if self.auth: + request.add_unredirected_header(self.auth_header, self.auth) + + return request + def reset_retry_count(self): # Python 2.6.5 will call this on 401 or 407 errors and thus loop # forever. We disable reset_retry_count completely and reset in @@ -435,6 +449,19 @@ return urllib2.HTTPBasicAuthHandler.http_error_auth_reqed( self, auth_header, host, req, headers) + def retry_http_basic_auth(self, host, req, realm): + user, pw = self.passwd.find_user_password(realm, host) + if pw is not None: + raw = "%s:%s" % (user, pw) + auth = 'Basic %s' % base64.b64encode(raw).strip() + if req.headers.get(self.auth_header, None) == auth: + return None + self.auth = auth + req.add_unredirected_header(self.auth_header, auth) + return self.parent.open(req, timeout=req.timeout) + else: + return None + handlerfuncs = [] def opener(ui, authinfo=None):