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

HTTPResponse.close() should consume all remaining data in body if any #70419

Closed
Jacky mannequin opened this issue Jan 28, 2016 · 4 comments
Closed

HTTPResponse.close() should consume all remaining data in body if any #70419

Jacky mannequin opened this issue Jan 28, 2016 · 4 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@Jacky
Copy link
Mannequin

Jacky mannequin commented Jan 28, 2016

BPO 26231
Nosy @vadmium, @iritkatriel
Files
  • zzz.py: A http server and a http client for reproducing the issue
  • 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 2021-06-18.13:38:09.669>
    created_at = <Date 2016-01-28.10:43:17.369>
    labels = ['type-feature', 'library']
    title = 'HTTPResponse.close() should consume all remaining data in body if any'
    updated_at = <Date 2021-06-18.13:38:09.668>
    user = 'https://bugs.python.org/Jacky'

    bugs.python.org fields:

    activity = <Date 2021-06-18.13:38:09.668>
    actor = 'iritkatriel'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-06-18.13:38:09.669>
    closer = 'iritkatriel'
    components = ['Library (Lib)']
    creation = <Date 2016-01-28.10:43:17.369>
    creator = 'Jacky'
    dependencies = []
    files = ['41742']
    hgrepos = []
    issue_num = 26231
    keywords = []
    message_count = 4.0
    messages = ['259122', '259124', '259127', '396064']
    nosy_count = 3.0
    nosy_names = ['martin.panter', 'Jacky', 'iritkatriel']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue26231'
    versions = ['Python 2.7']

    @Jacky
    Copy link
    Mannequin Author

    Jacky mannequin commented Jan 28, 2016

    HTTPResponse.close() in httplib should consume all remaining data in body if any. Or the followed request would get the unread content from the previous request, which result in the wrong result.

    The following code shown that the second request will get a wrong status code if calling HTTPResponse.close() on the first response.

    The whole code consists of a HTTP server and a client. The server will always return 403 for testing.

    def client():
        conn = httplib.HTTPConnection(HOST, PORT)
        conn.request('GET', PATH, None, headers={})
        rsp = conn.getresponse()
        print rsp.status
        rsp.close()  # close response
    
        conn.request('GET', PATH, None, headers={})
        rsp2 = conn.getresponse()
        print rsp2.status  # --> should be 403

    The full version see the attached file (The server used Tornado)

    @Jacky Jacky mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jan 28, 2016
    @vadmium
    Copy link
    Member

    vadmium commented Jan 28, 2016

    The documentation already says you have to read the whole response before sending a new request: <https://docs.python.org/2/library/httplib.html#httplib.HTTPConnection.getresponse\>. So I think this is more like a new feature than a bug fix, and would have to go into 3.6.

    But I am not sure it is a good idea. What if the response is a large multi-megabyte download? In that case, it might make more sense to shut down the connection and start a new one.

    It might be possible to add extra error checking to prevent reading a second response until the first is complete. I am not sure if that is practical or worthwhile though.

    @Jacky
    Copy link
    Mannequin Author

    Jacky mannequin commented Jan 28, 2016

    In my opinion, HTTPResponse.close() should do really close work. Not only releasing the underlying file obj but also need to consume the remaining data to make sure the request complete.

    If close() does not consume the remaining data, the user would have to do it after invoking close(), regardless of how large the data is. But how do do it, the HTTPResponse has been closed. So we have to use HTTPConnection.sock to do read work. However, this looks somewhat weird.

    @iritkatriel
    Copy link
    Member

    If close() does not consume the remaining data, the user would have to do it after invoking close(), regardless of how large the data is.

    Martin's point was that you can abandon the connection and create a new one if you choose not to consume all the data.

    But how do do it, the HTTPResponse has been closed.

    Right, if you want to consume the data you should do it before calling close().

    @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

    2 participants