Raibek/requests-github-issues huggingface dataset:
Setting a custom header to integer 0 (zero) results in 'int' object is not iterable error

When a custom header has a value of integer 0, here's the stacktrace:

```
Traceback (most recent call last):
  File "gtest.py", line 169, in <module>
    parse()
  File "gtest.py", line 158, in parse
    add_to_group(domain, argz[0], argz[1])
  File "gtest.py", line 107, in add_to_group
    r = requests.post('https://apps-apis.google.com/a/feeds/group/2.0/%s/%s/%s?prettyprint=%s' % (domain, groupname, email, debug), headers=headers)
  File "/home/pas/.virtualenvs/v/lib/python3.2/site-packages/requests/api.py", line 98, in post
    return request('post', url, data=data, **kwargs)
  File "/home/pas/.virtualenvs/v/lib/python3.2/site-packages/requests/safe_mode.py", line 39, in wrapped
    return function(method, url, **kwargs)
  File "/home/pas/.virtualenvs/v/lib/python3.2/site-packages/requests/api.py", line 51, in request
    return session.request(method=method, url=url, **kwargs)
  File "/home/pas/.virtualenvs/v/lib/python3.2/site-packages/requests/sessions.py", line 188, in request
    headers[k] = header_expand(v)
  File "/home/pas/.virtualenvs/v/lib/python3.2/site-packages/requests/utils.py", line 260, in header_expand
    for i, (value, params) in enumerate(headers):
TypeError: 'int' object is not iterable
```

Just run into this while trying to work around the missing Content-Lenght header for empty POST requests. (Installed via pip.)


Seems like this might be something I introduced. I'm looking into it right now.


Headers should be strings, really.


Ah, well then feel free to ignore 865. Jumped on this a bit too quickly it seems. :P


You can convince me otherwise :)


Oh I have nothing invested in this. It was a quick/simple fix. I just thought I'd broken something (again). I was under the same impression (that headers should be strings).


So it's for Humans, as long as they are RFC compliant then? :)

It was just confusing, because the error message made no sense, why would it iterate over a header value anyway; then after lurking a bit in the code I saw that it's possible to pass nice nested dicts as values.
Data of array not converted properly on content-type application/x-www-form-urlencoded

if you try to post `data={'key': ['value1', 'value2']}` with `headers={'content-type': 'application/x-www-form-urlencoded'}` it's translated into `key=value1&key=value2`.

while it should be `key[]=value1&key[]=value2`

the workaround for that is to do: `data={'key[]':['value1','value2']}`


Is there a spec for this somewhere?

I've never seen the foo[] semantic anywhere outside of PHP.


I'm with @shazow on this one, and I hadn't even seen it in use with PHP until I saw it [here](http://stackoverflow.com/questions/353379/how-to-get-multiple-parameters-with-same-name-from-a-url-in-php).


I had to do that while trying to use this API:

https://github.com/serverdensity/sd-api-docs/blob/master/sections/alerts.md


I haven't heard of this either :)


it's probably just bad api implementation made by serverdensity


Agreed :)
parsing encoding utf-8 page doesn't as expected

Test code:

```
#!/usr/bin/env python
import httplib
import requests

def test_by_req():
    url = "http://183.61.6.11/playerSearch.php"
    headers = {"Host" : "lolbox.duowan.com"}
    r = requests.get(url = url , headers = headers)
    assert r.status_code == httplib.OK
    print r.text


def test_by_httplib():
    path = "/playerSearch.php"
    headers = {"Host" : "lolbox.duowan.com"}
    conn = httplib.HTTPConnection(host="183.61.6.11")
    conn.request(method = "GET", url = path, headers = headers)
    resp = conn.getresponse()
    assert resp.status == httplib.OK
    print resp.read()


if __name__ == "__main__":
    test_by_req()
    test_by_httplib()
```

httplib works well, but requests doesn't, please confirm and fix it.


Please be cordial, and don't ask me to fix things for you?

---

The URL you are providing doesn't provide proper headers, so requests is guessing. You should be using `response.content`


Thanks for your reply.

---

I do add following into NGINX instance configuration file

```
...
charset utf-8;
...
```

then it generates proper header

```
Content-Type: text/html; charset=utf-8
```

instead of the former incorrect one

```
Content-Type: text/html
```

Everything works well right now.
response.raw.read() not working

The following code works, but will load the entire file in memory:

```
 def save_url(url, file_handle, size=(8 * 1024)):
     req = requests.get(url)
     file_handle.write(req.content)
```

This code, designed to improve upon the above code, writes 0 byte files without error. I'm having a hard time seeing where I've gone wrong in the documentation, but it simply seems that the read() socket is not functioning.

```
def save_url(url, file_handle, size=(8 * 1024)):
    req = requests.get(url)

    while True:
        chunk = req.raw.read(size)
        if chunk:
            file_handle.write(chunk)
        else:
            break
```

What do I need to do to make the read() socket work? Or is there a bug in Requests?


The second bit of code should do this:

``` python
def save_url(url, file_handle, size=(8 * 1024)):
    req = requests.get(url, prefetch=False)  # <<< prefetch is the key

    while True:
        chunk = req.raw.read(size)
        if chunk:
            file_handle.write(chunk)
        else:
            break
```


Ah! That worked!

The documentation seems to lack a good call out that if you want to use the raw socket, you need to specify prefetch=False.

Thanks again,
Brian


Pretty sure it is described in the docs but I'm glad to help. Care to close the issue for @kennethreitz?
No connection pooling for proxies

It appears to me that connection pooling plain doesn't work if you're connecting through a proxy. 

In the [Request.send method](https://github.com/kennethreitz/requests/blob/develop/requests/models.py#L546), when connecting through a proxy, instead of using the provided self._poolmanager object, it uses the static module-level function proxy_from_url. This instantiates a new connection pool for each request.

This is obviously partly an issue with urllib3, and it seems like this code is changed by the SOCKS proxy changes being considered for urllib3.

In the mean time it makes requests unusable for high-performance connections through a proxy.


For whatever proxies urllib3 already supports, there's no reason why it shouldn't be able to reuse the same connection. I suspect this is a requests-level thing.


Oh yes, it's just struck me how this should work. If there's a proxy involved then we should be using a ProxyManager instead of a ConnectionManager. So `proxymanager.proxy_from_url` should be called when the session is getting set up, instead of per-request.

However, I don't think it's that simple if a separate proxy is used for HTTPS, because then we'd need a separate ProxyManager for HTTPS connections.


This will be fixed during the refactor for Transport Adapters.


And it was. =)
requests adds packages directory to sys.path

Importing requests adds its internal packages directory to sys.path.  This causes problems on Python 3, because modules that try to use chardet end up importing requests.packages.chardet (which is incompatible with Python 3), even if a Python 3 chardet is available.

For example, beautifulsoup4:

``` Python
>>> import requests
>>> import bs4
>>> bs4.BeautifulSoup(b'\xae')
Traceback (most recent call last):
  File "<pyshell#4>", line 1, in <module>
    bs4.BeautifulSoup(b'\xae')
  File "C:\Python32\lib\site-packages\bs4\__init__.py", line 169, in __init__
    self.builder.prepare_markup(markup, from_encoding))
  File "C:\Python32\lib\site-packages\bs4\builder\_lxml.py", line 68, in prepare_markup
    dammit = UnicodeDammit(markup, try_encodings, is_html=True)
  File "C:\Python32\lib\site-packages\bs4\dammit.py", line 223, in __init__
    u = self._convert_from(chardet_dammit(self.markup))
  File "C:\Python32\lib\site-packages\bs4\dammit.py", line 30, in chardet_dammit
    return chardet.detect(s)['encoding']
  File "C:\Python32\lib\site-packages\requests\packages\chardet\__init__.py", line 21, in detect
    import universaldetector
ImportError: No module named universaldetector
```

If you install chardet2 (the version that works with Python 3), and import that module before importing requests, it works fine:

``` Python
>>> import chardet
>>> import requests
>>> import bs4
>>> bs4.BeautifulSoup(b'\xae')
<html><body><p>®</p></body></html>
>>> 
```

I reproduced this issue on Linux and Windows 7, with Python 3.2.3 and 3.3.0rc2.


I think this is an okay caveat for now. I'd be open to other suggestions though.


Is it possible to rename the modules in requests.packages so they're less likely to conflict with other modules (ie, "requests-chardet")?  Or have the requests setup script exclude (or rename) requests.packages.chardet when installing on Python 3, since that package should never be used in that case?


Where do you see this happening? I don't see any sys.path munging in production code except for `requests._oauth`: [https://gist.github.com/62b0d4614e885b9ba4b7]


requests._oauth is where it modifies sys.path.  When you import requests, it imports models which imports auth which imports _oauth.  If oauthlib isn't available elsewhere, then _oauth modifies sys.path.

``` Python
Python 3.2.3 (default, Apr 11 2012, 07:15:24) [MSC v.1500 32 bit (Intel)] on win32
Type "copyright", "credits" or "license()" for more information.
>>> import sys
>>> sys.path
['', ...]
>>> import requests
>>> sys.path
['C:\\Python32\\lib\\site-packages\\requests\\packages', '', ...]
>>> 
```


Yeah, I hadn't tried this before, but it certainly affects python 3k, even on linux.

I'm pretty sure simply doing:

``` python
# ...
except ImportError:
    # ...
    sys.path.insert(0, path)
    # ...
    sys.path.remove(path)
    # or you could so sys.path.pop(0)
```

I see nothing objectionable with that. We have imported what we need so we can leave the path alone, unless this is relied upon elsewhere.
added __len__ to response.

#854

I added **len** . I have it checking the headers first for content-length. If it must it will do check length of content, but that should be avoided if possible, right?


I messed this up. Didn't pull the latest before updating.


That isn't a problem since Travis showed that it would merge without a problem. You can also do:

```
git pull git://github.com/kennethreitz/requests.git --rebase
git push --force
```

to get it to update the pull request. Please don't open a new one.
Add response.__len__

Do people think it would be nice to have a response.__len__ simply as a convenience function? 


How well would this work with streaming APIs?


good point. it probably wouldn't work well. I will do some testing.


is there anyway with httpbin to simulate infinite streaming? I think thats the main concern, right?


I'm no expert on streaming APIs but it wouldn't make sense to me that they would provide a Content-length header. Also, if you were to measure the length of what has been sent, you would have to do a length measurement on something like `r.raw.read()` which would also consume what the user is looking for.

As for simulating infinite streaming on httpbin use the endpoint 'stream' followed by the number of lines you wish to stream, e.g., httpbin.org/stream/1000


so, looking at how Response.content handles things, it runs iter_chunk until there is nothing left. In the case of an infinite stream, this would also break. Do we think that its okay to assume that people who are making a request to an infinite stream would know not to call len, just like they know not to call .content.  I think it should be okay to first check for a content-length header, and fall back on checking the len of Response.content.


Whilst working on a library "for humans" I don't know if it's ever safe to assume anything. I for one frequently advise new users to use requests when working with anything and sadly most of them come back asking silly questions that could be answered by looking at the documentation. Anyway, in the end the decision is not mine. 


I feel the same way. I think though that adding **len** would help more people than it would confuse....
Resolve issue in #849

Resolves issue discovered in https://github.com/kennethreitz/requests/pull/849 for fixing empty proxies.

Thanks to all in that thread who helped in this!


Apparently working with a new branch doesn't do me any good, heh.  Will fix if wanting just the new commit to be in pull request.


Actually the second commit hasn't finished being tested yet. The first commit doesn't pass but that's that commit alone. Give it a second. (And it just went green for me)


I thought doing a new branch would also eliminate the commit from before, but... :(  All well, Travis approves of the new build though, so it's a bonus.


:cake:


@securityforus it all depends where you branch from.
Fixed issue with empty proxies being passed

Redone pull for fixing proxies using empty proxies as if they are real.

I.e.: if you had this set up:

``` python
proxy = {"https" : ""}
```

Requests would try to connect via empty proxy ("") instead of through a regular connection.


Apparently Travis says this doesn't build against 3.2, but as I don't have that environment set up I wasn't able to test it.


You could iter over a copy to prevent the runtime error?

``` python
for proxy_type,uri_ref in self.proxies.copy().items():
    if not uri_ref:
        del self.proxies[proxy_type]
```

Not a huge fan... but should work.


That might work.  The problem from my end though is I don't have a 3.2 environment set up, and to make it happen on my system would mean compiling Python from source, which to be honest I'm too lazy to do when I'm focused on 2.7.

Another option would be to basically do what you mentioned, but instead do this:

``` pthon
tmp_proxies = self.proxies

for proxy_type,uri_ref in tmp_proxies.items():
    if not uri_ref:
        del self.proxies[proxy_type]
```

But this is more so just guessing and thinking back to my C/C++ days when I had to do similar to solve issues.


just push up and see what travis says... thats what i did in #843.

If your worried bout commit history, when your satisfied do a squash merge into a new branch and pull request that branch.


``` python
tmp_proxies = self.proxies
```

I think that will have the same run time error, cause you would be still changing the dictionary you are iterating over.


I actually think this is because `dict.items()` returns an iterator now instead of a list like in python2. This should be fixable by doing `list(dict.items())` but I could be wrong. Another option would be to do:

``` python
tmp_proxies = self.proxies.copy()

for ptype, uri_ref in tmp_proxies.items():
    if not uri_ref:
       del self.proxies[ptype]
```

Since you're not changing the dictionary you're iterating over (although this is just an expansion on what @barberj was say).


i think list() is the right approach here


Yeah just came back to say that I tested it in the interactive console like so:

```
>>> d = {'a': 1, 'b': 0, 'c': 1}
>>> for k, v in list(d.items()):
...     if not v:
...             del d[k]
... 
>>> d
{'a': 1, 'c': 1}
```
Reset PYTHONDONTWRITEBYTECODE after installation

Fixes #839.


I actually think that Python just checks for the existence of the environ, so we should remove it completely if it didn't exist. 


Ok then. You want me to submit a separate patch just doing:

``` python
del os.environ['PYTHONDONTWRITEBYTECODE']
```


Oh you could have just added another commit, it would have updated the pull request automatically :)


Also, rebased for a clean history.
Remove Authorization header when following redirection

See http://bugs.python.org/issue3819 for a discussion
on a similar change in urllib2


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2337758) (merged 73ca5943 into c8f166f6).


I need to think about this.


Sure. One other option is to keep the authorization if the host/port etc is the same.
As the bugzilla discussion shows the rfc is ambiguous about it, but it seems that most libs/browsers do the same.


That Python ticket brings back bad memories. Before that fix in Python, urllib2 still sent basic auth after a redirect regardless of whether the URI matched the valid URIs specified in the `HTTPPasswordMgr` instance which was clearly wrong. However, urllib2's API does not map exactly to the requests API in that you don't specify the URIs to send the authentication token.

I think not forwarding the credentials if the sites are not of the "same origin" is a sane default although this could break backwards compatibility. For the strange use case (read: mine) where sending the credentials to some of the hosts in the redirection chain and not others based on other criteria is desired, I think [custom authentication](http://docs.python-requests.org/en/latest/user/advanced/#custom-authentication) is the right approach.


I'm going to reject this for now — but certainly keep it in mind for the future.

Thanks for the contribution!


Ok, no problem.
Might be worth adapting the new unit test to check for the cookie removal on redirect.
NTLM support

python-ntlm has HTTPNtlmAuthHandler for urllib2
http://code.google.com/p/python-ntlm/

shame I have to work with urllib2, for ntlm authentication.

Can someone help in implementing it in requests style ? 
[tried looking at the code, got me confused]

Israel


So, based on a cursory reading of the code it looks like NTLM would be difficult to implement in Requests' style. Part of the problem is that NTLM authenticates whole connections, not individual requests. This means that you can't simply implement a subclass of `requests.AuthBase`.

What you'd need to do is fork the library and adjust the behaviour of the `requests.Session` object to handle authentication. Sadly, I don't know NTLM at all, so I can't really help with how that should go, but implementing some of the stuff in `python-ntlm` can't hurt.


FYI: urllib3 already has an, albeit old-ish, ntlm implementation: https://github.com/shazow/urllib3/tree/master/urllib3/contrib

It might need some cleanup but you should be able to piggyback on it.


Closing for now. I'd like to see this in a separate package though!
Test for iter_content()

Replaces #834 and #823. We can do this ;)


<3


Actually, this appears to be against an old version of the code :(


Want to do a rebase or should i deal with it? :P


I give up :)


@alicebob it would be fairly simple.

```
$ git branch
  develop
  master
  pullrequest
  pullrequest2
* pullrequest3
$ git pull --rebase git://github.com/kennethreitz/requests.git develop
```

If there is a merge conflict at this stage, you have to resolve it and commit the difference. Otherwise, just follow this up with `git push --force origin`.


done, thanks!
permissive implementation of iter_content

This is #781 but it respects chunk_size.

This should also fix the issue from #823?


Beautiful! Just waiting for the tests now :)


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2309298) (merged 77cf9951 into 11758660).


:sparkles: :cake: :sparkles:


Requests v0.14.0 just released, which includes this awesome fix :)
Sfufix

This is for commit https://github.com/SecurityForUs/requests/commit/e35e561a55f4223b4b69312c01677042a854327c

Currently if you pass proxy settings like this:

``` python
proxy = {"https" : ""}
req = requests.get("http://www.google.com", proxies=proxy)
```

The request will be sent through an empty proxy ("").  Every request will return a 404 since it cannot connect to a non-existent proxy.  While I know there should be error checking for this on the developer's side, I thought it would be helpful to pass along a fix to this as well.


This pull request [fails](http://travis-ci.org/kennethreitz/requests/builds/2300350) (merged 784ee71f into 11758660).


Sorry for opening this up twice, I was hoping to clean up the commit list it showed but didn't work as planned.


You may want to fork another branch off of the official develop branch, e.g.,

```
$ git remote add upstream git://github.com/kennethreitz/requests.git
$ git checkout develop
$ git pull upstream develop
$ git checkout -b fix_proxies
$ git cherry-pick sfufix e35e561..784ee71  # or something similar
```

Hope this helps.


Thanks for the awesome patch! Would you mind resending it with the advise given by @sigmavirus24? Thanks!
Develop

This is for commit https://github.com/SecurityForUs/requests/commit/e35e561a55f4223b4b69312c01677042a854327c

Currently if you pass proxy settings like this:

``` python
proxy = {"https" : ""}
req = requests.get("http://www.google.com", proxies=proxy)
```

The request will be sent through an empty proxy ("").  Every request will return a 404 since it cannot connect to a non-existent proxy.  While I know there should be error checking for this on the developer's side, I thought it would be helpful to pass along a fix to this as well.


This pull request [fails](http://travis-ci.org/kennethreitz/requests/builds/2300273) (merged e35e561a into 11758660).


Probably should've named the first var name something else besides "type".  Can push a new commit if requested.
Develop

When prefetch = True (by default in 0.13.9) it causes iter_\* functions (i.e.: iter_content() & iter_lines()) to fail with the raised error stating the content has already been consumed.

I specified it as 'False' for default so such functions can work.  There is not a lot of information on how to fix the issue (with the GitHub links stating the issue is fixed when it really isn't).  For people who don't know the inner workings of Request it can be very discouraging.


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2299035) (merged 4704c771 into 81897dd1).


The adding of "README" is an accident which I apologize for you.  Was not intended to have that pushed as well.


No worries (about README) :)


Unfortunately, this was a major but very intentional change. We're working to make those methods work in either scenario instead of changing back.


Fair enough.  It's also easy to fix it (set prefetch=False) it's just not something that most people think of.  Would it be possible to add a notice in the docs to possibly "prefetch is enabled by default, if you plan on using any iter_\* functions please set it to False"?


Hoping to change this very very very soon :)
Updated content workflow section of the advanced user docs

The default for prefetch changed in 0.13.6 so have updated the relevant section in the docs.  Anything amis?  Not sure if you'd rather alter the default instead although I'd say defaulting to True sounds sensible.


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2286020) (merged 991b853b into 0ad44565).


:cake:
response.raw broken in >= 0.13.6

Reading from raw in 0.13.5 gives me the raw content, however in 0.13.6 and above I get an empty string.

In 0.13.5:

```
>>> requests.get('http://www.google.com').raw.read()
'...'  # the correct content
```

In 0.13.6:

```
>>> requests.get('http://www.google.com').raw.read()
''
```

I presume somewhere in requests raw is being mistakenly read from?


I'm not familiar with the changes made between 0.13.5 and 0.13.6 but for your example to work you must pass `prefetch=False`, e.g.,:

```
>>> from requests import get
>>> r = get('http://httpbin.org/get', prefetch=False)
>>> r
<Response [200]>
>>> r.raw
<requests.packages.urllib3.response.HTTPResponse object at 0x953350>
>>> r.raw.read()
'{\n  "url": "http://httpbin.org/get",\n  "headers": {\n    "Content-Length": "",\n    "Accept-Encoding": "identity, deflate, compress, gzip",\n    "Connection": "keep-alive",\n    "Accept": "*/*",\n    "User-Agent": "python-requests/0.13.8 CPython/2.6.2 Linux/2.6.30.3",\n    "Host": "httpbin.org",\n    "Content-Type": ""\n  },\n  "args": {},\n  "origin": "..."\n}'
>>> r.content
''
>>> s = get('http://httpbin.org/get')        
>>> s.raw
<requests.packages.urllib3.response.HTTPResponse object at 0x953690>
>>> s.raw.read()
''
>>> s.content
'{\n  "url": "http://httpbin.org/get",\n  "headers": {\n    "Content-Length": "",\n    "Accept-Encoding": "identity, deflate, compress, gzip",\n    "Connection": "keep-alive",\n    "Accept": "*/*",\n    "User-Agent": "python-requests/0.13.8 CPython/2.6.2 Linux/2.6.30.3",\n    "Host": "httpbin.org",\n    "Content-Type": ""\n  },\n  "args": {},\n  "origin": "..."\n}'
```

**Edited** to remove IP address.


Thanks, looks like an API change at [3c0b94047c](https://github.com/kennethreitz/requests/commit/3c0b94047c1ccfca4ac4f2fe32afef0ae314094e#L1L461) which is out of line with the docs at [workflow section of the docs](https://github.com/kennethreitz/requests/blob/develop/docs/user/advanced.rst#body-content-workflow).


Glad to help. You should submit a pull request to change those if you have the 
time.
save_as(filename)

So one can do: requests.get("http://foobar.com", prefetch=False).save_as("/tmp/foobar.com.html")
Will add it to the 'advanced docs' if this is accepted.


This pull request [fails](http://travis-ci.org/kennethreitz/requests/builds/2281143) (merged 520e9836 into 0ad44565).


The content in this Pull Request is duplicated by #823. Additionally, the name and description in this PR actually belong to #823 and vice-versa. I'm going to close this one, and centralise all of the discussion in #823.
A warning for iter_content() for prefetch=True requests

iter_content() doesn't work with requests with prefetch=True, which makes sense, but a warning might help.


This pull request [fails](http://travis-ci.org/kennethreitz/requests/builds/2281118) (merged 40c813b9 into 0ad44565).


This PR contains the commits from PRs #824 and #822. I've closed those PRs to centralise the discussion here. The messages for each commit are as follows:
- 4b99498: #822, There was no test for `iter_content()`.
- 520e983: The message above applies to this commit.
- 40c813b: #824, Add `save_as(filename)`, so one can do requests.get('http://example.com/', prefetch=False).save_as('/tmp/example.html'). Will add this to the advanced docs if accepted.


Thanks @Lukasa for centralizing the discussion. I'm personally a little torn about the `save_as` function for the Response object. It could easily become unwieldy. For example, what if someone also wants to save the response headers? Then they have to rewrite this recipe for themselves and the function serves no benefit. Still others might want as much information from the Response object as possible including the url they used (if they're using this towards logging). I think it is certainly useful, I just think it could introduce some possible unnecessary arguments in the future over how much it should include. Besides, developers can easily write a function for themselves to dump the content that they want.

I would lean on the side of simplicity and leave it out.

As for adding tests for `iter_content()`, I'm surprised it didn't already exist. :+1: for catching that.


(Thanks @Lukasa for fixing my messy pullrequest)

I see your reasoning, but because of the prefetch=False gotcha I didn't get it right the first time (I needed to look at the source of iter_content() to figure out how to use it). Having a save_as() which Just Works seems to be nice, if only as an example for anyone to make their own. An other option would be to add it to the documentation as 'recipe'.


I definitely agree having something which "Just Works" which is why I said I was torn on adding the functionality. We have a similar service provided with the `json` attribute. I just can see how providing this will cause some people to want to add more to it and get upset when those desires are shot down. In the end it isn't my decision anyway :-P.


I'm +1 on @alicebob's idea of using `save_as()` as a documented 'recipe'. Actually, I'm +1 on having a section of the documentation devoted to interesting examples or use cases. Not sure it should be part of the base library though.


:-1: on save_as. Users should know how to open a file and write to it. We should document how to do so very easily.


Yeah a recipes section of the docs wouldn't be a terrible idea though. Could probably separate the recipes into beginner (for ideas like this) and advanced for caching or using gevent with requests. 


@alicebob ping me again once you update this to just include the tests, and i'll happily include it :)
When proxy is used, Accept header is overwritten

When a request specifies both an `Accept` header and a proxy server, the `Accept` header value is overwritten with `*/*`. The provided `Accept` header should not be overwritten.

```
proxy = {'http' : 'http://localhost:8888'}
headers = {'Accept' : 'application/json'}
r = requests.get('http://www.yahoo.com', proxies=proxy, headers=headers)
```

 Raw request from proxy:

```
GET / HTTP/ 1.1
Host: www.yahoo.com
Accept-Encoding: identity, deflate, compress, gzip
Accept: */*
User-Agent: python-requests/0.13.6 CPython/2.7.2 Darwin/12.1.0
```


This is probably due to urllib3's logic here: https://github.com/shazow/urllib3/blob/master/urllib3/poolmanager.py#L131

I'm happy to accept a pull request for urllib3 to invert the logic (set default headers and overwrite if provided).


Thanks, just sent the pull request


Hi, this issue is still in latest requests 1.2.0, why was it closed?

Actually it is a little different. The headers are not replaced but duplicated:

```
GET http://www.yahoo.com/ HTTP/1.1
Accept-Encoding: identity
Host: www.yahoo.com
User-Agent: python-requests/1.2.0 CPython/3.3.0 Windows/7
Accept: */*
Accept-Encoding: gzip, deflate, compress
Accept: application/json
```

Which is worse, since output headers are in random order and therefore the behaviour is random.


@oliverjanik I think you're being bitten by something entirely different. For one, I would bet this happens even without the proxy. Secondly, are you setting the headers case-insensitively? If so, try doing `headers={'Accept': 'application/json', ...}`. Third, this was closed because it was fixed [upstream](https://github.com/shazow/urllib3/pull/93) in urllib3. If it is still present then it is likely still an issue with urllib3. Again, I'm fairly certain you're not being affected by this bug.


@sigmavirus24 I am using the same code as original poster: 

``` python
proxy = {'http' : 'http://localhost:8888'}
headers = {'Accept' : 'application/json'}
r = requests.get('http://www.yahoo.com', proxies=proxy, headers=headers)
```

Without proxy:

``` python
headers = {'Accept' : 'application/json'}
r = requests.get('http://www.yahoo.com', headers=headers)
```

Headers (via WireShark):

```
GET / HTTP/1.1
Host: www.yahoo.com
Accept-Encoding: identity
User-Agent: python-requests/1.2.0 CPython/3.3.0 Windows/7
Accept: application/json
Accept-Encoding: gzip, deflate, compress
```

So it's not happening without proxy.


After further investigations:
- Happens with proxy only
- Requests uses b'Accept' as key
- Urllib3 uses 'Accept' as key
- On Python 3 those are different values and are not merged when urllib3 calls update on headers dictionary

It seems my issue is actually #1338.
Data encoding should take place after calling into auth provider

I discovered this bug when using the auth.OAuth1 auth provider. If I did a simple

``` python
oauth = OAuth1(key,secret,signature_type=SIGNATURE_TYPE_BODY)
requests.post(url, data={'some':'param'}, auth=oauth)
```

only the data <code>{'some':'param'}</code> gets transmitted, all the OAuth specific parameters aren't packed into the body.

My patch moves the data encoding after the calling into the auth provider.


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2276322) (merged 52d2e2ee into 0ad44565).


@idan?


just as a short note: the problem is not only related to OAuth , OAuth is just the only auth provider available at the moment that might change the body data and therefore generates this bug.


Thanks!
Correct OAuth1 with query or data.

Sorry, it's me again.

auth.py still have problems. POST with data can't be signed correctly. So i dive into the code and rewrite it.

And in fact, there's no "multipart/form-encoded" but only "multipart/form-data".


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2257949) (merged 8a1d53eb into b9ecf456).


@idan @dgouldin @gulopine code review? :)


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2258667) (merged 46ff4b38 into b9ecf456).


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2258707) (merged aded42f2 into b9ecf456).


Looks good! Needs tests and then ready to merge.


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2258817) (merged babac736 into b9ecf456).


Awesome, thanks!
import cchardet if available


I'll consider this in the future, but chardet doesn't seem to be causing any problems for anyone. :)


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2242765) (merged fc9b8a02 into 1be2a55a).


OK
But chardet has some problem.

Example: http://m-p.sakura.ne.jp/Html/anime.html

``` python
import requests, cchardet, chardet

r = requests.get("http://m-p.sakura.ne.jp/Html/anime.html")
chardet.detect(r.content) # ISO-8859-2
cchardet.detect(r.content) # SHIFT-JIS <- Correct!
```

Of course, this is a problem of chardet...
Thanks


Hmm, I can include this I suppose, but I'll keep it undocumented.
Parsing of "Allow" Headers

As per the RFC (RFC2616), the "OPTIONS" method is used to identify communication options on the request/response chain.

Within a RESTful System, the OPTIONS method can be used to display which methods are allowed for the authenticated user.

Typically they are returned as comma-seperated values within the 'Allow' header.

e.g Allow: GET, POST, PUT

These non-cacheable headers allow for a client to discover the methods available against a resource.

I am proposing that the allow headers are parsed if available, and presented within a pragmatic and pythonic way.

Here are my thoughts so far (note that the github API does not support options for any of the methods I tried, this is all theoretical):

> > > a = requests.options('https://github.com')
> > > a.allow('get')
> > > True
> > > a.allow('post')
> > > False

Some key things to think of:
- Some API will not return allow headers when options is called.
- Should this parsing be available only for the options call. Within a RESTful architecture no other method should return the "Allow" header.
- All comparisons should be completed on lowercase strings.

Can I get your thoughts around introducing this header parsing for the OPTIONS method?


If parsing of Allow headers was in any way complicated, I'd be all for this feature. However, Allow headers have got a pretty simple form, so they aren't tough for the end user to parse if they need to. This seems like it would be an addition to the API for no particular gain in simplicity or function.
Remove exception eating from dispatch_hook.


Resolves #782.


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2235143) (merged 57b4d397 into 94869e72).


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2235147) (merged 57b4d397 into 94869e72).


:cake:
Certificate verification errors not end-user friendly

We are currently using requests in a rich client application (still in development). It is indeed a great improvement over urllib2 and friends! Especially it is great that it does verify the server certificate when using https.

The error reporting though is quite lacking - the API reports a single SSLError for any verification failure:

```
SSLError: [Errno 1] _ssl.c:504: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
```

In this case the host name did not match the name given in the certificate but it is not possible to infer that from the Exception. When accessing the same site from Firefox I get

```
<host.example.com> uses an invalid security certificate.

The certificate is not trusted because it is self-signed.
The certificate is only valid for <otherhost.example.com>

(Error code: sec_error_untrusted_issuer)
```

I think requests should report similar information in the result exception for SSLError.

I know this is asking for much since the underlying SSL modules do not do this yet.


I've considered doing this in the past, but due to the complexities of SSL errors (and the fairly hands-off support we have of them), I decided to keep it as transparent as possible. 

This might change in the future though :)
Timing of Request

I am not sure if this is out of scope of requests, but I was wondering about the implementation of some sort of request time metric, as in the time it takes from the start to completion of the request.

For example:

> > > resp = requests.get('https://github.com')
> > > resp.time #(or something similar)
> > > 124ms # (Represented as ms/seconds etc, but actually returned as milliseconds within a time object or similar)

I am more then happy to get started on this, I would just like a discussion about the relevance of this feature to the context and purpose of requests as a package.


I considered adding this early on, actually, but decided to keep the scope of the library as concise as possible. It's pretty easy to do this now with the existing hooks system, which seems to be what people are doing now.

---

Keep this issues coming though, these are great :)


Thanks for the response :)

On 26/08/2012, at 12:31 AM, Kenneth Reitz notifications@github.com wrote:

I considered adding this early on, actually, but decided to keep the scope
of the library as concise as possible. It's pretty easy to do this now with

## the existing hooks system, which seems to be what people are doing now.

Keep this issues coming though, these are great :)

—
Reply to this email directly or view it on
GitHubhttps://github.com/kennethreitz/requests/issues/809#issuecomment-8023784.
updateStatusWithMedia not working

i think this could be related to the other issue "Post not working".

i get an error:

twython.twython.TwythonError: u'Unauthorized: Authentication credentials were missing or incorrect. -- Could not authenticate you.'

for a workaround, until this get fixes,  i had to generate the headers to work. here is an example workaround:

from twython import Twython

twitter = Twython(
    twitter_token = 'XXXXX',
    twitter_secret = 'XXXX',
    oauth_token = 'XXXX',
    oauth_token_secret = 'XXXX',
    headers = { "Authorization": 'OAuth oauth_consumer_key="XXXXX", oauth_nonce="XXXXX", oauth_signature="XXXX", oauth_signature_method="HMAC-SHA1", oauth_timestamp="1345442", oauth_token="XXXXXXX", oauth_version="1.0"' }
)

twitter.updateStatusWithMedia('sample.jpg', status='hello!')


This should be better now :)


still does not work for me ...   

``` python
  response = twitterImage.updateStatusWithMedia("C:\\Users\\Desktop\\python-twitter-0.8.2\\examples\\data\\1.png", status='thats me :)')
  File "build\bdist.win32\egg\twython\twython.py", line 445, in updateStatusWithMedia
  File "build\bdist.win32\egg\twython\twython.py", line 448, in _media_update
  File "build\bdist.win32\egg\twython\twython.py", line 233, in post
  File "build\bdist.win32\egg\twython\twython.py", line 225, in request
  File "build\bdist.win32\egg\twython\twython.py", line 206, in _request
ython.twython.TwythonError: u'Unauthorized: Authentication credentials were missing or incorrect. -- Could not authenticate you.'
```

normal status update works great!


I'm getting the same error - does anyone have this working with the latest version of Twython?  

@y2rayk how do you get the oauth_signature param for the headers in your workaround?
General Housekeeping

os was imported twice.

Not much of a change. Tests worked A-Ok


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2209649) (merged 217a9d50 into 4889adce).


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2209653) (merged 217a9d50 into 4889adce).


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2209686) (merged 29aa21a2 into 4889adce).


Thanks!
Remove None values from post data.

This pull request resolves the discussion in #378 by preventing None values from being posted.


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2206075) (merged 751e7a0a into 4889adce).


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2206079) (merged 751e7a0a into 4889adce).


Awesome, thanks!
Session params ommited in 0.13.8

Whenn subclassing the Session class like this:

class TubeQuery(Session):

```
def __init__(self):
    super(TubeQuery,self).__init__(params={'v':'2','key':devkey,"alt":"json"})
    self.server="http://gdata.youtube.com"
    self.key=devkey
```

and then using the session in a "with"-statement, the params are not passed anymore, since 0.13.8. Is this supposed to be like that?

with self as req:
    reg.get(self.server+"foobar")


This seems to be broken by [1722f289](https://github.com/kennethreitz/requests/commit/1722f289711e3d03b0e00fb548f3e8ea92544775).  Since params and proxies are no longer dictionaries and are lists of tuples instead, they no longer have an 'items' function, which is a check in the session#merge_kwargs() function.  Therefore instead of merging the params passed in with the request and the session, it only uses the local params.

Not sure of an ideal solution for this, doing a check for the items() function or type list might not be strict enough as the elements in the list are significant.

``` python
    # Bypass if not a dictionary (e.g. timeout)
    if not hasattr(default_kwarg, 'items'):
        return local_kwarg
```


That's one of my commits so I'll dive into this and see if I can fix it ASAP.


I just gave this a shot, not sure if this is the best way to go about it but it seems to work:  [f18733d2be](https://github.com/weak/requests/commit/f18733d2be4a9c3d0d3ceea31be9e191c05f21d6).


I'm trying to do it the opposite way by finishing what I had started (e.g. coercing everything to a list of key-value tuples).


Sounds good, thanks for taking a look :)


Thank you for narrowing it down. But I'm going to say that yours looks like the least mess for two reasons:
1. My work so far trips over boolean values, e.g., `local_kwarg = to_key_val_list(local_kwarg)` where `isinstance(local_kwarg, bool) is True`.
2. If the defaults have something like `{'page': '1'}` and local has `{'page': '2'}`, we would have to do a lot more work to overwrite the default. I'm going to think about this more to see if I'm missing an obvious way of doing this in a quick way so that ordering isn't lost.


Like I mention in my commit message, 5025bd5 has the potential (because of nested for-loops) to take a while on large dictionaries/(key, val) lists. It does work though and can be tested with this [gist](https://gist.github.com/3426624)

Should have run the tests, this breaks hooks (at least) and probably also headers. I guess I may as well put the work into finally getting those to coerce to key, val lists.


And fixing this for hooks isn't viable since requests relies on the setdefault ability for hooks, which is why I ruled them out on #179.

I'm going to keep working on this to see if there's anyway to do this with the (key, val) lists.


Fixed by @weak, thanks!


@sigmavirus24 feel free to send your final version when it's ready :)


In switching everything functionally back to dicts (as OrderedDicts) this will be fixed. Just waiting on a verification in #817. Sorry for the delay.


No one seems to have any objections, so I'm going to go ahead and implement it like this.
Custom mimetypes for files in multipart request

I added support for custom mimetypes in mutipart request.

Usage:
r = requests.post(url, files={'file': ('file.xml', file_content)})
or
r = requests.post(url, files={'file': ('file.xml', file_content, 'text/xml')})
or
r = requests.post(url, files={'file': ('file.xml', file_content, 'application/xml')})


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2189646) (merged 2f9b4b41 into 4889adce).


Part of this should actually be a PR on shazow/urllib3 and the other part would depend on @shazow accepting it unless I'm speaking out of turn.


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2189650) (merged 2f9b4b41 into 4889adce).


+1 for pull request on urllib3.


We should indeed have this discussion in urllib3 land, though I'm wondering if having no-tuple, 2-tuple, 3-tuple is going too far. Perhaps we need a better interface at this point. :)


Same here, I think it's very unintuitive.


@vasa81 thanks for the contribution, but we're going to hold off for now until we figure out a better way to approach this.
Tiny formatting changes in Advanced docs

Just noticed when checking out the documentation for the new [link headers](http://docs.python-requests.org/en/latest/user/advanced/#link-headers) feature (quite the productive sprint you're having in Hobart! Very cool.) that there were quite a few paragraphs ending in ".:", which was ugly. Removed those, and generally made changes with colons.


Excellent, thanks :)


This pull request [fails](http://travis-ci.org/kennethreitz/requests/builds/2178783) (merged b5ce5029 into c323047d).


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2178787) (merged b5ce5029 into c323047d).
Added in a link handler as per issue #711

Basic Basic handling of links. So far has very basic error handling, and might not cater for some cases. Handles the syntax of link headers that are demonstrated below.

> > > from requests.utils import parse_header_links
> > > parse_header_links('http:/.../front.jpeg; rel=front; type="image/jpeg",http://.../back.jpeg; rel=back;type="image/jpeg"')
> > > [{'url': 'http:/.../front.jpeg', 'type': 'image/jpeg', 'rel': 'front'}, {'url': 'http://.../back.jpeg', 'type': 'image/jpeg', 'rel': 'back'}]
> > > parse_header_links('http:/.../front.jpeg;')
> > > [{'url': 'http:/.../front.jpeg'}]
> > > parse_header_links('http:/.../front.jpeg')
> > > [{'url': 'http:/.../front.jpeg'}]
> > > parse_header_links('http:/.../front.jpeg; a=t')
> > > [{'url': 'http:/.../front.jpeg', 'a': 't'}]


:sparkles: :cake: :sparkles:


This pull request [fails](http://travis-ci.org/kennethreitz/requests/builds/2178021) (merged f3159ed4 into 3514816d).
Fix kennethreitz/requests#790

Fix kennethreitz/requests#790


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2174801) (merged 301d1a83 into 2d5d6f99).


This pull request [passes](http://travis-ci.org/kennethreitz/requests/builds/2174805) (merged 301d1a83 into 2d5d6f99).


Anyone can handle this?
What the bot said is wrong. It is not merged. The bot gives a wrong commit.
cc @Lukasa @kennethreitz


Correct. If you want Travis to re-test your commit, you should rebase onto the newest commit from `kennethreitz/requests`. (Add `kennethreitz/requests` as a git remote then `git pull --rebase` from it.)

For what it's worth, PEP8 recommends that you should use Python's [implicit line continuation](http://www.python.org/dev/peps/pep-0008/#maximum-line-length) instead of the backslash.

EDIT: So it's clear, only Kenneth merges pull requests. Until he gets time to look at this, nothing more will happen on it. =)


@Lukasa Thank you. Either form is accepted. If the issue can be fixed, i don't care what and how is modified.


@ayanamist hmm, this seems like an odd way to fix the problem. I need to get a better understanding of why this is necessary. Any additional info would be great!


Merging with a few changes, thanks!
Support SSL client certificates

It would be handy if we could support HTTP transactions where the server required client-side SSL authentication.

An ugly urllib2/httplib implementation is here: http://stackoverflow.com/a/5899320

I imagine a sample interaction would look something like this:

```
response = requests.get(url, client_cert="/path/to/cert", client_key="/path/to/key")
```

or

```
response = requests.get(url, pem="/path/to/pem")
```


These are fully supported.


They just happen to be fully undocumented :)


Oops, I take that back:

http://docs.python-requests.org/en/latest/user/advanced/#ssl-cert-verification