Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The proxy key's string should ignore case. #69254

Closed
ChuangCao mannequin opened this issue Sep 11, 2015 · 7 comments
Closed

The proxy key's string should ignore case. #69254

ChuangCao mannequin opened this issue Sep 11, 2015 · 7 comments
Labels
3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@ChuangCao
Copy link
Mannequin

ChuangCao mannequin commented Sep 11, 2015

BPO 25068
Nosy @Rosuav, @vadmium, @matrixise, @ZackerySpytz, @miss-islington
PRs
  • bpo-25068: urllib.request.ProxyHandler now lowercases the dict keys #13489
  • [3.8] bpo-25068: urllib.request.ProxyHandler now lowercases the dict keys (GH-13489) #16107
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2019-09-13.14:12:08.105>
    created_at = <Date 2015-09-11.08:46:13.166>
    labels = ['3.8', 'type-feature', 'library']
    title = "The proxy key's string should ignore case."
    updated_at = <Date 2019-09-13.14:26:03.246>
    user = 'https://bugs.python.org/ChuangCao'

    bugs.python.org fields:

    activity = <Date 2019-09-13.14:26:03.246>
    actor = 'miss-islington'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-09-13.14:12:08.105>
    closer = 'matrixise'
    components = ['Library (Lib)']
    creation = <Date 2015-09-11.08:46:13.166>
    creator = 'Chuang Cao'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 25068
    keywords = ['patch']
    message_count = 7.0
    messages = ['250454', '250455', '250458', '250459', '352336', '352337', '352339']
    nosy_count = 6.0
    nosy_names = ['Rosuav', 'martin.panter', 'matrixise', 'Chuang Cao', 'ZackerySpytz', 'miss-islington']
    pr_nums = ['13489', '16107']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue25068'
    versions = ['Python 3.8']

    @ChuangCao
    Copy link
    Mannequin Author

    ChuangCao mannequin commented Sep 11, 2015

    When use a urllib2.ProxyHandler to set a proxy, if the proxies's key is an upper string, like "HTTP", "HTTPS". The proxy url can't be used, because they can't find the <type>_open function.

    Two way can sovle the issue.

    1. Specify it in document to let users to use a lower string like "http".

    2. Add a patch in urllib2.py:

    diff -up ./urllib2.py.orig ./urllib2.py
    --- ./urllib2.py.orig	2015-09-11 15:06:55.686927934 +0800
    +++ ./urllib2.py	2015-09-11 16:56:48.306138898 +0800
    @@ -102,6 +102,7 @@ import sys
     import time
     import urlparse
     import bisect
    +import string
     
     try:
         from cStringIO import StringIO
    @@ -713,6 +714,7 @@ class ProxyHandler(BaseHandler):
             assert hasattr(proxies, 'has_key'), "proxies must be a mapping"
             self.proxies = proxies
             for type, url in proxies.items():
    +            type = string.lower(type)
                 setattr(self, '%s_open' % type,
                         lambda r, proxy=url, type=type, meth=self.proxy_open: \
                         meth(r, proxy, type))

    I think the second way is a good way.

    @ChuangCao ChuangCao mannequin added performance Performance or resource usage stdlib Python modules in the Lib dir labels Sep 11, 2015
    @Rosuav
    Copy link
    Contributor

    Rosuav commented Sep 11, 2015

    This sounds like a feature enhancement, which means it (almost certainly) won't be applied to Python 2.7. Does the same question come up in Python 3?

    Also (FWIW) if you can confidently assume that all the keys are strings. then type.lower() is better than string.lower(type).

    @vadmium
    Copy link
    Member

    vadmium commented Sep 11, 2015

    Looking at the code, I think Python 3 is in the same boat. Most things in Python are case-sensitive, but I think it is reasonable to make an exception here, since the protocol schemes in general are insensitive. E.g. urlopen("HTTPS://bugs.python.org/issue25068") still uses "https" internally.

    It would be good to include a test case and a note in the documentation if we added this though.

    @vadmium vadmium added type-feature A feature request or enhancement and removed performance Performance or resource usage labels Sep 11, 2015
    @vadmium
    Copy link
    Member

    vadmium commented Sep 11, 2015

    On the other hand, a documentation update saying it has to be lower case would be fine for Python 2.7 and 3.4+, if you wanted to go that way.

    @ZackerySpytz ZackerySpytz mannequin added the 3.8 only security fixes label May 22, 2019
    @matrixise
    Copy link
    Member

    New changeset b761e3a by Stéphane Wirtel (Zackery Spytz) in branch 'master':
    bpo-25068: urllib.request.ProxyHandler now lowercases the dict keys (GH-13489)
    b761e3a

    @matrixise
    Copy link
    Member

    Thank you for your contribution, your PR has been merged into master but not in 3.8.

    @miss-islington
    Copy link
    Contributor

    New changeset 590ed09 by Miss Islington (bot) in branch '3.8':
    bpo-25068: urllib.request.ProxyHandler now lowercases the dict keys (GH-13489)
    590ed09

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants