Raibek/requests-github-issues huggingface dataset:
Method to determine stream size pre-decoding

I would like to know the amount of data transferred over the wire for accounting purposes when streaming large compressed files.

Currently, I do:

```
req = requests.get(url, stream=True)
```

followed by `req.iter_content()`. Unfortunately the server doesn't respond with anything telling me the length and looking at the source of requests I can't find a way of knowing the original size. It looks like that information is [effectively lost at this point in the code](https://github.com/kennethreitz/requests/blob/2f39e0e2aa791da13a8358f7a48c0365461029d8/requests/packages/urllib3/response.py#L188). Any chance we could claw it back?


Apologies if this is a bogus issue, I just realised this is in the urllib3 package, just close this if I should report it there and there is no other solution you're aware of.


I think the right thing to do here is what you've done, which is raise an issue on `urllib3`. With that in mind, we'll close this. Thanks for raising this issue!
Response.text documentation does not mention chardet is bundled

Currently Response.text documentation mentions that chardet module is used to auto-detect response encoding if the module is available. This should probably be revised, since apparently chardet is bundled with requests itself. It might also be sensible to mention that chardet is painfully slow, and to describe the proper recipe for getting response as text while avoiding chardet.


I agree that we should replace that section of the docs, especially as it's wrong: we use [sigmavirus24/charade](https://github.com/sigmavirus24/charade), not chardet. I've got that change made already.

I'm not sure that we should include a discussion of the speed of `charade` in the documentation. Yes, it's slow. But should the _core documentation_ be providing optimisation tips? All that will happen is people will see them and use them, even when 99% of people will never need them. Seems senseless to me.


I'm not sure about optimisation tips, but I did find it surprising that r.text may be a really slow operation, since it looks like simple property access, not like statistical analysis of the response contents that may take tens of seconds.

Also, capitalize the if :)


I don't know. It might be worth mentioning that when the server does not provide an encoding in the `Content-Type` header that we'll try to auto-detect it for you. In that case, it _may_ take a short while but it will be accurate and more importantly it will encode the text properly. Should we document the trick of setting `Response.encoding` property to avoid this?


I really think that the current documentation, which suggests that Requests will attempt to guess, is the correct level of detail, at least for quickstart. I'm not opposed to adding a section about encoding to the advanced documentation, but we should be wary about suggesting that people should optimise this away. In the overwhelming majority of cases I suspect the `charade` delay is at least less than the network round trip. I'll see if I can get some hard data to prove that.
handles Retry-After header in 503 Service Unavailable

I had to write a wrapper to reqeust so that 503 Service Unavailable, if followed with Retry-After header to retry in given seconds. Can it be handled by the library by option?

Guess this is too abstract to be filed as an issue, because there are many other case you may wish to retry with an interval (unlike using requests.adapters.HTTPAdapter which retry immediately), especially when the target web service is protected by an anti-DDOS service. Should've discussed before filing, but mailling list seems to be down, emails to requests@librelist.org was not answered by any maillist bot.


Your message definitely made the mailing list. I'm going to close this issue, and respond on the list.


Is there an archive for the mailing list so I can look at it around the above dates?

There does not appear to be: that list is basically defunct anyway.

A search of requests source for Retry-After didn't turn up anything. Is there a common way that requests' users comply with the HTTP/1.1 RFC? https://tools.ietf.org/html/rfc7231#page-69

Yeah, it's actually pretty easy. If you enable retries then we'll automatically respect Retry-After when present. The code change is small"

```python
import requests
from requests.adapters import HTTPAdapter

s = requests.Session()
s.mount('http://', HTTPAdapter(max_retries=5))
s.mount('https://', HTTPAdapter(max_retries=5))

# Use Session as normal
```

Thanks, that's the pointer I needed to read through requests and urllib3 and work the detail out for myself. Hopefully, it will help others that also tread this way too.
loosen URL handling for non-native URL schemes

Requests supports adding other URL schemes by mounting an adapter for them. For
schemes that have unfamiliar URLs, requests currently prevents the request from ever reaching the adapter, making it impossible to extend requests with sometimes convenient extensions. This change allows requests to detect this and gives the adapter author the full ability to shoot themselves firmly in the foot.

NOTE: This is to be compatible with the previous work I did to add a file URL scheme to Requests (which is now available here: https://github.com/jvantuyl/requests-file). I've since added a data URL scheme implementation (available https://github.com/jvantuyl/requests-data).


This is the re-opened version of #1710.


Hrm.  The build failed, though it looks like it may have been a transient connection error with httpbin?


Yeah, just a transient problem. As discussed previously, I'm generally happy with this kind of change. It's up to @sigmavirus24 and @kennethreitz to agree/disagree with this.


It seems @kennethreitz has warmed to the idea so I'll withdraw my objections once the tests are made a bit better. You should check to make sure the url isn't changed in the future.


@sigmavirus24 How's that?


Tests look good to me. Thanks for that.


Ugh, sorry, I missed this when I originally reviewed the code. We use py.test, so can you write the test using a py.test assert, rather than a unittest one? That will mean changing `self.assertEqual(test_url, preq.url)` to `assert test_url == preq.url`.


How's that?


Beautiful, thanks! 


I'm still torn about this. Could be convinced. Sorry for being difficult.


ಠ_ಠ

Ummmmm, please?

I've got two plugins that are useful for different minor needs.  This change makes them work, and it does it with a really obvious escape hatch.  I can't imagine a lower-stress way of doing this, and I really don't think it can come back to bite you.  I mean, anybody using this is going to _know_ that they're off in the weeds.

Would you prefer to add something like the following?

```
warnings.warn("use of non-HTTP protocols is unsupported", RuntimeWarning)
```


I can buy you beer.


@jvantuyl This isn't Kenneth sitting on the fence, this is Kenneth being super busy. =) Requests takes some time to merge pull requests because Kenneth has the final say, and he's got about a million responsibilities. You'll just have to be patient I'm afraid.

(Also, it would help if you rebased to make it easier for Kenneth to merge if he decides to.)


Commit rebased, code tested, and patience refreshed...

Beer still available.


@Lukasa you make me sound way more impressive than I am. If I ever need a PR person, remind me to hire you :P


@jvantuyl :sparkles: :beers: :sparkles: 


Yay!  @kennethreitz Look me up next time you're in San Francisco.  Beer, on me.
Redirection breaks on oddly cased protocols

Sample url:

```
http://apiservices.krxd.net/click_tracker/track?kx_event_uid=irs-h88b&kx_pixelname=zorg-sp1&clk=http://ib.adnxs.com/clktrb?id=126549
```

This monstrosity actually redirects to:

```
Https://zorg.ditzo.nl?utm_source=Startpagina_Zorg&utm_campaign=zorg&utm_medium=TXT&utm_content=txt01 [302]
```

Suggested fix in:
requests/sessions.py
    url = resp.headers['location']
to
    url = resp.headers['location'].lower()


What version of Requests are you using? I'm fairly sure we fixed this a while ago.


Yeah, we definitely did. The 2.0 release contains a fix that should fix your particular problem, and that fix was subsequently refined in 2.0.1.


It's in the latest code, I've issued a pull request.


2.0.1 does break for me with that url. Try:

```
import cookielib
import requests

url = 'http://apiservices.krxd.net/click_tracker/track?kx_event_uid=irs-h88b&kx_pixelname=zorg-sp1&clk=http://ib.adnxs.com/clktrb?id=126549'

class DeclineCookiePolicy(cookielib.DefaultCookiePolicy):
    def set_ok(self, cookie, request):
        return False
decline_policy = DeclineCookiePolicy(rfc2965=True, strict_ns_domain=cookielib.DefaultCookiePolicy.DomainStrict)

jar = cookielib.CookieJar(policy=decline_policy)
headers = {
    'Accept-Encoding': 'identity, deflate, compress, gzip',
    'Accept': '*/*',
    'Connection': 'close',
    'User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_5) AppleWebKit/536.30.1 (KHTML, like Gecko) Version/6.0.5 Safari/536.30.1'
            }
s = requests.session()
s.keep_alive = False
response = s.get(url, verify=False, timeout=30, headers=headers, cookies=jar, stream=True, allow_redirects=False)
content_length = response.headers.get('content-length', '0')
responsecode = response.status_code
elapsed = response.elapsed.microseconds
if responsecode >= 300 and responsecode < 400 and response.headers.get('location'):
    redirect_counter = 1
    print "    %d: redirect to %s [%d]" % (redirect_counter, response.headers.get('location'), responsecode)
    for redirect in s.resolve_redirects(response, response.request):
        print redirect
        redirect_counter += 1
        responsecode = redirect.status_code
        elapsed += redirect.elapsed.microseconds
        new_location = redirect.headers.get('location')
        if responsecode >= 300 and responsecode < 400 and new_location:
            print "    %d: redirect to %s [%d]" % (redirect_counter, new_location, responsecode)
            if redirect_counter > 5:
                raise requests.TooManyRedirects
        else:
            break
response.close()
```


Ugh, _facepalm_. I was running the code on a VM with an older version of requests. Apologies.


=P It's quite alright.
isalnum() in utils.py should be isdigit()

"f4".isalnum()    # True
"f4".isdigit()    # False

unquote_unreserved() will now break in `c = chr(int(h, 16))` if there's, for example, a "%es" in a url. 


Why would there ever be a "%es" in a URL?


Sadly, some doubleclick.net links are odd urls. Example I came across: 

```
http://ad-emea.doubleclick.net/clk;192366622;48236574;p?http://nl.hotels.com/?rffrid=ola.hcom.nl.%esid!.%epid!.%ecid!
```


That's not an odd URL, that's a totally invalid URL. You simply cannot have an unencoded entire URL in the query string of a URL. If doubleclick.net is genuinely providing that URL someone should take this up with them because it's totally crazy.


Well, odd url or not, doesn't hurt to make sure the percents are followed by digits before converting to int :)


Sure it does. The reason is does is that the percent encoding is in hex, which allows letters. %2A, for instance, encodes the asterisk character.


Right, good point. I completely overlooked that. :(


That said, the error is raised when you do a requote_uri() on an uri that is actually not escaped and contains percent signs. Instead of raising an exception, how about silently ignoring and continuing with `parts[i] = '%' + parts[i]` ?


`requote_uri()` explicitly expects to receive a quoted URI. It shouldn't handle being 'accidentally' passed unquoted ones. That's a whole other function.

Let's break this problem down and look at it as part of a whole. We use `requote_uri` in two places: when handling redirects, and when preparing a URL for sending. The first is 'trusted' input: the server has to be able to get that right, and if it doesn't we absolutely should fail. If a server sends us a URL with invalid percent-escape sequences in it we shouldn't attempt to guess what the server meant, we should die.

The other case is when handling user input, e.g.

``` python
r = requests.get('http://ad-emea.doubleclick.net/clk;192366622;48236574;p?http://nl.hotels.com/?rffrid=ola.hcom.nl.%esid!.%epid!.%ecid!')
```

This will fail with an `InvalidURL` exception.

The convention in Requests is that if you pass us the query string yourself, rather than letting us build it, you had better handle encoding it yourself. We need to establish such a convention to avoid burning users. For instance, they could pass us a URL with the path `?value=%20&value2=%21`. How are we to know whether that's been percent-encoded or not? They're valid percent-escape sequences, but that doesn't mean anything, maybe the user wanted literal percent symbols, e.g. `?value=%2520&value2=%2521`. For that matter, what about the ampersand? Did they want that encoded too, i.e. `?value=%2520%26value2%2521`?

Requests cannot know this in the general case. For that reason, we _have_ to make sure the user does this themselves. Only the user knows what they want here.


I support absolutely everything @Lukasa has said here.


Thanks for your input, it makes good sense.


Ok. Thanks for raising this though, it's excellent when people get in contact with their problems. Keep them coming!
Fixed #1711

I think i missed the point that `cookies` can be `Dict or CookieJar`.
This fix will work for `MozillaCookieJar`.


This diff is confusing the hell out of me. You've got a change in your commit (`sessions.pyL76`) that already exists in master. Did you rebase a whole ton of commits?


ah, sorry.
I will make another clean pull request.


(That change came from #1696, for those who are interested)
Regression 2.0.1: Using MozillaCookieJar does not work

Could not find an issue raised for this, not sure if this was an expected change either. This is reproducible on master.

Existing code fails on update to `requests-2.0.1`. The cause seems to be triggered by the change at https://github.com/kennethreitz/requests/commit/012f0334ce43fe23044fc58e4246a804db88650d#diff-28e67177469c0d36b068d68d9f6043bfR326

The parameter `cookies` expects either `Dict` or `CookieJar`. Treating `MozillaCookieJar` as a dict triggers the error in this instance.

The following code highlights the issue:

``` py
import sys
import requests
from os.path import expanduser

if sys.version_info.major >= 3:
    from http.cookiejar import MozillaCookieJar
else:
    from cookielib import MozillaCookieJar

URL = 'https://bugzilla.redhat.com'
COOKIE_FILE = expanduser('~/.bugzillacookies')

cookiejar = MozillaCookieJar(COOKIE_FILE)
cookiejar.load()

requests.get(URL, cookies=cookiejar)
```

The following `AttributeError` is thrown:

```
Traceback (most recent call last):
  File "rtest.py", line 16, in <module>
    requests.get(URL, cookies=cookiejar)
  File "/tmp/rtestenv/lib/python2.7/site-packages/requests/api.py", line 55, in get
    return request('get', url, **kwargs)
  File "/tmp/rtestenv/lib/python2.7/site-packages/requests/api.py", line 44, in request
    return session.request(method=method, url=url, **kwargs)
  File "/tmp/rtestenv/lib/python2.7/site-packages/requests/sessions.py", line 327, in request
    self.cookies = cookiejar_from_dict(cookies, cookiejar=self.cookies, overwrite=False)
  File "/tmp/rtestenv/lib/python2.7/site-packages/requests/cookies.py", line 410, in cookiejar_from_dict
    cookiejar.set_cookie(create_cookie(name, cookie_dict[name]))
AttributeError: MozillaCookieJar instance has no attribute '__getitem__'
```


Mm, good spot. I think we should try to do something smarter here. Thanks for raising this issue! :cake:


@abn can you try out the code here: https://github.com/kennethreitz/requests/pull/1713


@sigmavirus24 https://github.com/kennethreitz/requests/pull/1713 works for me.


Thanks for dobule checking @abn
Fix for AppEngine

Google AppEngine does not allow you to import netrc because netrc import pwd.

This patch moves the import from the top of the module to inside the function get_netrc_auth where the function body was already wrapped in a try catch.


I am strongly +0 on this. Requests does not support Google App Engine, and there has never been any expectation that it will function correctly on GAE. I'm not opposed to this change, because it's minor and should have no functional effect, but I equally do not want to begin to establish a tradition of establishing special-cases for GAE.

I'll let @sigmavirus24 and @kennethreitz provide their opinions instead.


@samuelhug is the the only thing that doesn't work on GAE?


@kennethreitz I haven't run extensive tests. However, I came across this issue while testing out coto/gae-boilerplate and this fix was the only modification required.


Excellent. In that case, I'm happy to merge this. As a general case, though, I think App Engine should be fixing its problems :)
restoring pull request fixing #1623

Because i thought it will not be merged, i checkout-ed auth.py in my another pull request.
As for that pull request is merged, my previous fix of auth.py to add 'MD5-sess' is deleted in master branch.
https://github.com/kennethreitz/requests/commit/3501ea463432400c77479dddedab3b7af967b618
(it's original commit)
Sorry for my mistake and sorry for confusing :(
This is pull request restoring previous fixes.
thanks.


Yeah, it would probably be best if we put this back. =P Thanks! :cake:
POST 301 turns into GET

I was trying to POST a request to a RESTful service and was having this issue:
The service is automatically redirecting all request from `http` to `https`

```
2013-10-24 14:16:54,190 - DEBUG - connectionpool.py:_make_request:414 - "POST /api/v1/account/login HTTP/1.1" 301 0
2013-10-24 14:16:54,191 - INFO - connectionpool.py:_new_conn:695 - Starting new HTTPS connection (1): https://blah.com
2013-10-24 14:16:54,271 - DEBUG - connectionpool.py:_make_request:374 - Setting read timeout to 7.5
2013-10-24 14:16:54,282 - DEBUG - connectionpool.py:_make_request:414 - "GET /api/v1/account/login HTTP/1.1" 405 0
```

After the redirect `POST` got converted into `GET` which fails this request. It doesn't seem like I got this issue in request 1.x

Any help?


Sorry I was being stupid, that's a RFC standard, closing!
v2.0.1

TODO: 
- [x] get :+1: from @lukasa and @sigmavirus24 
- [x] update changelog

Fixes #1659


/cc @Lukasa @sigmavirus24 


Generally happy with this. There's a bit of work to do to update the changelog and confirm we haven't done anything backward incompatible. I'll do that tonight [from this](https://github.com/kennethreitz/requests/compare/v2.0...master) unless one of you two wants to take it on earlier.


cutting it once this build passes
Deleted the extra word in PreparedRequest.prepare's docstring

My second PR in a night, I must be on a roll! Just one question, should I have added my name to AUTHORS in this branch as well or is it good as it is?


Awesome again, thanks! This is one I can go ahead and merge straight away, so I will.

For now assume that both PRs will be merged. In the unlikely event that they won't, we'll come back and make sure your name gets added. =)

Thanks so much again! :cake: :cookie:


Perfect, thanks a lot for all your help, I really appreciate it!
The img on the main page is not showing properly

The http://ci.kennethreitz.org/buildStatus/icon?job=requests-trunk img is not being displayed. When I open it in the browser it says 

> Service Unavailable
> The service is temporarily unavailable. Please try again later.

Am I missing something ?


Looks like it's 404ing. @kennethreitz?


[This link](http://ci.kennethreitz.org/job/requests-trunk/badge/icon) seems to work best.


@sigmavirus24 that one doesn't work when there's a referrer.


might just turn it off.


it's a weird behavior with GitHub's CDN.
Add clarification to MissingSchema error

Just a short bit that suggests a possible fix to the problem


LGTM. Thanks! :cake:


Since we're working on Python 2.6+ which supports `str#format` wouldn't it be easier to do

``` python
raise MissingSchema("Invalid URL {0!r}: No schema supplied. "
                    "Perhaps you meant http://{0}?".format(url))
```

Then there's no reason to use `(url, url)`


Agreed, just wanted to be consistent with the rest of the file. Fixed in b5d2160


I might go through and update all of these to use `#format` instead in the library. It is the "future" after all. Any objections @Lukasa @kennethreitz ?


And thanks @kevinburke. LGTM!


@sigmavirus24 Go for it. =)
Requests Session problem, it does not handle headers automatically.

I must add some headers to POST, and remove some headers to GET.

For example, if I do not use:
s.headers.update({'Content-Length':''})
s.headers.update({'Content-Type':''})

before the POST, it will not post correctly.

Also if I have Content-Type and Content-Length in my headers, GET will not work properly.

I'm manually adding and removing them all the time, I thought Requests did handle it automatically, doesn't it?

Also, there's a page that in order to get a response, you must have these in the headers:
s.headers.update({'If-Modified-Since':''})
s.headers.update({'If-None-Match':''})

If I do not add it manually, it returns 404.


What data are you trying to post and to what service? You should never be setting the `Content-Length` header since we handle those for you. You should only set the `Content-Type` when you're actually posting some data but this sounds like the server you're trying to use is broken or you have other things set that should not be.

Further more, we do not set headers like `If-Modified-Since` or `If-None-Match` because we can not know when a server requires them. That is your job.

As for "automatic" headers, there are only some which are set automatically and the rest we do not set. For the last two, you should be checking the headers of the previous response for `Last-Modified` and `ETag` headers and using those values respectively.

This is not an issue with requests.


Okay, I just thought it would be automatic.
Thanks for the answer.


If you'd like help, send me an email or hop onto IRC.
Support for bandwidth throttling

Are there plans to allow limiting the amount of available bandwidth in streaming mode? This would be useful in particular when uploading or downloading several large files simultaneously. Being able to specify a global cap or at least a cap at the individual request level would prevent streams from sucking all available bandwidth.


There are no plans for such and I believe it is currently completely untenable. There is no support like this in any other library like requests that I know of and the place to add anything like this would be in shazow/urllib3. I'm not even entirely certain it would be possible to do in urllib3, especially since I find it hard to believe this is even possible with httplib (which urllib3 uses).

I'll leave this open so @Lukasa and @kennethreitz can comment, but not only am I :-1: on the idea in general, I'm also pretty sure it is not technically possible at the moment.


Thanks for the quick response @sigmavirus24! I think this could be very useful in bandwidth constrained environments but admittedly I'm unfamiliar with requests inner workings so I completely understand if it's unfeasible to implement. Let me know if this does become a possibility in the future roadmap. Happy to contribute any way I can.


The answer is it's not in the roadmap. There are discussions afoot about removing urllib3's dependence on httplib, and if that ever happens it's a discussion we can reopen. However, as @sigmavirus24 quite rightly suggests, even if urllib3 could do it we wouldn't expose it. There's just not a nice way to do it.
Problem POST'ing png file because of UnicodeError

Here is the code I'm using:

``` python
files = {'file': (upload_handle.upload_token.key, open("test.png", "rb"))}
resp = requests.post(url, files=files)
```

This raises the error:

```
UnicodeDecodeError: 'utf8' codec can't decode byte 0x89 in position 140: invalid start byte
```

This problem is caused by the fact that the content-length header is actually a unicode object. When the actual body of the request is being constructed, python attempts to coerce the entire request into unicode resulting in the decode error.

After tracing it, the cause is the following lines:

requests/models.py: 

```
self.prepare_content_length(body)
# -------
l = super_len(body)
self.headers['Content-Length'] = str(l)
```

where `str = unicode` is declared in compat.py


Yep, that's a crass little bug. Thanks so much for pointing it out! I'll go through the headers we set and make sure we don't do this anywhere else. =)
Custom Sphinx theme

It would be really cool if Requests had its own theme for Sphinx (or moved off of Sphinx ..?) with a design to match.

(I'm not offering to do this, haha... would be a good chance for someone in the community though ?)


We already do: http://docs.python-requests.org/en/latest/


@kevinburke I'm quite baffled by this. How do we not already have a custom theme? Certainly we share it with Flask and the other Flask related projects that use it (and anyone else who decides to use it contrary the license) but this is not a standard or built-in theme.

Also, why move off of Sphinx? We would give up the automatic updates provided by rtfd.org.


I agree moving off Sphinx is pretty dumb, I just figured it would be nice if, when you went to `python-requests.org` it was immediately obvious "Oh! I'm on the Python Requests website," the same way when I'm on Twitter Bootstrap's homepage I know it's the Bootstrap website.

I guess if you want to share the identity with the other Pocoo sites then I understand. 


It would not be awful to maybe redesign it.


Most people associate the theme with Requests now, not Flask :)
Raise error instead of using invalid user-supplied content-type

#### Problem

When making a POST request with the `files` argument, one should not specify the `'content-type'` header. This is because internally, requests forms the content-type header with a boundary string matching that used in the body; if this boundary string is not present in the header then there will be a server-side error. However, if the user does supply a content-type header, then although requests still forms the correct header internally, it silently drops it in favor of the user-supplied header, which it knows to be invalid, because the user has no way of knowing the boundary string that requests will send in the body.
#### Example

``` python
from test_requests import httpbin
import requests
url = httpbin('post')
with open('requirements.txt') as f:
    resp = requests.post(url, files={'some': f},
                         headers={'content-type': 'multipart/form-data'})
    resp.raise_for_status()

# requests.exceptions.HTTPError: 500 Server Error: INTERNAL SERVER ERROR
```
#### Proposed fix

This PR raises an exception if the user attempts to supply a value for the `'content-type'` header when making a POST request with the `files` argument.

The example above now results in a client-side `ValueError`:

``` python
# ValueError: 'content-type' should not be specified in headers dict when using the `files` argument.
```


I generally lean towards the side of allowing the user to shoot himself in the foot if they so desire. 

The question is: did this behavior surprise you?


> The question is: did this behavior surprise you?

That's not the question I would ask. I see this question a lot on 
StackOverflow. I usually admonish people while answering their question but I 
think it is common enough to either raise the error or silently ignore them.  
On the other hand, I'm quite fine with letting people shoot themselves in the 
foot. This is HTTP for Humans, not for people who read enough of the docs to 
figure out what they want without taking the time to realize that none of the 
examples set that header. We can amend the docs to either be painfully 
explicit about this (which I doubt will help much) or we can take action to 
prevent the user from setting it in cases like this. I'll just reiterate that 
if we're going to start protecting users from themselves, we should do it for 
at least the Content-Length header as well. That's another one people set 
without recognizing that they don't have to and absolutely shouldn't


Unfortunately, users setting the `Content-Type` header explicitly seems very common. Equally common is users specifying `Host`, `Content-Length` and `User-Agent`, all of them unnecessarily.

With that said, I think I'm with @kennethreitz on this one. While I can't think of a reason a user would legitimately want to do this, they _might_ (I guess misbehaving servers could be a reason). If they do, we shouldn't be getting in their way.


:cake:
models.text Behaviour (encoding choice)

I have a question about the models module behaviour, around the **text** method.
When no encoding is given, there is an encoding detection, which is a good thing : 

```
if self.encoding is None:  
    encoding = self.apparent_encoding
```

When an encoding is given, first try use it for decoding : 

```
try:  
    content = str(self.content, encoding, errors='replace')  
except (LookupError, TypeError):  
    content = str(self.content, errors='replace')
```

What I don't understand is, when an exception occurs, why it is not tried to use the **self.apparent_encoding** method, to do the 

```
content = str(self.content, errors='replace') 
```

in a more clever way.

It then would be something like that : 

```
try:  
    content = str(self.content, encoding, errors='replace')  
except (LookupError, TypeError):
    encoding = self.apparent_encoding
    content = str(self.content, encoding, errors='replace')
```

Wouldn't it be better to try to detect the encoding instead of using locale one ?


With the caveat that I hope you understand this would not solve your issue because we aren't catching that SystemError, I actually think this wouldn't be a bad idea if `charade` weren't painfully slow. If you'd like to make charade faster, then by all means we could do this, but I think the primary concern would be one of speed.

`charade` is pretty accurate but that comes at the cost of being slow. There are alternative which are very fast but not as accurate. And of course, character encoding detection is a very inexact science in the first place. This is evidenced by the fact that Chrome/Chromium detected your content as ISO-8859-1 and charade as ISO-8859-2. There's no guarantee that there wouldn't be cases where that would raise an issue.

Furthermore, assuming charade gets it wrong, then it's entirely possible that we'll still see a really awful exception because that call in the except block would not catch any errors thrown by the second call. Yes we don't handle that now but that's because we should raise an exception in the event we can't decode using that either.

What catching those two would look like is:

``` python
try:
    content = str(self.content, encoding, errors='replace')
except (LookupError, TypeError):
    pass
else:
    try:
        content = str(self.content, self.apparent_encoding, errors='replace')
    except (LookupError, TypeError):
        content = str(self.content, errors='replace')
```

I'm not quite fond of that. And I'm not sure if it could be extracted to a function/method that would work the way we want easily.


Closing to focus discussion on #1737
Wrongly declared UTF-7 webpage makes requests crashing

Example webpage : http://www.patisserielidy.com/
**HTTP headers** : 

```
HTTP/1.1 200 OK
Server: Apache
X-Powered-By: PHP/5.3.26
X-Pingback: http://www.patisserielidy.com/xmlrpc.php
Transfer-Encoding: chunked
Content-Type: text/html; charset=UTF-7
```

**Declared encoding** : 

```
<meta http-equiv="Content-Type" content="text/html; charset=UTF-7" />
```

**Detected encoding by Chromium** : 

```
ISO-8859-1
```

Exception thrown on requests/models.py:664 when trying to get content of the request result, with the attribute text.

**Stack** : 

```
File "/home/glebourg/dev/companies/pycompanycrawler/virtualenv/lib/python3.3/site-packages/requests/models.py", line 664, in text
    content = str(self.content, encoding, errors='replace')
  File "/home/glebourg/local/datapublica/lib/python3.3/encodings/utf_7.py", line 12, in decode
    return codecs.utf_7_decode(input, errors, True)
SystemError: invalid maximum character passed to PyUnicode_New
```

It seems the trial of conversion from UTF-7 to UTF-8 causes an unexpected error (SystemError, while only LookupError, TypeError are planned to happen there).

I have also filled a report on Python bugs tracker : http://bugs.python.org/msg200117


As a quick workaround for this case, you can either set `Response.encoding` to the correct encoding, or set it to `None` if you want us to guess.

I'll have a think about whether we should do something smarter here, but it doesn't help that an unexpected exception is being thrown by the stdlib.


So if you were to set it to `None` as @Lukasa suggests, we would guess that it was encoded as `ISO-8859-2` instead of what Chrome detects it as.

I tried this on OSX w/ Python 2.7.5. It appears you're using 3.3 though and that may be entirely relevant too. I'll test with that too.

With Python 3.3, using `r.text` I too get the exception and the same guessed encoding. Setting `r.encoding` to `None` does not cause the exception to be raised though.

To be fair, I'm not sure the exception is entirely incorrect on Python's part. It seems like an extraordinary exception that they were not even planning on finding. That said, I'm sure they'll either update the documentation or they'll fix it to return either of those two errors (more likely LookupError).

I'm also bugging someone who can get your bug report out of the spam filter.


Python commiters released a patch : http://bugs.python.org/issue19279
So we can say this issue really is on the Python side. Should come with 3.4 release.
Certificate name checking does not honor the Host header

When verifying SSL certificates, requests only uses information from the URL string passed to `get()`, and ignores the HTTP Host header (passed in with the optional `headers` argument). This causes a failure in my case because the URL I am using is actually a local IP address. (If I use the domain name, it resolves to the server's public IP, but I need to work with the local one.)

In other words, I expect

```
requests.get('https://192.168.X.X/' headers={'Host': 'example.com'})
```

to work as long as the certificate for 192.168.X.X matches example.com (or *.example.com). Instead, it throws an exception. Is this a bug that could be fixed, or is there some other way to accomplish this task that I am missing?


@sigmavirus24 Should we be filing this under our long list of "Do we trust the Host: header" bugs?


@Lukasa I think so. My concern is that we keep special-casing areas of the library around this and I really really hate that.


Agreed. We need a better way of getting host information. Perhaps a property on the `Request` object?


I can't envision anything off the top of my head. What do you have in mind?


Something like

``` python
class PreparedRequest(object):
    @property
    def host(self):
        if 'host' in self.headers.keys():
            return self.headers['host']
        else:
            return urlparse(self.url).host
```

I think there are a few other places where we consult the URL for this information, so this might work a bit better. Maybe worth renaming, because sometimes we _do_ want the URL host.

Quick, tell me why it won't work. =)


This works fine for me. For the times we do want the URL host, we should have a property that is the parsed url, e.g., 

``` python
@property
def parsed_url(self):
    return urlparse(self.url)
```


Also a good idea. This would be a useful pair of fixes.

Can you thin of a better name for that `host` property though? The best I have is `asserted_host`, which frankly sucks pretty hard.


@Lukasa do we still want to do this?

I can't think of a better name for the attribute frankly, but a better implementation might be:

``` python
class PreparedRequest(object):
    @property
    def parsed_url(self):
        return urlparse(self.url)

    @property
    def host(self):
        return self.headers.get('host', self.prepared_url.host)
```

That should be slightly faster than checking if the header is in the dictionary and then retrieving it. 

I don't like `asserted_host` at all.

I continue to be :-1: on changes like this though. Further, do we even do any of the certificate name checking or is that entirely in urllib3?


Mm, urllib3 seems to do it. Maybe that's the place to look.


It should be possible to create a TransportAdapter for `https://192.168.X.X/` which passes the `assert_hostname` parameter to urllib3, which tell urllib3 to use another hostname.
[WIP] Better chunked handling.

Following #1642, I think we should treat urllib3's pooled connections better when we manage them ourselves. This PR provides the following extra functionality:
- Clean up connections when we hit problems during chunked upload, rather than leaking them.
- Return connections to the pool when a chunked upload is successful, rather than leaking it.

The diff is a little unclear, so let me clarify: I wrapped most of the chunked code in a `try` block, then added a catch-everything exception handler that will attempt to close the connection if we hit it. Otherwise, we'll return the presumably-fine connection to the pool.

I'd like some code review on this. @sigmavirus24, can you confirm this is doing what I think it is, and can you give me some suggestions for how best to test it? @shazow, if you have time, can you confirm that I'm obeying the semantics of the urllib3 connection pool? (e.g. should I check if we get a `Connection: close` header on the response?


Ideally the streaming bit should be a feature of urllib3. Otherwise you're just circumventing half of what urllib3 does, so there's not much semantics to obey—you're working directly with httplib objects. :) For returning the connection, urllib3 will do the right thing if it's closed IIRC.


Hmm, are you interested in me moving some of this functionality to urllib3 at some point?


@lukasa For sure. I recall some streaming-related issues open for urllib3 but I'm on superbad internet right now so I can't look for them. Just take a quick look for them before you dive in, then have at it. :)


@Lukasa this definitely looks correct. If there's a better way to do this in urllib3, then I'll hold off on providing testing advice, especially since the best way I can think to test this particular bit is through a urllib3 inspired manner.


I'm not comfortable with merging this in until we have our test suite working again. Maybe I'll do that tonight.


@kennethreitz the only thing failing in the tests are the regression tests I added to prevent cookies from being lost on authentication challenge responses. If you run the tests against a local HTTPBin instance (based off of master) then you'll see green. Regardless, I think this wouldn't hurt if we merged it early as a sort of bug fix. I would, however, like to see some of the functionality move up into urllib3 though.


@sigmavirus24 :cake:


My plan was basically to provide this in Requests until we can move all of the chunked upload functionality to urllib3, when we can remove this nasty branch of code and pretend it never existed.


Pretend what never existed? ;)


Should we merge this? Should this be a part of 2.1?


#yolo
Requests eats up entire CPU and occasionally closes the stream early

``` python
import requests

expected = 11424397 # whatever size the file should be

for i in range(0, 100):
    size = 0
    print "Downloading %d..." % i
    r = requests.get("http://somehost.com/bigfile.mp4", stream=True)

    while True:
        d = r.raw.read(1024 * 1024)
        if not d: break
        size += len(d)

    print "Size %d and expected %d" % (size, expected)
    assert size == expected, "Size %d != expected %d" % (size, expected)
```

In trying to download a large file, the above code exposes two apparent problems:
- Regardless of the read size, the CPU gets pegged
- Periodically one of the streams closes early

Reported for @zedshaw since he's apparently blocked from the repo.


Additional details:
- OSX
- Same thing happens with iter_content
- Requests version:  2.0.0


Taking a look at this, I can't reproduce in an interactive interpreter. I'll try with a script, but I'm going to get the size of this file first.

Downloading `http://download.fedoraproject.org/pub/fedora/linux/releases/19/Live/x86_64/Fedora-Live-Desktop-x86_64-19-1.iso` puts my CPU usage at ~5% from Python according to OS X's Activity Monitor when done in the REPL. `top` basically agrees as well.


Yup, still unable to reproduce, even from a script. Get the occasional CPU jump from a GC run, but otherwise it's exactly as low as I'd expect. I think we need a more accurate repro scenario: what Python version, and what file (if possible).


@zedshaw if you can see this and can provide more info, please email me. I hope we can figure out who blocked you and why.


This issue seems like it has died, so I'm closing it for inactivity.
Second time lucky for json related fixes.

Following discussion on #1665. =)


:+1: :+1: :cake: :cactus: :cake: :+1: :+1: :all_the_emoji:


(I created a new label just for this PR)


Awesome new label. Time to apply it to ALL THE THINGS. =D


all the sparkle cake sparkles


I can't wait to have CI working again. But, one must have his priorities in line. 

/me off to play Pokémon
access socket

Hi, can I access earlier socket to make new POST request to same server, because I don't want to waste time to make connection with server so

server.com
GET /index.php

server.com
POST /somewhere/in/www.php
param=value&param=value

I want to be able to make POST as soon as possible, so I don't want to close connection and reuse earlier socket/session


so it would look like this

def callback( session, response ):
    session.getSocket().post( '/path', headers=headers, data=postData )


Hi there! Thanks for raising this issue!

Firstly, you've opened this question in two places, here and Stack Overflow. Stack Overflow was the correct location for this question, you didn't need to open it here. =)

Secondly, this is not actually a direct Requests question, it's a question about [requests-futures](https://github.com/ross/requests-futures), which is a separate project managed by the awesome @ross. In particular, vanilla Requests doesn't provide any form of background callback.

However, for standard Requests, you can simple continue to use the same `Session` object. `Session` performs connection pooling, so as long as the previous connection is still open (the server's choice) we'll re-use it.
Adding myself to AUTHORS.rst

Taking a page out of @mattspitz's book.

https://github.com/kennethreitz/requests/pull/1439
# dreamsdocometrue!


I should add, this is partly in jest since I know @mattspitz.

My contribution is probably not as substantial as others', in which case #dreamsdonotcometrueaseasilyasihoped.


if anyone has submitted as much as a typo fix, their name should be appended.


Love it.


Hi, @voberoi!

It is a [Dream Come True](http://www.youtube.com/watch?v=HH5lEIhjOHY).


@mattspitz you're just filled with those youtube videos, aren't you? :P


@mattspitz writes the best test cases.


:heart:
Increasing super_len compatibilty to include BytesIO and cStringIO objects.

Added a check for 'getvalue' attr, calling it to retrieve the length if we can.

We also try/except the fileno() call, which can throw
io.UnsupportedOperation for BytesIO because, well, they're not files.

And a unit test!


Looks good to me! Add yourself to the AUTHORS file and then wait for Kenneth to get some time to take a look. =)

Thanks so much! :cake:


Added!  Thank you!

:crocodile:
Session hooks broken

Request hooks are being [merged](https://github.com/kennethreitz/requests/blob/master/requests/sessions.py#L264) with session hooks; since both hook dicts have a list as the value, one simply overwrites the other.


Great spot, thanks! We should improve our merging logic here.


I might take a crack at this in an hour or so


Hm. This has always been the behaviour of how per-request hooks work with session hooks but it isn't exactly intuitive. My concern is whether people are relying on this behaviour since the logic in `merge_setting` hasn't really changed in over a year (at least).

I have to wonder if this did the same thing on older versions of requests or if I'm just remembering incorrectly. Either way, the simplest solution would be to not try to special case this inside of `merge_setting` but to use a different function, e.g., `merge_hooks` (or more generally `merge_lists`) to create a merged list of both.

The simplest way to avoid duplication is: `set(session_setting).merge(request_setting)`.

Still I'm not 100% certain this is the expected behaviour and I don't have the time to test it right now. I'll take a peak later tonight.


Let me clarify: session hooks are completely ignored (in version 2.0.0) regardless of whether any per-request hooks were set. They used to work in Requests 1.2.3.


Ah that's a big help.


@mgax can you verify this commit fixes this for you? https://github.com/sigmavirus24/requests/commit/5bf396d5d12bf9debc667a509c84f640560da517


@sigmavirus24 :+1:
Requests doesn't catch requests.packages.urllib3.exceptions.ProxyError

Requests doesn't catch requests.packages.urllib3.exceptions.ProxyError and translate it into a requests module specific exception which derives from RequestException as it does for other errors originating from urllib3. This means if trying to catch any exception derived from RequestException so as to treat it specially, the urllib3 ProxyError will be missed.


Simple test script is:

```
import requests

PROXY_HOST = 'http://localhost'
PROXY_HTTP_PORT = 8080

REMOTE_URL = 'http://www.python.org'

proxies = { 'http': '%s:%s' % (PROXY_HOST, PROXY_HTTP_PORT) }

response = requests.get(REMOTE_URL, proxies=proxies)

print(len(response.content))
```

Without change get:

```
requests.packages.urllib3.exceptions.ProxyError: Cannot connect to proxy. Socket error: [Errno 61] Connection refused.
```

With the change, get:

```
requests.exceptions.ProxyError: Cannot connect to proxy. Socket error: [Errno 61] Connection refused.
```


@GrahamDumpleton is the best type of contributor.


@GrahamDumpleton is the best kind of everything
max_retries does not appear to apply to timeout

This server

```
import time

request = 0

def application(environ, start):
    start("200 OK", [])
    global request
    request += 1
    if request >= 5:
        return ["GOODBYE\n"]
    time.sleep(1)
    return ["HELLO\n"]
```

seems to fails immediately with:

```
session = requests.Session()
session.mount("http://", requests.adapters.HTTPAdapter(max_retries=5))
session.mount("https://", requests.adapters.HTTPAdapter(max_retries=5))
session.get("http://localhost:8080", timeout=0.5)
```

(I opened shazow/urllib3#251 because I assumed it was a urllib3 issue, but he redirected me back here)


Definitely still a urllib3 bug. =) @shazow is playing silly buggers, I think. I've put a reproducible script in the urllib3 issue. So you don't feel like you're being messed around I'm going to leave this issue open, but I'm convinced it's upstream.


Thank you :)

(For doing both of those things.)

Cheers.
Headers not sent when POST'ing generator

```
#!/usr/bin/env python3
import requests

def c():
    yield b"a"

print(requests.post("http://localhost:8801/", data=c()).text, headers={"content-type": "application/x-www-form-urlencoded", "x-donkey": "balls"})
```

The server sees:

```
POST / HTTP/1.1
Host: localhost:8801
Accept-Encoding: gzip, deflate, compress
Accept: */*
Transfer-Encoding: chunked
User-Agent: python-requests/2.0.0 CPython/3.3.1 Linux/3.11.0-031100rc4-generic

1
a
0
```

If it is not possible to set headers on these requests, I would expect an exception.


You're not sending the Headers in the request. Count your parentheses carefully, as I'll demonstrate by newlines:

``` python
print(
    requests.post(
        "http://localhost:8801/", data=c()
    ).text, headers={"content-type": "application/x-www-form-urlencoded", "x-donkey": "balls"}
)
```

The correct syntax is:

``` python
print(requests.post("http://localhost:8801/", data=c(), headers={"content-type": "application/x-www-form-urlencoded", "x-donkey": "balls"}).text)
```


Oh. Well, sorry then. You rule :)


No need to apologise! =)