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

httplib doesn't specify content-length header for POST requests without data #58926

Closed
ArveKnudsen mannequin opened this issue May 4, 2012 · 18 comments
Closed

httplib doesn't specify content-length header for POST requests without data #58926

ArveKnudsen mannequin opened this issue May 4, 2012 · 18 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ArveKnudsen
Copy link
Mannequin

ArveKnudsen mannequin commented May 4, 2012

BPO 14721
Nosy @jcea, @orsenthil, @ned-deily, @ezio-melotti, @merwok, @demianbrecht
Files
  • httplib-2.7.patch: Patch httplib in Python 2.7 to set 'content-length: 0' automatically
  • 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 2012-05-19.09:00:21.969>
    created_at = <Date 2012-05-04.10:06:43.303>
    labels = ['type-bug', 'library']
    title = "httplib doesn't specify content-length header for POST requests without data"
    updated_at = <Date 2015-02-27.17:31:57.657>
    user = 'https://bugs.python.org/ArveKnudsen'

    bugs.python.org fields:

    activity = <Date 2015-02-27.17:31:57.657>
    actor = 'demian.brecht'
    assignee = 'orsenthil'
    closed = True
    closed_date = <Date 2012-05-19.09:00:21.969>
    closer = 'orsenthil'
    components = ['Library (Lib)']
    creation = <Date 2012-05-04.10:06:43.303>
    creator = 'Arve.Knudsen'
    dependencies = []
    files = ['25456']
    hgrepos = []
    issue_num = 14721
    keywords = ['patch']
    message_count = 18.0
    messages = ['159915', '159935', '159936', '159938', '159942', '159946', '159954', '159959', '159964', '159965', '161096', '161098', '161099', '161154', '236600', '236686', '236731', '236804']
    nosy_count = 10.0
    nosy_names = ['jcea', 'orsenthil', 'ned.deily', 'ezio.melotti', 'eric.araujo', 'Arve.Knudsen', 'python-dev', 'piotr.dobrogost', 'demian.brecht', 'jimr']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue14721'
    versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

    @ArveKnudsen
    Copy link
    Mannequin Author

    ArveKnudsen mannequin commented May 4, 2012

    httplib doesn't specify the HTTP header 'content-length' for POST requests without data. Conceptually this makes sense, considering the empty content. However, IIS (7.5) servers don't accept such requests and respond with a 411 status code. See this question on StackOverflow for reference: http://stackoverflow.com/questions/5915131/can-i-send-an-empty-http-post-webrequest-object-from-c-sharp-to-iis.

    See also issue #223 of the Requests project, https://github.com/kennethreitz/requests/issues/223, which regards this problem in the context of the requests Python library.

    The following code makes a data-less POST request to the HTTP sniffer Fiddler running on localhost:
    import httplib
    conn = httplib.HTTPConnection("localhost", 8888)
    conn.request("POST", "/post", "", {})
    conn.close()

    Fiddler reports that it receives the following headers for the POST request, as you can see 'content-length' is not included:
    POST http://localhost:8888/post HTTP/1.1
    Host: localhost:8888
    Accept-Encoding: identity

    @ArveKnudsen ArveKnudsen mannequin added the stdlib Python modules in the Lib dir label May 4, 2012
    @jcea
    Copy link
    Member

    jcea commented May 4, 2012

    Could you provide a patch?

    Does this affect 3.x too?

    @ezio-melotti ezio-melotti added the type-bug An unexpected behavior, bug, or error label May 4, 2012
    @ArveKnudsen
    Copy link
    Mannequin Author

    ArveKnudsen mannequin commented May 4, 2012

    I can look into patch and 3.x tonight I think. Should I provide a test with an eventual patch?

    @jcea
    Copy link
    Member

    jcea commented May 4, 2012

    Patch with test, please :-).

    I know it is a pain in the ass, but the result is having a higher quality python.

    @piotrdobrogost
    Copy link
    Mannequin

    piotrdobrogost mannequin commented May 4, 2012

    Fiddler reports that it receives the following headers for the POST request

    Python 3.2.3 on Windows Vista 64bit gives the same output for

    import http.client
    conn = http.client.HTTPConnection('localhost',8888)
    conn.request("POST", "/post", "", {})
    conn.close()

    @ArveKnudsen
    Copy link
    Mannequin Author

    ArveKnudsen mannequin commented May 4, 2012

    Which HTTP methods should we auto-define content-length for? POST and PUT? I noticed that my first attempt at a patch would define content-length also for GET requests, which broke a unit test (test_ipv6host_header).

    @ArveKnudsen
    Copy link
    Mannequin Author

    ArveKnudsen mannequin commented May 4, 2012

    Actually, when inspecting the HTTP requests sent by Chrome for the different methods (a great little Chrome app called Postman let me fire requests manually), I found that content-length would be set for most methods. I could confirm that 'content-length: 0' would be set for the following methods: POST, PUT, PATCH, DELETE and HEAD.

    I guess it should be good enough to model that behaviour in httplib?

    @jcea
    Copy link
    Member

    jcea commented May 4, 2012

    HEAD?. It doesn't make sense in HEAD if it doesn't make sense in GET.

    Looking around, I found this, to mud the water a little bit more: <http://fixunix.com/tcp-ip/66198-http-rfc-related-question-content-length-0-get-request.html\>.

    Not being explicit about this is a "bug" in the specification, I think.

    @ArveKnudsen
    Copy link
    Mannequin Author

    ArveKnudsen mannequin commented May 4, 2012

    Here's my initial proposal for a patch against httplib, based on the 2.7 branch of cpython repository. I've included a couple of tests, which check that when content is empty, content-length is set to 0 for certain methods (POST/PUT etc) and not specified for others.

    @ArveKnudsen
    Copy link
    Mannequin Author

    ArveKnudsen mannequin commented May 4, 2012

    Yes, I agree it doesn't make much sense for HEAD AFAICT, but Chrome does it. Maybe there's a reason?

    @orsenthil
    Copy link
    Member

    The rule for content-length seems, if there is a body for a request, even if the body is "" ( empty body), then you should send the Content-Length.

    The mistake in the Python httplib was, the set_content_length was called with this condition.

    if body and ('content-length' not in header_names):

    If the body was '', this was skipped. The default for GET and methods which do not use body was body=None and that was statement for correct in those cases.

    A simple fix which covers the applicable methods and follows the definition of content-length seems to me like this:

    •    if body and ('content-length' not in header_names):
      

    + if body is not None and 'content-length' not in header_names:

    I prefer this rather than checking for methods explicitly as it could go into unnecessary details. (Things like if you are not sending a body why are you sending a Content-Length?. This fails the definition of Content-Length itself). The Patch is fine, I would adopt that for the above check and commit it all the active versions.

    Thanks Arve Knudsen, for the bug report and the patch.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 19, 2012

    New changeset 57f1d13c2cd4 by Senthil Kumaran in branch '2.7':
    Fix bpo-14721: Send Content-length: 0 for empty body () in the http.request
    http://hg.python.org/cpython/rev/57f1d13c2cd4

    New changeset 6da1ab5f777d by Senthil Kumaran in branch '3.2':
    Fix bpo-14721: Send Content-length: 0 for empty body () in the http.client requests
    http://hg.python.org/cpython/rev/6da1ab5f777d

    New changeset 732d70746fc0 by Senthil Kumaran in branch 'default':
    merge - Fix bpo-14721: Send Content-length: 0 for empty body () in the http.client requests
    http://hg.python.org/cpython/rev/732d70746fc0

    @orsenthil
    Copy link
    Member

    This is fixed in all the branches. Thanks!

    @orsenthil orsenthil self-assigned this May 19, 2012
    @jcea
    Copy link
    Member

    jcea commented May 19, 2012

    Too late for asking to keep the parenthesis :-). I hate to have to remember non-obvious precedence rules :-). Cognitive overhead.

    @jimr
    Copy link
    Mannequin

    jimr mannequin commented Feb 25, 2015

    The fix for this still doesn't set Content-Length to zero when body is None, but I don't see any reason why this should be the case. For example, the following snippet would work for any 'empty' body:

    if 'content-length' not in header_names:
        self._set_content_length(body if body is not None else '')

    I'm happy to produce a patch if there's any chance it would be merged.

    @demianbrecht
    Copy link
    Mannequin

    demianbrecht mannequin commented Feb 26, 2015

    I'm happy to produce a patch if there's any chance it would be merged.

    If the patch adheres to the RFC, then I see no reason why it shouldn't be merged. What makes this a little more tricky than the snippet that you included in your post though (which would include the Content-Length header for all HTTP methods) is the following from RFC 7230:

    A user agent SHOULD send a Content-Length in a request message when
    no Transfer-Encoding is sent and the request method defines a meaning
    for an enclosed payload body. For example, a Content-Length header
    field is normally sent in a POST request even when the value is 0
    (indicating an empty payload body). A user agent SHOULD NOT send a
    Content-Length header field when the request message does not contain
    a payload body and the method semantics do not anticipate such a
    body.

    Currently, there is nothing in the http package that defines whether or not a given HTTP method expects a body (as far as I'm aware at any rate), although this would be a simple addition. I'd imagine that the result might look like this:

    _METHODS_EXPECTING_BODIES = {'OPTIONS', 'POST', 'PUT', 'PATCH'}

    if method.upper() in _METHODS_EXPECTING_BODIES and \
    'content-length' not in header_names:
    self._set_content_length(body)

    I'd prefer to have the conversion from None to empty string done in the body of _set_content_length in order to ensure consistency should the call be made from elsewhere.

    @ned-deily
    Copy link
    Member

    James, Demian: this issue has been closed for almost three years and the changes released long ago: comments made here will likely be ignored. Please open a new issue if you want them to be acted on.

    @demianbrecht
    Copy link
    Mannequin

    demianbrecht mannequin commented Feb 27, 2015

    Thanks for the heads up Ned.

    James: I've created bpo-23539 in the event that you'd like to contribute a patch.

    @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-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants