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

Requests 2.11: check_header_validity failed on header with integer value #3477

Closed
lmazuel opened this issue Aug 8, 2016 · 23 comments
Closed

Comments

@lmazuel
Copy link

lmazuel commented Aug 8, 2016

Hi,

Since requests 2.11, all my calls using requests for my app are broken. After debugging, it seems that this version does not accept header with integer value, like it was before.

2.10:

In [1]: import requests

In [2]: requests.get('http://bing.com', headers={'Content-Length': 42})
Out[2]: <Response [200]>

2.11

In [1]: import requests

In [2]: requests.get('http://bing.com', headers={'Content-Length': 42})
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
D:\VSProjects\azure-sdk-for-python\env3.5\Lib\site-packages\requests\utils.py in check_header_validity(header)
    751     try:
--> 752         if not pat.match(value):
    753             raise InvalidHeader("Invalid return character or leading space in header: %s" % name)

TypeError: expected string or bytes-like object

During handling of the above exception, another exception occurred:

InvalidHeader                             Traceback (most recent call last)
<ipython-input-2-ae7ec2933e34> in <module>()
----> 1 requests.get('http://bing.com', headers={'Content-Length': 42})

D:\VSProjects\azure-sdk-for-python\env3.5\Lib\site-packages\requests\api.py in get(url, params, **kwargs)
     68
     69     kwargs.setdefault('allow_redirects', True)
---> 70     return request('get', url, params=params, **kwargs)
     71
     72

D:\VSProjects\azure-sdk-for-python\env3.5\Lib\site-packages\requests\api.py in request(method, url, **kwargs)
     54     # cases, and look like a memory leak in others.
     55     with sessions.Session() as session:
---> 56         return session.request(method=method, url=url, **kwargs)
     57
     58

D:\VSProjects\azure-sdk-for-python\env3.5\Lib\site-packages\requests\sessions.py in request(self, method, url, params, data, headers, cookies, files, auth, timeout, allow_redirects, proxies, hooks, stream, verify, cert, json)
    455             hooks = hooks,
    456         )
--> 457         prep = self.prepare_request(req)
    458
    459         proxies = proxies or {}

D:\VSProjects\azure-sdk-for-python\env3.5\Lib\site-packages\requests\sessions.py in prepare_request(self, request)
    388             auth=merge_setting(auth, self.auth),
    389             cookies=merged_cookies,
--> 390             hooks=merge_hooks(request.hooks, self.hooks),
    391         )
    392         return p

D:\VSProjects\azure-sdk-for-python\env3.5\Lib\site-packages\requests\models.py in prepare(self, method, url, headers, files, data, params, auth, cookies, hooks, json)
    293         self.prepare_method(method)
    294         self.prepare_url(url, params)
--> 295         self.prepare_headers(headers)
    296         self.prepare_cookies(cookies)
    297         self.prepare_body(data, files, json)

D:\VSProjects\azure-sdk-for-python\env3.5\Lib\site-packages\requests\models.py in prepare_headers(self, headers)
    407             for header in headers.items():
    408                 # Raise exception on invalid header value.
--> 409                 check_header_validity(header)
    410                 name, value = header
    411                 self.headers[to_native_string(name)] = value

D:\VSProjects\azure-sdk-for-python\env3.5\Lib\site-packages\requests\utils.py in check_header_validity(header)
    754     except TypeError:
    755         raise InvalidHeader("Header value %s must be of type str or bytes, "
--> 756                             "not %s" % (value, type(value)))
    757
    758

InvalidHeader: Header value 42 must be of type str or bytes, not <class 'int'>

We define 'Content-Length' in each request. Anyway, using an integer for an header which is semantically an integer makes sense no?

@Lukasa
Copy link
Member

Lukasa commented Aug 8, 2016

Non-strings for header values was never an accepted way to use Requests, sadly, and while it was allowed in prior versions we have since made changes that disallow it. This is primarily because headers are really string-string mappings, but also because the general approach of calling str on things that are passed to Requests has a tendency to misbehave in unexpected ways that surprise users.

The TL;DR of this is: yes, this was done on purpose, and no, we don't think it's a bug.

@Lukasa
Copy link
Member

Lukasa commented Aug 8, 2016

See also: #865.

@clarkbreyman-yammer
Copy link

Seems like a breaking change for a dot-release, no?

@lmazuel
Copy link
Author

lmazuel commented Aug 8, 2016

And you didn't describe this huge modification in the ChangeLog :(

@kennethreitz
Copy link
Contributor

This is not a breaking API change — the API is clearly documented as being string-based, throughout the documentation. The fact that you were using it with integers, instead of its intended input, is a bug in your code, not with in this codebase or a change in the API.

Adding a note in the changelog about this non-breaking change may be a good idea.

@Lukasa
Copy link
Member

Lukasa commented Aug 8, 2016

@clarkbreyman-yammer To be clear, this is a bit of a borderline case. You can see the decision making most recently in #3386 and #3388, but the basic argument is: non-string headers were never intended to work, so the fact that they stopped working is acceptable. In fact, that have previously not worked in the past.

@lmazuel You're right that this got missed from the changelog though, and that's 100% my fault: the breakage here was incidental of our tighter verification of header values, and as a result I didn't see it when I was compiling the changelog. I'd welcome a PR that updates the changelog if you'd like to make one.

@sigmavirus24
Copy link
Contributor

And to be clear, all three of us are in agreement on this. The fact that sending an integer there ever worked was purely luck. It used to not work at all and we decided it should never work long ago. We never had anything that enforced that though.

@kennethreitz
Copy link
Contributor

That being said, adding that as a supported feature is potentially a great idea, and would be a welcome API change in my book (if the implementation was made well). However, the current status of things is likely to remain for the extended future.

@kennethreitz
Copy link
Contributor

Added a note in the changelog.

@Lukasa
Copy link
Member

Lukasa commented Aug 8, 2016

Thanks @kennethreitz.

@lmazuel
Copy link
Author

lmazuel commented Aug 8, 2016

Ok, thanks for the clarification. I will update my code accordingly.
@kennethreitz if you could add your note in PyPI too, would be amazing.

@kennethreitz
Copy link
Contributor

@lmazuel on it!

@kennethreitz
Copy link
Contributor

@lmazuel done ✨🍰✨

@kennethreitz
Copy link
Contributor

@lmazuel p.s. thank you for caring :P

@lmazuel
Copy link
Author

lmazuel commented Aug 8, 2016

I fixed my code, everything else is working great. Thanks for your ultra quick response time!

@lmazuel lmazuel closed this as completed Aug 8, 2016
little-dude referenced this issue in little-dude/bambou Aug 29, 2016
as of 2.11, requests raises an exception if header values are not `str`
or `bytes`.

see: https://github.com/kennethreitz/requests/issues/3477
@Lukasa
Copy link
Member

Lukasa commented Sep 27, 2016

or how to translate a headers in dict type to str type

Wait, what? How were you sending headers before?

@COLDMOUNT
Copy link

Just as the guide on the page --
http://docs.python-requests.org/en/master/user/quickstart/ -- "If you'd
like to add HTTP headers to a request, simply pass in a dict to the headers
parameter." But now dict type is no longer accepted, and what does the str
type look like?

2016-09-27 17:51 GMT+08:00 Cory Benfield notifications@github.com:

or how to translate a headers in dict type to str type

Wait, what? How were you sending headers before?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kennethreitz/requests/issues/3477#issuecomment-249819028,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ARIa8_ZPJmzxH2lWmpTvSJ3PvHE7jlpvks5quOcrgaJpZM4JffFG
.

@Lukasa
Copy link
Member

Lukasa commented Sep 27, 2016

@COLDMOUNT dict is absolutely accepted, we haven't changed that at all. What we have changed is that the keys and values of that dict must now be strings. Previously it was possible for them to be a few other types by accident, which has now been resolved. The documentation does not need changing.

@COLDMOUNT
Copy link

Got it, cool! Thanks! :)

2016-09-27 18:23 GMT+08:00 Cory Benfield notifications@github.com:

@COLDMOUNT https://github.com/COLDMOUNT dict is absolutely accepted,
we haven't changed that at all. What we have changed is that the keys and
values of that dict must now be strings. Previously it was possible for
them to be a few other types by accident, which has now been resolved. The
documentation does not need changing.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kennethreitz/requests/issues/3477#issuecomment-249825918,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ARIa8z0jZ-ovBtvFWl4hTXnkk_kJXobrks5quO6ggaJpZM4JffFG
.

mikem23 referenced this issue in mikem23/koji-playground Nov 23, 2016
This has seemingly always been documented, and broken in a recent release.

Reference: https://github.com/kennethreitz/requests/issues/3477

Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
mineo referenced this issue in mineo/python-musicbrainz-ngs Dec 10, 2016
Recent versions of requests no longer accept integer header values.

See https://github.com/kennethreitz/requests/issues/3477

Signed-off-by: Wieland Hoffmann <themineo@gmail.com>
mkouhei referenced this issue in mkouhei/swiftsc Apr 22, 2017
mineo referenced this issue in mineo/python-musicbrainz-ngs Jun 11, 2017
Recent versions of requests no longer accept integer header values.

See https://github.com/kennethreitz/requests/issues/3477

Signed-off-by: Wieland Hoffmann <themineo@gmail.com>
openstack-gerrit pushed a commit to openstack/python-mistralclient that referenced this issue Aug 3, 2017
1. Adds test for target-* parameters.
2. Fixes an error that is caused by requests not accepting non-string
   header parameters psf/requests#3477

Closes-Bug: 1708099
Change-Id: Ia5d1ad5062751c15928cbfd7709665a3ed769603
panlinux added a commit to panlinux/libcloud that referenced this issue Aug 28, 2018
Since python-requests 2.11
(see psf/requests#3477), integer values
in headers are no longer automatically converted to strings, so we need
to do it ourselves.

This is the same change that was done a couple of lines above for the
Content-Length header in commit 28a5590.
asfgit pushed a commit to apache/libcloud that referenced this issue Aug 29, 2018
Since python-requests 2.11
(see psf/requests#3477), integer values
in headers are no longer automatically converted to strings, so we need
to do it ourselves.

This is the same change that was done a couple of lines above for the
Content-Length header in commit 28a5590.

Signed-off-by: Quentin Pradet <quentinp@apache.org>
mineo referenced this issue in mineo/python-musicbrainz-ngs Oct 7, 2018
Recent versions of requests no longer accept integer header values.

See https://github.com/kennethreitz/requests/issues/3477

Signed-off-by: Wieland Hoffmann <themineo@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants