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

ValueError: Content-Length should be specified #55291

Closed
WilliamWu mannequin opened this issue Jan 31, 2011 · 25 comments
Closed

ValueError: Content-Length should be specified #55291

WilliamWu mannequin opened this issue Jan 31, 2011 · 25 comments
Assignees
Labels
docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error

Comments

@WilliamWu
Copy link
Mannequin

WilliamWu mannequin commented Jan 31, 2011

BPO 11082
Nosy @birkenfeld, @orsenthil, @ezio-melotti, @merwok
Files
  • test_urllib_request.patch: unit test file
  • urllib_request.patch: the patch file of request.py
  • issue11082.diff
  • issue11082-2.diff
  • issue-11082-docs.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 2012-03-16.01:18:26.438>
    created_at = <Date 2011-01-31.19:18:12.176>
    labels = ['type-bug', 'docs']
    title = 'ValueError: Content-Length should be specified'
    updated_at = <Date 2012-03-16.01:18:26.420>
    user = 'https://bugs.python.org/WilliamWu'

    bugs.python.org fields:

    activity = <Date 2012-03-16.01:18:26.420>
    actor = 'orsenthil'
    assignee = 'orsenthil'
    closed = True
    closed_date = <Date 2012-03-16.01:18:26.438>
    closer = 'orsenthil'
    components = ['Documentation']
    creation = <Date 2011-01-31.19:18:12.176>
    creator = 'William.Wu'
    dependencies = []
    files = ['20633', '20634', '20732', '20741', '23641']
    hgrepos = []
    issue_num = 11082
    keywords = ['patch']
    message_count = 25.0
    messages = ['127646', '127725', '127728', '127731', '127744', '127752', '127753', '127896', '127897', '127904', '128295', '128304', '128308', '128314', '128321', '128361', '128362', '128367', '128380', '128440', '129547', '147324', '147330', '155984', '155985']
    nosy_count = 6.0
    nosy_names = ['georg.brandl', 'orsenthil', 'ezio.melotti', 'eric.araujo', 'William.Wu', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue11082'
    versions = ['Python 3.2']

    @WilliamWu
    Copy link
    Mannequin Author

    WilliamWu mannequin commented Jan 31, 2011

    I found this bug when I started to trying Python 3.2 release candidate 1.

    When using urllib.request.urlopen to handle HTTP POST, I got the error message:

    ValueError: Content-Length should be specified for iterable data of type <class 'str'> 'foo=bar'

    I'll attach the patch and test case.

    @WilliamWu WilliamWu mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jan 31, 2011
    @birkenfeld
    Copy link
    Member

    Senthil, could this be a regression of the recent urllib transfer-encoding changes?

    @orsenthil
    Copy link
    Member

    The POST data should be bytes. So in the attached test case, instead of

    request = urllib.request.Request('http://does.not.matter', 'foo=bar')

    it should be:

    request = urllib.request.Request('http://does.not.matter', b'foo=bar')

    And the Content-Length will be calculated using this logic.

    mv = memoryview(data)
    Content-length = len(mv) * mv.itemsize

    Should we emphasize further that data should be bytes? I think error
    message can be improved here. This issue is not a blocker.

    @vstinner
    Copy link
    Member

    vstinner commented Feb 2, 2011

    Since r70638, the http client encodes unicode to ISO-8859-1:
    << RFC 2616 says that iso-8859-1 is the default charset for HTTP entity
    bodies, but we encoded strings using ascii. See
    http://bugs.python.org/issue5314. Changed docs and code to use
    iso-8859-1. >>

    @WilliamWu
    Copy link
    Mannequin Author

    WilliamWu mannequin commented Feb 2, 2011

    If the POST data should be bytes which I also think reasonable,

    should urllib.parse.urlencode return bytes instead of str?

    >>> urllib.parse.urlencode({'foo': 'bar'})
    'foo=bar'
    >>> urllib.parse.urlencode({b'foo': b'bar'})
    'foo=bar'

    @birkenfeld
    Copy link
    Member

    That would seem correct to me.

    @birkenfeld
    Copy link
    Member

    It is also a question whether to disallow str explicitly, instead of letting it go through the Iterable check.

    @WilliamWu
    Copy link
    Mannequin Author

    WilliamWu mannequin commented Feb 4, 2011

    So, what's the decision to be taken? I'm willing to provide patches (if I need to), but I need to know *the reasonable behaviors*. :)

    @orsenthil
    Copy link
    Member

    For this particular issue, I think, it is good idea to disallow str
    explicitly, instead of letting it go through the Iterable and raise a
    ValueError with a different message.

    @birkenfeld
    Copy link
    Member

    Then let us do that.

    Senthil, what about urlencode of bytes values returning a str?

    @orsenthil
    Copy link
    Member

    On Sat, Feb 5, 2011 at 12:33 AM, Georg Brandl <report@bugs.python.org> wrote:

    Then let us do that.

    Senthil, what about urlencode of bytes values returning a str?

    Sorry, the late response. I needed some time to look at this one and I
    could only do it today.
    We have been having str return value from urlencode, due to quote,
    quote_plus methods which can take bytes/str but return str. (urlencode
    is nothing but quote_plus which acts on two-element sequences like
    name,value pairs.

    The resultant str is certainly useful when we want to construct a URL
    query string. But, when we want to do a POST via urlopen, we need
    bytes data. So, I think best to advice through the documentation that,
    urlencoded value must be converted to bytes if it is to be used for
    POST'ing using urlopen.

    (Well, I also think that urllencode could be modified to the effect
    that if the key,value pairs are bytes, return the bytes output.
    Technically, this would involve overriding some behavior of
    quote/quote_plus and the return value will suitable only for POST data
    and not for url query construction. I would rather choose to discuss
    this before settling on the behavior and I remember that quote's
    behavior did take a significant amount discussion).

    So, for this issue. A patch would a prevent str data from being posted
    with ValueError explaining the reason that bytes and required and a
    documentation update in the urlencode method which would inform the
    users that the output be converted to bytes before it is POSTed,

    Shall we go ahead with this, Georg? I shall post a patch immediately.

    @orsenthil
    Copy link
    Member

    Here is a patch to resolve this issue. Let me know if it okay to commit.(Or feel free to commit too).

    @vstinner
    Copy link
    Member

    Georg Brandl: can this fix go into Python 3.2? It changes the API.

    I like any patch rejecting unicode where unicode is irrevelant (where we don't know how to choose the right encoding).

    About the patch: you should maybe add a test to ensure that str is rejected with a TypeError.

    @orsenthil
    Copy link
    Member

    Victor - It does not change the API. Only that the ValueError message which had a confusing message that "for iterable data of type <class 'str'> " is made clear that POST should not be a str.
    Yes, a TypeError instead of ValueError would be more appropriate here.

    @birkenfeld
    Copy link
    Member

    Victor: I don't see an API change here. A ValueError is raised in both cases, and error messages are not part of the API. (And BTW, you may call me Georg.)

    Senthil: I'd like the documentation change to be a bit more explicit that a string is always returned. Since the string must be encoded, it would also be helpful to mention which encoding to use.

    The new exception message looks slightly incorrect to me: the argument can also be an iterable of bytes.

    @orsenthil
    Copy link
    Member

    Here is the patch with addressing the comments.

    1. It should be TypeError instead of ValueError when str is rejected. We introduced ValueError for this release only, so there is no breakage here.
    2. Informed the user that string should be encoded to bytes using user-specified encoding before it is sent as POST data. Also pointed to Examples in the urllib.request section. ( I could not hyperlink directly to #examples section. If that is possible, it would be better).

    @orsenthil
    Copy link
    Member

    patch with Docs :ref: to proper section.

    @birkenfeld
    Copy link
    Member

    I still find "user-specified encoding" unclear. This can be addressed at a different time though.

    Your exception message is missing a space between "bytes" and "or"; otherwise this is ok to commit.

    @orsenthil
    Copy link
    Member

    Thanks. Committed in r88394.

    Regarding the encoding information. An explanation of this sort might be helpful.

    The string can be encoded using ISO-8859-1 which is the default encoding for POST data or the user can also encode using a custom encoding , in which case, Content-Type: header should specify the encoding value in addition to 'application/x-www-form-urlencoded' which is specified for POST data. For Example:, if one has to use utf-8

    >> sdata = urllib.parse.urlencode({"ሀ":"ሢ"})
    >> bytesdata = sdata.encode('utf-8')
    >>reqheaders=dict([("Content-Type"," application/x-www-form-urlencoded ; charset=utf-8")])
    >>urllib.request.Request('http://www.example.com',bytesdata,reqheaders)

    @birkenfeld
    Copy link
    Member

    Lowering priority and making a doc issue now that the code change has been made.

    @birkenfeld birkenfeld added docs Documentation in the Doc dir and removed stdlib Python modules in the Lib dir deferred-blocker labels Feb 12, 2011
    @merwok
    Copy link
    Member

    merwok commented Feb 26, 2011

    BTW, there was no entry in Misc/NEWS, so this was not in whatsnew/3.2.rst.

    @orsenthil
    Copy link
    Member

    Here is the docs patch which will help us close the issue. Addressing Eric's last comment - I believe the what's new and News for this issue was added with the feature, this one was Exception msg change.

    @ezio-melotti
    Copy link
    Member

    I think the information in the patch should go in the urlopen doc, not in urlencode. Adding a note to urlencode that says that the result must be encoded is fine, but all the details about the default encoding and the fact that an extra Content-Type header is necessary when the encoding is not iso-8859-1 belong to the urlopen doc IMHO. (Is the header actually necessary? I think I always sent utf-8 without header, and the example you added in r88394 also uses utf-8 with no extra headers.)
    There are also a few minor issues with the patch -- I'll leave a note in rietveld for that.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 16, 2012

    New changeset 057cf78ed576 by Senthil Kumaran in branch '3.2':
    Explain the use of charset parameter with Content-Type header. bpo-11082
    http://hg.python.org/cpython/rev/057cf78ed576

    New changeset 90e35b91756d by Senthil Kumaran in branch 'default':
    Explain the use of charset parameter with Content-Type header: bpo-11082
    http://hg.python.org/cpython/rev/90e35b91756d

    @orsenthil
    Copy link
    Member

    I have rewritten some parts of the documentation, explaining the use of charset parameter with Content-Type header. Used the suggestions from the previous patch's review comments too. I see that other parts of the documentation can be improved further, I shall do that separately. I am closing this issue. Thanks!

    @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
    docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants