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

Urllib2 authentication memory. #51408

Closed
bradobro mannequin opened this issue Oct 17, 2009 · 12 comments
Closed

Urllib2 authentication memory. #51408

bradobro mannequin opened this issue Oct 17, 2009 · 12 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@bradobro
Copy link
Mannequin

bradobro mannequin commented Oct 17, 2009

BPO 7159
Nosy @terryjreedy, @orsenthil, @bitdancer, @axitkhurana, @vadmium
Files
  • auth.patch
  • auth10.patch
  • prior_auth_docs.patch
  • auth11.patch
  • 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 = 'https://github.com/orsenthil'
    closed_at = <Date 2016-05-24.16:41:29.167>
    created_at = <Date 2009-10-17.15:32:38.254>
    labels = ['type-feature', 'library']
    title = 'Urllib2 authentication memory.'
    updated_at = <Date 2016-05-24.16:41:29.166>
    user = 'https://bugs.python.org/bradobro'

    bugs.python.org fields:

    activity = <Date 2016-05-24.16:41:29.166>
    actor = 'terry.reedy'
    assignee = 'orsenthil'
    closed = True
    closed_date = <Date 2016-05-24.16:41:29.167>
    closer = 'terry.reedy'
    components = ['Library (Lib)']
    creation = <Date 2009-10-17.15:32:38.254>
    creator = 'bradobro'
    dependencies = []
    files = ['39045', '39062', '39074', '39075']
    hgrepos = []
    issue_num = 7159
    keywords = ['patch']
    message_count = 12.0
    messages = ['94180', '236620', '240367', '241136', '241180', '241182', '241258', '241259', '241260', '241271', '266231', '266246']
    nosy_count = 11.0
    nosy_names = ['terry.reedy', 'orsenthil', 'dstanek', 'r.david.murray', 'cvrebert', 'bradobro', 'BreamoreBoy', 'python-dev', 'axitkhurana', 'martin.panter', 'mvolz']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue7159'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @bradobro
    Copy link
    Mannequin Author

    bradobro mannequin commented Oct 17, 2009

    For each request requiring HTTP authentication, urllib2 submits the
    request without authentication, receives the server's 401
    error/challenge, then re-submits the request with authentication.

    This is compliant behavior. The problem comes in that urllib2 repeats
    this for every ensuing request to the same namespace.

    At times this is just an inefficiency--every request gets sent twice,
    often with POST data (which can be sizeable).

    Sometimes, especially with large POST bodies, this causes a connection
    failure.

    (Mercurial suffers greatly from this (and I have suggested workaround to
    that team.)

    This isn't non-compliant behavior, but RFC2617 (sections 2, 3.3)
    suggests that once an HTTP client authenticates, it pre-emptively send
    authentication with ensuing requests.

    More analysis and fix suggestions at
    bitbucket.org/bradobro/liquidhg/wiki/Home.

    @bradobro bradobro mannequin added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir labels Oct 17, 2009
    @orsenthil orsenthil self-assigned this Oct 18, 2009
    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Feb 25, 2015

    @senthil what is your opinion of this?

    @BreamoreBoy BreamoreBoy mannequin added performance Performance or resource usage and removed type-bug An unexpected behavior, bug, or error labels Feb 25, 2015
    @bitdancer
    Copy link
    Member

    bitdancer commented Apr 9, 2015

    See also bpo-19494. I think this would be a new feature, and it may be that it should leverge the feature added in bpo-19494. The difference here is that we are proposing to allow it to happen automatically after the initial 401, whereas in 19494 we send it pre-emptively even on the first call.

    @bitdancer bitdancer added type-feature A feature request or enhancement and removed performance Performance or resource usage labels Apr 9, 2015
    @axitkhurana
    Copy link
    Mannequin

    axitkhurana mannequin commented Apr 15, 2015

    Adding a patch for Python 3+

    Some notes:

    • Adding a new password manager to handle this case

    • The new handler added in bpo-19494 had couple of issues

      • test passes even if we use the old handler in added test
      • uses request.host instead of request.full_url as key for password
    • The new handler did assume realm = None before and still does.

    I'm using the same logic for adding keys for adding authenticated urls/realm as for login credentials in basic auth handler.

    @bitdancer
    Copy link
    Member

    bitdancer commented Apr 15, 2015

    Added some review comments.

    I think the urllib documentation does not really explain how to *use* these classes, and it should, but that is a separate issue.

    @axitkhurana
    Copy link
    Mannequin

    axitkhurana mannequin commented Apr 15, 2015

    Updated patch with review changes

    @bitdancer
    Copy link
    Member

    bitdancer commented Apr 16, 2015

    Here is the doc patch for the design that Akshit and I agreed to after re-consideration of the API. This eliminates the HTTPBasicPriorAuthHandler in favor of the HTTPPasswdMgrWithPriorAuth password manager and putting support for prior auth on the AbstractBasicAuthHandler based on whether or not the password_mgr supports the is_authenticated and updated_authenticated methods. This redesign means that the Proxy handler automatically gets support for prior auth.

    @bitdancer
    Copy link
    Member

    bitdancer commented Apr 16, 2015

    Actually attaching the patch this time.

    @axitkhurana
    Copy link
    Mannequin

    axitkhurana mannequin commented Apr 16, 2015

    Updated code as per docs

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 16, 2015

    New changeset 1b9e81cb83bc by R David Murray in branch 'default':
    bpo-7159: generalize urllib prior auth support.
    https://hg.python.org/cpython/rev/1b9e81cb83bc

    @terryjreedy
    Copy link
    Member

    terryjreedy commented May 24, 2016

    Should this be closed? A substantial patch was pushed year ago and I see no indication of further issue.

    @bradobro
    Copy link
    Mannequin Author

    bradobro mannequin commented May 24, 2016

    Yes, Go ahead. Thanks.

    On Tue, May 24, 2016 at 1:56 AM, Terry J. Reedy <report@bugs.python.org>
    wrote:

    Terry J. Reedy added the comment:

    Should this be closed? A substantial patch was pushed year ago and I see
    no indication of further issue.

    ----------
    nosy: +terry.reedy


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue7159\>


    @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
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants