Patchwork url: remove unnecessary deletion of environ variables while dealing with proxy

login
register
mail settings
Submitter Pulkit Goyal
Date Dec. 23, 2016, 7:47 p.m.
Message ID <32515c15c0fb7fbe74e6.1482522458@pulkit-goyal>
Download mbox | patch
Permalink /patch/18021/
State Accepted
Headers show

Comments

Pulkit Goyal - Dec. 23, 2016, 7:47 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1482522374 -19800
#      Sat Dec 24 01:16:14 2016 +0530
# Node ID 32515c15c0fb7fbe74e6bbd61574baba3dd6e217
# Parent  79fe51eaad172888e8d59431743d6fa466e0dfc9
url: remove unnecessary deletion of environ variables while dealing with proxy

Currently we delete proxy environment variables if ui.config contains proxy
values. This is unnecessary because urllib2.ProxyHandler class only reads proxy
from environment it is initialised by None. But url.py never passes None,
so there is no point urllib2 will take environment variables in account.

This also prevents deleting environment variables which is not safe.

This code was introduced while resolving Bug 2451 even it is in one of comments
(sixth one) on bug that we can safely remove this part.
Link to bug : https://bz.mercurial-scm.org/show_bug.cgi?id=2451
Augie Fackler - Dec. 24, 2016, 7:33 p.m.
On Sat, Dec 24, 2016 at 01:17:38AM +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1482522374 -19800
> #      Sat Dec 24 01:16:14 2016 +0530
> # Node ID 32515c15c0fb7fbe74e6bbd61574baba3dd6e217
> # Parent  79fe51eaad172888e8d59431743d6fa466e0dfc9
> url: remove unnecessary deletion of environ variables while dealing with proxy

Queued, thanks.

>
> Currently we delete proxy environment variables if ui.config contains proxy
> values. This is unnecessary because urllib2.ProxyHandler class only reads proxy
> from environment it is initialised by None. But url.py never passes None,
> so there is no point urllib2 will take environment variables in account.
>
> This also prevents deleting environment variables which is not safe.
>
> This code was introduced while resolving Bug 2451 even it is in one of comments
> (sixth one) on bug that we can safely remove this part.
> Link to bug : https://bz.mercurial-scm.org/show_bug.cgi?id=2451
>
> diff -r 79fe51eaad17 -r 32515c15c0fb mercurial/url.py
> --- a/mercurial/url.py	Thu Dec 22 19:35:30 2016 +0530
> +++ b/mercurial/url.py	Sat Dec 24 01:16:14 2016 +0530
> @@ -15,7 +15,6 @@
>
>  from .i18n import _
>  from . import (
> -    encoding,
>      error,
>      httpconnection as httpconnectionmod,
>      keepalive,
> @@ -115,17 +114,6 @@
>          else:
>              proxies = {}
>
> -        # urllib2 takes proxy values from the environment and those
> -        # will take precedence if found. So, if there's a config entry
> -        # defining a proxy, drop the environment ones
> -        if ui.config("http_proxy", "host"):
> -            for env in ["HTTP_PROXY", "http_proxy", "no_proxy"]:
> -                try:
> -                    if env in encoding.environ:
> -                        del encoding.environ[env]
> -                except OSError:
> -                    pass
> -
>          urlreq.proxyhandler.__init__(self, proxies)
>          self.ui = ui
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff -r 79fe51eaad17 -r 32515c15c0fb mercurial/url.py
--- a/mercurial/url.py	Thu Dec 22 19:35:30 2016 +0530
+++ b/mercurial/url.py	Sat Dec 24 01:16:14 2016 +0530
@@ -15,7 +15,6 @@ 
 
 from .i18n import _
 from . import (
-    encoding,
     error,
     httpconnection as httpconnectionmod,
     keepalive,
@@ -115,17 +114,6 @@ 
         else:
             proxies = {}
 
-        # urllib2 takes proxy values from the environment and those
-        # will take precedence if found. So, if there's a config entry
-        # defining a proxy, drop the environment ones
-        if ui.config("http_proxy", "host"):
-            for env in ["HTTP_PROXY", "http_proxy", "no_proxy"]:
-                try:
-                    if env in encoding.environ:
-                        del encoding.environ[env]
-                except OSError:
-                    pass
-
         urlreq.proxyhandler.__init__(self, proxies)
         self.ui = ui