Raibek/requests-github-issues huggingface dataset:
Made default_user_agent reusable

This is not exciting change, but will help for people who wants to change name of user agent alone. Also name can be unicode.


I'm strongly +0 on this change. I can see how it's useful, but it's hidden away in an undocumented library function that we never planned to expose to the world, so I'm quite happy to let this change go unmerged as well. It's not hard to construct a User-Agent on your own. =)

More importantly, the code isn't right. This:

``` python
return " ".join([u'%s/%s' % (name, __version__),
                 u'%s/%s' % (_implementation, _implementation_version),
                 u'%s/%s' % (p_system, p_release)])
```

has multiple bugs in it. Firstly, if `name` is a bytestring on Python 3 then this will cause a bug. If it's a bytestring in Python 2 encoded in anything other than ASCII, this will also cause a bug.

If we insist on doing this, then `utils.to_native_str()` is the function to use here, and you must desperately avoid passing bytestrings into unicode strings. This set of problems makes me think that this function is far more complex than it's worth.


You didn't need to close this, it's possible that Kenneth or Ian will want it. =) +0 represents neutrality, not opposition.


@Lukasa I am wondering why you didn't use `0`. 


I'm surprised that you haven't seen +0 before @kracekumar. We use it excessively to vote on topics and it is used in lots of other projects.

+1 := strongly in favor
+0 := neutral but leaning positively toward the change
-0 := neutral but not exactly a fan of the change
-1 := very much against the change.

I'll respond to the PR itself later today


@sigmavirus24 I have seen `+0` in mailing lists and other places. Sure. I am glad to fix the bugs pointed out by @Lukasa if needed. Waiting for the comments to improve. 


I'm not really sure. It's a small change but we would then have to expose it to the user and that's what I'm not exactly a fan of. Certainly they can find it on their own now but that's not the same as exposing it to them which would be encouraging its use. How commonly do requests users set their own User-Agent string and want it to include all of that information? Frankly I'm not convinced it is all that frequently. That said if @kennethreitz wants this I'm perfectly okay with it granted that it handles strings correctly (as @Lukasa already mentioned).


Seems harmless enough!


Let's just not document it.
Browser compatible redirects don't work for APIs

In the redirection code there are a couple of special rules for handling redirections: 
kennethreitz/requests@4bceb312f1b99d36a25f2985b5606e98b6f0d8cd/requests/sessions.py#L127 shows handling of rfc2616 and some browser specific handling for https://en.wikipedia.org/wiki/Post/Redirect/Get 

I understand why that is the default, but for writing code that talks to APIs this doesn't work. 

Could we introduce a flag or something that we can set to be 'api_compatible' and do more predictable redirect handling?


If you want to override Requests' redirect logic, the way to do it is to set `allow_redirects` to false.

It's also unclear to me why APIs would ignore that set of behaviours. Writing an API that assumes that a 301/302 will keep the verb the same on redirect is willfully blind, especially when two other status codes exist that do not have this ambiguity. If an API writer really wants to ensure that the same verb is used after a redirect, they should be using 307.
Ensure sockets are properly closed on object destruction

Fix issue @tardyp reported on #239

We have not been able to reproduce on a simple testcase. We can only make it happen on our complex environment; involving buildbot, twisted and txrequests.

This fix no longer exhibits the issue. Please, give us a feedback whether this solutions is correct.


It's certainly possible that the fixes are correct, but all your changes are in urllib3. This is actually a third party library that is maintained separately from Requests, and we simply include a direct copy of their code. Can I suggest you raise the issue at [shazow/urllib3](https://github.com/shazow/urllib3)?
Make morsel_to_cookie more pythonic

Fix a pep8 error. Prior to fixing this, I implemented some tests to cover the code I was fixing. This should help prevent regression issues.


@sigmavirus24 - Pushed the changes we discussed. Let me know if you have anything else!

Thanks!


With the exception that the diff is a bit noisy due to @mdbecker alphabetizing portions (imports and one functions params), this is good to merge in my opinion.

For those who don't care to read :+1: :shipit: 


Breaks the builds!

http://ci.kennethreitz.org/job/requests-pr/123/PYTHON=2.7/console


Thanks @kennethreitz. Looks like there is a bug here (either in my test or in the code.) The issue is that `time.mktime` returns a time in localtime which I wasn't accounting for in my test. There are 2 ways to solve this problem:
1. Modify `morsel_to_cookie` to subtract `time.timezone` so that the resulting value represents UTC time. This will result in the value of `expires` being the same on all systems regardless of timezone.
2. Modify `TestMorselToCookieExpires.test_expires_valid_str` to subtract `time.timezone` so that the test always passes.

The former solution seems correct to me, and I can't find anything in the code or RFCs to contradict this. Unfortunately this change would definitely impact users of this module so I'd like to hear others opinions on this.

Thanks!


I've pushed a new copy of the branch with the first solution mentioned above. This should fix the build.


Nifty CI system! Let me know if you want any more changes!


@mdbecker the PR currently doesn't merge cleanly, could you rebase off of master?


But, the tests do pass! :D


Sure, let me rebase!


All set!


:metal:


Took one more quick look over it and I'm once again :+1: :shipit: 


Ah, before we do this, can we try to keep the tests in `py.test` style? This means removing all those calls to `self.assert*` and replacing them with plain `assert` statements. =)


@Lukasa re-pushed


:sparkles: :cake: :sparkles:


Awesome, this is great work! Thanks @mdbecker! :cake:
Implemetation of IP address ranges for no_proxy environment variable

Hello,

During OpenStack implementation we found out that it is not possible to use IP ranges in no_proxy environment variable. We needed to exclude whole range of IP addresses (172.28.1.0/24), not only domain names or particular IP addresses.

Now it is possible to specify no_proxy like

```
no_proxy=127.0.0.1,192.168.0.0/24
```

and request to http://192.168.0.20:5000/ won't go through http proxy.

I added tests for no_proxy function as well.

Kamil


Hi Kamil, thanks for this work! Unfortunately, there are a number of problems that would need to be addressed before we could merge this pull request.
1. You've checked in an Intellij IDEA file, `.idea/dictionaries/kmadac.xml`. This needs to be removed.
2. `netaddr` is a third-party module that isn't part of the standard library. Requests tries very hard not to have external dependencies other than those we vendor in, and vendoring in `netaddr` for this task is excessive. You'll need an implementation that doesn't rely on it.

If you can resolve these, I see no reason for us not to merge this.


I second everything @Lukasa said. If you can, please rebase out everything except f9a48e0


Hi Guys,

All objections should be fixed now. There is no dependency on non-stdlib modules, tests are added and unnecessary files are removed too. 


I'm :+1:


I'm so sorry, I have further feedback. =(


@Lukasa don't apologize. Those were good catches that I totally missed because I skimmed over them.

I'm :+1: when @Lukasa's comments are addressed.


@Lukasa what feedback do you have? Is it still standing?


All satisfied, I think I'm happy now. =)
Requests failing over integer in cookie

Hi,

I described it here:
http://stackoverflow.com/questions/20325571/python-requests-failing-over-integer-in-cookie

But basically this creates an error:

```
import requests
r = requests.get('http://www.c1000.nl/kies-uw-winkel.aspx')
...   
File "/srv/www/li/venv/local/lib/python2.7/site-packages/requests/cookies.py", line 270, in set_cookie
if cookie.value.startswith('"') and cookie.value.endswith('"'):
AttributeError: 'int' object has no attribute 'startswith'
```

Is this a known issue? Is there any workaround? (i.e. do not parse cookies?)


Hi there!

First, the answer. Yes, this is a known issue, and a fix is already present in master.

Now, some notes. Asking this question on Stack Overflow was exactly the right thing to do. When you do that, please wait more than 30 minutes before moving to this issue tracker. Both I and @sigmavirus24 regularly look at the Python-Requests tag on Stack Overflow, so you'd have got our attention before long. Asking the question both there and here is unnecessary duplication.

Secondly, the question 'is this a known issue' is one you should be able to answer just as well as us. It is a known issue: issue #1745 tracked it. A [very simple GitHub search](https://github.com/kennethreitz/requests/search?o=desc&q=cookie+AttributeError&s=created&type=Issues) could have shown you this issue quite quickly. I appreciate that it's not always obvious, but if everyone took five minutes to browse some old related issues I'd save hours of my life over the long term.


Hi,

First of all: sorry, as I of course do not want to waste your time and also for double posting both on SO and here. 
I did search for it on both SO and on github actually, but I searched for "cookie" and "integer" which didn't come up with any hits. So I hope you understand that I wasn't consciously/lazily wasting your time at the very least...


I do. =) Just wanted to make sure. Thanks for taking the effort! :cake:
HTTP Basic Auth credentials extracted from URL are %-encoded

I was relying on credential auto-extraction from the hostname [1] to perform HTTP Basic Auth but I was getting "401 Unauthorized": the spaces in the password were substituted by `%20`. Manually extracting them with `urlsplit` and passing them to the `auth` parameter as a tuple works.

[1] http://docs.python-requests.org/en/latest/user/authentication/#netrc-authentication (second paragraph)


Hmm, that behaviour feels wrong to me. Probably should fix this. =) Thanks! :cake:


Thanks for reporting this @lerks
Quote qop values in digest auth.

Resolves #1765.


#1765 was already a PR, why did you open this one? We could have added tests after the fact. Can you rebase this to work off of #1765 so that @bicycle1885 gets some credit?


Uh, yeah, that's because I'm an idiot and didn't spot it. Like a moron.


> Uh, yeah, that's because I'm an idiot and didn't spot it. Like a moron.

No need to be so hard on yourself.


Also, :+1: will label with "Minion Seal of Approval"
Prepare new release (2.1.0)?

There are a few nasty bugs that we've fixed since the last release, including the broken SNI support. It'd be good to get another release out. I think a few things have changed dramatically enough that this is probably a minor release (e.g. takes us to 2.1.0). Thoughts? /cc @kennethreitz @sigmavirus24

Still to do:
- [ ] Merge outstanding PRs (definitely ~~#1713, #1657, #1768, and #1766~~; maybe #1729, #1628 and #1743 depending on how @kennethreitz feels).
- [ ] Update changelog to match those PRs.

If nothing else Runscope could do with this release, so it'd be nice to be in good shape.


2.1.0 is a little overdue so I'm :+1:. I'll review the PRs you listed and provide feedback again just to be safe (in case I missed any of them).


I would say "definitely" #1766. That shouldn't be a "maybe"


Looking over the PRs listed above now:
- #1713 :+1: merge it
- #1657 :+1: merge it
- #1729 :-1: do not merge it (I don't fee it is ready yet. @Lukasa and I left feedback that hasn't been addressed)
- #1628 :-1: do not merge it (neither @kennethreitz nor I are very fond of this)
- #1766 :+1: merge it
- #1743 +0 I don't see anything awful about allowing for separate timeouts, it is just the API under question. I've proposed a different way of handling the same feature. I'm not sure this _has_ to be in 2.1 though 


Updated to inclue #1768 which is a :+1: merge it.


I don't want to be pushy, but I'm very excited for 2.1 :smile: The pending 2.x SNI stuff is huge for us. If I can help with the release at all, let me know!


@kennethreitz this PR wasn't finished. There's still stuff missing from the Changelog


@sigmavirus24 d'oh, sorry guys. I got ahead of myself :) Hoping to do this release tonight.
Requests to a domain name defined in /etc/hosts fails.

```
  ~/workspace/venv/local/lib/python2.7/site-packages/requests/api.py line 55 in get
      return request('get', url, **kwargs)
  ~/workspace/venv/local/lib/python2.7/site-packages/requests/api.py line 44 in request
      return session.request(method=method, url=url, **kwargs)
  ~/workspace/venv/local/lib/python2.7/site-packages/requests/sessions.py line 335 in request
      resp = self.send(prep, **send_kwargs)
  ~/workspace/venv/local/lib/python2.7/site-packages/requests/sessions.py line 438 in send
      r = adapter.send(request, **kwargs)
  ~/workspace/venv/local/lib/python2.7/site-packages/requests/adapters.py line 327 in send
      raise ConnectionError(e)
   ConnectionError: HTTPConnectionPool(host='myserver.dev', port=80): Max retries exceeded with url: /accounts/ (Caused by <class 'socket.gaierror'>: [Errno -2] Name or service not known)
```

`myserver.dev` is defined in `/etc/hosts`

**Edit:** Rewrite bug request.


Why is this a requests bug?


Ok let me explain it differently. I had the problem before and the fix was to use gevent master. Since gevent has been released, the bug came back.


Your references were useful, so I'm going to reproduce them here: surfly/gevent#271 and surfly/gevent#21.

Both of those refer to bugs in gevent. The bug you're seeing now is that when you use requests with gevent, those bugs appear.

This is not a bug in requests. If there is a bug in an underlying library, it should be reported and fixed there, not here.


Ok
binary file upload fails with python 2.7 under windows

Following  three lines cause a decoding error under windows.
somehow something seems to attempt decoding the binary file contents.

Same code works fine on Linux ( with a real url of course ;-) )

```
#!/usr/bin/env python
import requests
files = { 'file' : ( 'raw.dat', '\xff' ) }
requests.post('http://example.dies.before.url.is.used', files=files)
```

```
 c:\tmp>python reqbug.py
 Traceback (most recent call last):
  File "reqbug.py", line 4, in <module>
    requests.post(my_url, files=files)
  File "C:\Python27\lib\site-packages\requests\api.py", line 88, in post
    return request('post', url, data=data, **kwargs)
  File "C:\Python27\lib\site-packages\requests\api.py", line 44, in request
    return session.request(method=method, url=url, **kwargs)
  File "C:\Python27\lib\site-packages\requests\sessions.py", line 361, in request
    resp = self.send(prep, **send_kwargs)
  File "C:\Python27\lib\site-packages\requests\sessions.py", line 464, in send
    r = adapter.send(request, **kwargs)
  File "C:\Python27\lib\site-packages\requests\adapters.py", line 321, in send
    timeout=timeout
  File "C:\Python27\lib\site-packages\requests\packages\urllib3\connectionpool.py", line 471, in urlopen
    body=body, headers=headers)
  File "C:\Python27\lib\site-packages\requests\packages\urllib3\connectionpool.py", line 285, in _make_request
    conn.request(method, url, **httplib_request_kw)
  File "C:\Python27\lib\httplib.py", line 946, in request
    self._send_request(method, url, body, headers)
  File "C:\Python27\lib\httplib.py", line 987, in _send_request
    self.endheaders(body)
  File "C:\Python27\lib\httplib.py", line 940, in endheaders
    self._send_output(message_body)
  File "C:\Python27\lib\httplib.py", line 801, in _send_output
    msg += message_body
UnicodeDecodeError: 'ascii' codec can't decode byte 0xff in position 103: ordinal not in range(128)
```


Not for me it doesn't: Windows 7, Python 2.7.4, Requests 2.0.1. Can you provide the versions of all of your software as well?


By the way, the overwhelming cause of this is having a unicode string as either the method or in the headers. Make sure that you don't do either of those things. =)


d:\work\mhcomm\mg_app>python
Python 2.7 (r27:82525, Jul  4 2010, 09:01:59) [MSC v.1500 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.

d:\work\mhcomm\mg_app>python -c "import requests ; print requests.**version**"
2.0.1

Tried on another windows machine with 2.7.4 and it seems to work.
Will update that machine and report back.
Didn't have any other issues with this machine so far.


So it looks like your version of Python is massively out of date. For comparison, I'm running `Python 2.7.4 (default, Apr  6 2013, 19:55:15) [MSC v.1500 64 bit (AMD64)] on win32`. Try running in a more recent version of Python 2.7 than that.


Thanks all. 
Problem disappeared with updating 2.7 to 2.7.4
I'm now a happy requests user :-) again even on this machine


Glad to hear it!
Fix hangs on streaming uploads with HTTPDigestAuth

When using Digest Authentication, the client resends the same request
after the server responds with the 401 "Unauthorized". However, when
doing streaming uploads, it gets stuck because the body data (a
file-like object) is already consumed at the initial request.

The patch fixes this by rewinding the file-like object before
resending the request.


This fixes #1755


LGTM. Thanks for this! :cake:
builtin_str in python2

builtin_str should be defined as unicode for py2 version OR in models.py (line 107) don't re-encode data.
When passing files & datas, datas are réencoded from unicode to str, and raise an UnicodeEncodeError.


`builtin_str` means literally natural `str` type of python. So, there is no reason that `builtin_str` should be defined as `unicode` in python2.
Can you explain in detail about how you passed data to `requests`?


It's not clear to me what your problem is here. You've provided fixes, but not indicated what the problem is. Let me address your fixes.

Firstly, `builtin_str` should not be `unicode` in Python 2. It's purpose is to be a shorthand for 'whatever kind of string is used by default'. To be clear, `isinstance("string literal", builtin_str)` should evaluate to `True` on every platform. In Python 2, that means `bytes`.

Secondly, we don't re-encode data. If your data value is a bytestring, it does not get re-encoded. If you care about what encoding we use, encode your data before passing it to us.


Sorry, I close this issue, It came from a weird installation of the module.
Thanks for your answers.
partial download using range header not working

This is supposed to download a partial chunk of the URL (bytes 1000 to 2000)

```
target = 'http://10.0.0.10/words.txt'
headers = {'Range':'bytes=1000-2000'}
r = requests.get (target, headers=headers)
print r.text
```

It's not working and I get the whole file.

It does however work with the standard library's urllib2. I believe the problem is in the way requests is sending the request, since the server's logs show a difference in the response code.

The full code and some server configuration information here (I've tried the commented out things). I've also shown the code I use for urllib2.

http://bpaste.net/show/dWrjTbxBZKRxjSGZEma7/

I am using 2.0.1 of requests.

```
>>> import requests
>>> requests.__version__
'2.0.1'
```


I suspect the server is at fault here.

``` python
>>> import requests
>>> url = 'http://mirrors.manchester.m247.com/ubuntu-releases//precise/ubuntu-12.04.3-desktop-amd64.iso'
>>> r = requests.get(url, headers={'Range': 'bytes=1000-2000'}, stream=True)
>>> r.status_code
206
```

If you believe the difference is in the request being sent, you should use a tool like tcpdump or wireshark to compare the actual requests on the wire.


Here is tcpdump for requests: http://bpaste.net/show/152701/

Here is tcpdump for urllib2: http://bpaste.net/show/152703/

Command lines and code are included.

The full headers being sent are indeed different. As you say it could be a combination of that and the server, or a server setting: however it _does_ work with the standard library's urllib2 and one would expect requests to emulate this behaviour?

I could probably get it working from seeing the difference in the headers I guess. Not really sure what the extra repeated bits at the end of the requests are. I'd have to read up on the protocol.


Update: Looking at tcpdump's output, I determined that adding "Accept-Encoding:Identity" to the headers send by requests made the code work for this server (and a few other random ones on the net such as BBC News, for which it was not working before). The test server does have compression (Apache mod_deflate) installed. Something somewhere is not following some standard I guess, and I don't know what it is (yet).


Ugh, that server is being stupid. From the [relevant spec](http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html):

> The "identity" content-coding is always acceptable, unless specifically refused because the Accept-Encoding field includes "identity;q=0", or because the field includes "*;q=0" and does not explicitly include the "identity" content-coding. If the Accept-Encoding field-value is empty, then only the "identity" encoding is acceptable.

If Accept-Encoding is not present, it's still acceptable, because we haven't specifically excluded it. I'm still blaming the server. =)


I'm with @Lukasa 

If there's a header we send by default which causes this, you can override it with `headers={'Header-Name': None}` to get rid of it before we send the request.

I'm in favor of closing this as it is not a real bug.
import error if ndg-httpsclient and pyasn1 are installed

Supposedly these dependencies are needed for SNI support in python2.

However requests doesn't even import when these dependencies are installed.

virtualenv-2.7 testX; . testX/bin/activate
pip install ipython requests ndg-httpsclient pyasn1

```
In [1]: import requests
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-1-686486c241c8> in <module>()
----> 1 import requests

/Users/dima/testr/testX/lib/python2.7/site-packages/requests/__init__.py in <module>()
     51 # Attempt to enable urllib3's SNI support, if possible
     52 try:
---> 53     from .packages.urllib3.contrib import pyopenssl
     54     pyopenssl.inject_into_urllib3()
     55 except ImportError:

/Users/dima/testr/testX/lib/python2.7/site-packages/requests/packages/urllib3/contrib/pyopenssl.py in <module>()
     53
     54 orig_util_HAS_SNI = util.HAS_SNI
---> 55 orig_connectionpool_ssl_wrap_socket = connectionpool.ssl_wrap_socket
     56
     57

AttributeError: 'module' object has no attribute 'ssl_wrap_socket'
```


Please check the open issues before you raise one. This is being tracked in #1732.


thanks for catching this. glad to see it already got some attention!
Streaming uploads with HTTPBasicAuth causes stuck on resending the request

`HTTPBasicAuth` seems to have the same issue described in requests/requests-kerberos#23.


Resolved by #1760


@akitada please don't close your own issues. We'll close them when we are ready to.


Oh, sorry! I'm not familiar with the process.


No need to apologize @akitada 


Fixed in 2.1.0.
Fails to make some https connections with PyPy 2.2

This is effecting Pip pretty aggressively. 

Also being tracked by PyPy: https://bugs.pypy.org/issue1645

/cc @Lukasa @sigmavirus24 


Are both communities seeing the same bug? Because it looks like Alex fixed it [here](https://bitbucket.org/pypy/pypy/commits/963c6d6d7d6c8cb32d1e338ef620e9edd9e479fa). He's certainly marked it as fixed in the PyPy bug tracker.
sessions.request not processing cookies properly

we do a post, passing the cookies from the previous response as the cookies:

requests.post(...,cookies=resp.cookies...)

the request function of the Session class appears to be presuming the cookies are always a dict, even though the comments indicate it could also be a cookiejar.

self.cookies = cookiejar_from_dict(cookies, cookiejar=self.cookies, overwrite=False)

the only way we can get it to work is if we use:

requests.post(...,cookies=resp.cookies.get_dict...)

Thanks


This is a duplicate of #1711.


Thanks for reporting it!


Excellent. Can I get an updated version from git? It doesn’t appear to be in master. I’m seeing other cookie problems, and I’d like to try the latest code.

Thanks!

Glenn

On Nov 21, 2013, at 5:34 PM, Ian Cordasco notifications@github.com wrote:

> Thanks for reporting it!
> 
> —
> Reply to this email directly or view it on GitHub.


@glennbach Linked from that issue is the relevant pull request, which is #1713. It hasn't yet been accepted, but feel free to try it.
The method resolve_redirects should also prepare_auth again so OAuth1 would work

Whenever a request made with an OAuth1 auth object results in a redirect to a new location, it would then result in the Authorization header becoming invalid (wrong signature) thus making the function provided by `allow_redirect` ultimately useless in the context of an OAuth1 session. A potential fix is to call  `prepare_auth` again after the `prepare_cookies` in the `resolve_redirects` loop, but I have no idea what potential side effects are and how to begin writing a test in that gigantic method within a loop so no pull requests from me, for now.


Also this is reported under requests/requests-oauthlib#94.


So, the initial evaluation is that `prepare_auth` can't be placed there. We throw the authentication object away, we aren't carrying it around. The question is: do we think we should re-evaluate authentication for each new request? If we do, how should we do it?

One option is to extend the Requests authentication handlers to allow them to be placed on `Session`s. How's that for an idea?


We now strip all authentication from redirects, so this issue is now invalid. =)
Make charade library optional

The charade library in the packages directory is quite big (500kByte)

It is only needed if you give a response not encoding. I think this is an error in the code which uses the request library.

Zen of Python "In the face of ambiguity, refuse the temptation to guess."

I think the library should be optional, and not in this repository.

I prefer an exception, if I don't give encoding, since I would get this exception during development. 

With the current implementation the guess could be good during development, and wrong in the live system.


Charade _is_ optional. As you've spotted, if you set the encoding yourself, we won't use it.

As for making the charade download itself optional, that doesn't fit Requests' policy of making the common use case easy. We're strongly disinclined to force people to go out and download charade to make encoding guessing work.

If you really can't spare the 500kB, it's not very difficult to strip out charade yourself and use that packaged version of the library. But it's hard to imagine that you really can't spare 500kB. =D


I just wanted state my concerns. I am happy with your decision. 


@guettli Thanks for providing your feedback! It's always appreciated. :cookie:
#1746 don't lowercase the whole url

#1746

Modified adapters not to lowercase the whole url.
Because `urlparse` automatically makes scheme lowercase, `scheme.lower()` is unnecessary.


Heh, so it does. Good spot!

I'm happy with this, :+1:.


Not sure if this is useful, but in the course of investigating urlparse/ursplit for my own purposes, I thought an explicit comment here might be good for the long term -- the standard library docs don't seem to mention at all that urlparse/urlsplit lower-case/normalize the scheme, so it seems to me that doing it that way would depend potentially on a side-effect that could change.

However, the docs do say that `geturl()` invoked on a structured parse result "may differ from the original URL in that the scheme may be normalized to lower case and empty components may be dropped" (http://docs.python.org/3.3/library/urllib.parse.html#structured-parse-results), so presumably if that's the effect you want, you should get it by using `SplitResult.geturl()` on the result from an urlparse/urlsplit.

Thus +daftshady's decision to use geturl to do the scheme lowering, instead of parsing and then unparsing, only?

Apologies if this level of explicit comment isn't considered good form.


@ViktorHaag it's considered the most excellent of form! Thanks for the fantastic observations.
Don't lowercase the whole URL when opening a connection

As mentioned [here](https://github.com/kennethreitz/requests/commit/01993d21dc7d6fa8f0acf657425d2fead9b2ddea).

The specific line of code in question is [here](https://github.com/kennethreitz/requests/blob/master/requests/adapters.py#L209). We're lowercasing the whole URL. This _should_ be fine, but in practice there are clearly servers that will expect the `Host` header to match a specific case (because they're stupid).

We should fix this, especially as fixing it is so easy (we only want to lowercase the scheme) and doesn't hurt us at all.


I agree; it probably should have just been an URL with lowered() scheme, leaving the rest intact. My apologies for this.


@ViktorHaag we will never forgive you. You've broken requests in such an irreparable way that you are banned for life. (I'm kidding of course. It isn't a big deal at all. We all make mistakes. Thank you for contributing what you did.)


On a separate, more serious, note since #1747 was merged, should we leave this open until the next release of requests or close it now? It might be useful to have open in case others want to file the same issue but then again, we've seen that people sometimes don't even look at open issues before filing their own. Opinions @kennethreitz @Lukasa ?


I've established a trend with #1732 of leaving issues open until the next release. I think we should continue to do that. =) I'm sick of people repeatedly re-raising fixed issues. The other option is to maintain a list of issues that have been fixed in trunk in somewhere like a GitHub wiki.


@Lukasa It might be worthwhile to tag them as "fixed" in order to help people get a better perspective on what is still open. Of course, I'm being selfish here as I was just browsing to see if there was any low hanging fruit and read this thread ;)


@ionrock Not a bad idea actually, would help me keep track of which ones have to be closed at the point of release.


I've gone through and started adding that label to some of them


Fixed in 2.1.0.
Small fix broken cookie parse

Code:

``` python
import requests
resp = requests.get('http://www.servanegaxotte.com') 
# or khvigra.ru, nastersrl.ru, spcentre2000.ru etc.
```

Raised exception:

```
  File "C:\Python27\lib\cookielib.py", line 1645, in extract_cookies
    self.set_cookie(cookie)
  File "C:\Python27\lib\site-packages\requests\cookies.py", line 270, in set_cookie
    if cookie.value.startswith('"') and cookie.value.endswith('"'):
AttributeError: 'NoneType' object has no attribute 'startswith'
```


IMHO, is it better to check whether `cookie.value` is not None or catching AttributeError instead of using `hasattr`?


Ordinary I'd insist on catching `AttributeError`. However, because of short-circuit logical evaluation, in this case I think `hasattr` is the better way to go.


 :sparkles:  :cookie: :sparkles:
Separate connect timeouts

Support separate connect timeouts in Requests by allowing you to write something like

``` python
import requests.structures.Timeout
t = Timeout(connect=3, read=7)
requests.get('https://api.twilio.com', timeout=t)
```

Adds tests, docs and some new exceptions for this functionality.

Please note, this kills the current `stream` based branching of timeouts. The `timeout` value applies only to the amount of time between bytes sent by the server. So, if a read timeout is set to 20 seconds and the server streams one byte every 15 seconds, the read timeout will never get hit. This means long streaming downloads are ok and won't hit the read timeout even if it's specified.

It is perfectly valid to set a read timeout on a streaming request - if the request doesn't start streaming in 15 seconds, then raise an error. With the current master, this is impossible.


I'm 100% sitting on the fence here. This is an API change, so we need to see if @kennethreitz likes it or not. =)


I don't think this is needed, personally. I appreciate the contribution, though.

Perhaps we should automatically enforce the timeout for the first bytes being sent for the stream. Would this solve your initial problem?


> I don't think this is needed, personally.

Are you referring to separate connect timeouts in general, or the ability to set a timeout on a streaming request? I'm more or less indifferent about the latter, and it's easy to revert.


Well, I've done a lot of work to try to prevent there from being a Timeout object :)


That's fine... I guess I am indifferent about the interface, but would appreciate the ability to specify different connect and read timeouts via python-requests.


@kevinburke i'll try to chat with you on IRC about this this week. :)


Sorry if it seems like I'm being OCD about mundane details... that's kinda the point of the project though :)


This is pretty essential for any use of a streaming API, since without a way to set a timeout there's no good way to detect a stalled request. Twitter, for instance, sends a newline every 30 seconds, and advises you consider the connection stalled if 90 seconds go by without one.

I naturally assumed that the timeout parameter applied to all reads as documented, and wasn't special-cased for streaming requests - why not just revert to that behaviour?


That's basically what I'm thinking of doing :)


Why not use a tuple to represent the different kinds of timeout when people want to differentiate. It avoids adding a `Timeout` class and we can destructure it to the above usage?


Hi Kenneth,
Would be nice to chat a bit more about your reasoning for closing this PR - I've tried reaching out in IRC, not sure of the best way to get in touch.

Best,
Kevin
Added multipart param, to be used in post request (harmless otherwise)

basic usage:
r = requests.post(url, data=data, multipart=True)

it will behave same as
r = requests.post(url, data=data, files={'filename':data})
whithout having  ----> ; filename="filename" <----- in the Content-Disposition's multipart body

this somethimes will not be accepted:
Content-Disposition: form-data; name="data"; filename="filename"

this it should:
Content-Disposition: form-data; name="data"


Thanks for this! However, we are almost never going to accept new parameters in the Requests functional API, especially not ones that are simply True/False. =)

The expected method for doing what you're trying to do [can be found here](http://stackoverflow.com/a/19974452/1401686). Basically what you want is:

``` python
r = requests.post(url, files={"data": ("", data, '', {})})
```

Again, thanks for putting in the time! Let me know if you have any further problems.
Wheel

Let's get ourselves green on [this list](http://pythonwheels.com/). We're pure python, so this should be totally trivial for us. I'll try to provide a PR tonight.


Alright, I've extended the Makefile so that it should be possible to just start doing this transparently. =) Let me know what we think!


:+1:

Can we add "wheel" to the (dev-)?requirements file? This means that any virtualenv will be fully equipped to deploy it. Then again not everyone needs it.


Sure, why not?


I guess this makes me doubly :+1:?


I was so afraid of this


:wheelchair: 


Why were you afraid @kennethreitz ?


i just thought it'd be a lot harder than that. :P
Response and Request objects are pickleable.

Includes a basic test. More could be add to confirm known attributes
that could cause problems.

This should fix #1367.


Looks fine in principle to me, but I'd want @sigmavirus24 to take a look. =)


If you feel you want to add more tests @ionrock then go ahead but I'm happy with this as it is. :+1:


:pickle: (this should be a thing)

Thanks!
SNI support broken on Python 2.X in 2.0.1.

`urllib3` had a bug, we accidentally pulled it in, we didn't catch it because we don't test with SNI support (should we have that as a separate build?). Happily the awesome @t-8ch has already provided us with a patch, which is already merged, so we don't necessarily have an action on this: it'll be quietly fixed in the next release. Just raising this issue so that people who look on the issue tracker see it.


I bumped into this today, adding a little background to help some people who might have the same issue:

It's not just that SNI support is broken ; if you have the relevant dependencies installed, it will be impossible to install requests 2.0.1. To reproduce, do the following in a new virtualenv:
- pip install pyOpenSSL ndg-httpsclient pyasn1
- pip install requests==2.0.1

Here is what I get:

```
Downloading/unpacking requests==2.0.1
  Downloading requests-2.0.1.tar.gz (412kB): 412kB downloaded
  Running setup.py egg_info for package requests
    Traceback (most recent call last):
      File "<string>", line 16, in <module>
      File "/home/mat/.virtualenvs/testenv/build/requests/setup.py", line 6, in <module>
        import requests
      File "requests/__init__.py", line 53, in <module>
        from .packages.urllib3.contrib import pyopenssl
      File "requests/packages/urllib3/contrib/pyopenssl.py", line 55, in <module>
        orig_connectionpool_ssl_wrap_socket = connectionpool.ssl_wrap_socket
    AttributeError: 'module' object has no attribute 'ssl_wrap_socket'
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):

  File "<string>", line 16, in <module>

  File "/home/mat/.virtualenvs/testenv/build/requests/setup.py", line 6, in <module>

    import requests

  File "requests/__init__.py", line 53, in <module>

    from .packages.urllib3.contrib import pyopenssl

  File "requests/packages/urllib3/contrib/pyopenssl.py", line 55, in <module>

    orig_connectionpool_ssl_wrap_socket = connectionpool.ssl_wrap_socket

AttributeError: 'module' object has no attribute 'ssl_wrap_socket'
```

Cleaning up the broken build and installing any other version works.


Good spot, I only bumped into this the other way around (requests installed, then subsequently install the dependencies for SNI).


Fixed in 2.1.0.
SSL certificate verify failed

When using requests on https://cloudconvert.org/ i get this Exception, but Firefox says the certificate is alright.

I alredy notified cloudconvert, and wil update this accordingly, but as far as i know this can be a requests issue.

```
Traceback (most recent call last):
  File "C:\Users\Gcq\Documents\GitHub\python-cloudconvert\CloudConvert.py", line 75, in <module>
    print requests.get("https://cloudconvert.org/contact").text
  File "C:\Python27\lib\site-packages\requests\api.py", line 55, in get
    return request('get', url, **kwargs)
  File "C:\Python27\lib\site-packages\requests\api.py", line 44, in request
    return session.request(method=method, url=url, **kwargs)
  File "C:\Python27\lib\site-packages\requests\sessions.py", line 335, in request
    resp = self.send(prep, **send_kwargs)
  File "C:\Python27\lib\site-packages\requests\sessions.py", line 438, in send
    r = adapter.send(request, **kwargs)
  File "C:\Python27\lib\site-packages\requests\adapters.py", line 331, in send
    raise SSLError(e)
SSLError: [Errno 1] _ssl.c:510: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
```


Firstly, what version of Requests are you using? Requests 2.0.1 was released recently that contains a number of updates to our baked-in certificates.

Using 2.0.1, I get different behaviour to you. In Python 3, everything works perfectly out of the box. In Python 2, you'll need to install the requirements for SNI support. It's worth noting that our SNI support is actually bugged in 2.0.1: this will be fixed in the next release.


I had an outdated version indeed. I updated to 2.0.1 and still getting the exception.

What are the requeriments for SNI? I looked trough some related issues, ans installed urllib3, but that doesn't fix it.


Are you getting that exact exception? Because I get one that's clearly related to SNI.

The requirements for SNI in Python 2.X can be found [here](http://stackoverflow.com/questions/18578439/using-requests-with-tls-doesnt-give-sni-support/18579484#18579484).


The Exception is exactly the same. No SNI related. Should I install the requeriments and see if it works or am i missing something?


No, 2.0.1 has bugged SNI support so it won't work. That's very strange. Can you try in Python 3.3?


Installing Python3. How can i get the commands to work without messing up my Python27 installation?


Python 3 should cleanly install alongside Python 2.7. You can install pip in Python 3.3, and then use the Python 3 pip executable to install Requests.


OK. It works. But sadly this means i'll have to change to Py3k. A change i would have to do sonner than later, but really sucks.

Anyway :) thanks. It works.


I expect the SNI issue should get fixed in the main release fairly soon, which will mean you can go back to Python 2.7.


Alternatively, you can install the version of Requests from the GitHub repository, which already contains the SNI fix.


Thats easier! Thank you.
lowercase cookie names when check

lowercase cookie names when check badargs


I'm +1 on the replacing of `type` with `isinstance`. It's less clear to me that we need to lowercase all the keyword arguments to `create_cookie`. Why is this a problem?


I want to pass the badargs check and set a cookie which contains upper-case letter. I just lowercase cookie names when checking badargs.

```
create_cookie(name, value, Domain='example.com')

d = {'Domain': 'example.com', 'Path': '/'}
create_cookie(name, value, **d)
```

or I should lowercase them before passing?


Hmm. Is it necessary to have the `Domain` key be uppercase?

Ordinarily, if you are using keyword arguments (which we are), you would have to match the case of the argument.


Ok. I will process them before passing.
Fix for #1728

This is how I think things should be fixed if we are okay with adding the cookiejar to `PreparedRequest`s. What do you guys think?


I want to have a discussion on #1728 before we merge this. =)

I'll make some comments inline on the diff anyway though.


@Lukasa if I take over the rest of this PR (to address our feedback) would we be comfortable getting it into 2.1? I feel like this addresses a real concern and I think you were in agreement with @gazpachoking at the end of #1728. Correct me if I'm wrong. 

For reference, I'll pull his commits onto a branch of mine, and just fix up the one minor thing and send a new PR and then close this one. Sound good?


If you think you can get it done in a fairly swift manner, sure. =) 


@Lukasa done in #1776
Proxy configuration is not re-evaluated on redirects

`requests.get` will fail if a URL which falls under `no_proxy` policy is redirected to an URL for which a proxy is mandatory.


Yep, that's definitely right. Good spot! Thanks for raising this issue! :cake:


@sigmavirus24 I really want to add a regression test for this, but we have essentially got no testing infrasrtucture for  proxies. Do you have any bright ideas/will Betamax help with this, or do I need to look into spinning up an open HTTP proxy?


Frankly I'm not sure if Betamax would help. It works on the Adapter level so the answer is possibly not.


@Lukasa frankly, I don't understand the problem well. Proxies have always been your thing and I never attempted to get enough background on them.

So what Betamax does is replace the adapters that the Session object is using and then use a regular HTTPAdapter to make the actual request if necessary. The reason I'm not certain, is because of [`get_connection`](https://github.com/kennethreitz/requests/blob/master/requests/adapters.py#L187..L211] on the `HTTPAdapter`.

Betamax looks at the Prepared Request that comes through and then matches that against previously recorded (if any) requests to find an appropriate response. Because of how requests handles redirects, even redirects are recorded properly. If the logic you're looking for happens outside of the HTTPAdapter (any of the layers above it), Betamax can probably help so long as the info is on the Prepared Request. If it happens underneath the covers of the HTTPAdapter, then the answer is still "maybe" because the request may have the data you're looking for, but I'm not certain.


Hmm. I'll have a think. This is awkward to test. It might be possible to stub out something really low level.


Have there been any progress on this issue? 


Ah, sorry, there has not. I'll put this on my list of things to look at this week. =)


This appears to get handled now via `rebuild_proxies` and `rebuild_auth`:
https://github.com/kennethreitz/requests/blob/master/requests/sessions.py#L175-177

(via https://github.com/kennethreitz/requests/commit/4d8cb3244e8e4f84b250c10a48e025f9a8bf6137, should be 2.3.0)


Ah, correct, so it is. Thanks @blueyed!
Update urllib3

... and use the opportunity to add myself to `AUTHORS.rst`


LGTM. Always happy to take new urllib3 code!


:sparkles: :cake: :sparkles:


@t-8ch how were you not already in the AUTHORS.rst file? This is an unacceptable oversight and I'm glad you remedied it.


@sigmavirus24 To be honest, there are only a handful of contributions codewise (directly to requests not updating urllib3) by me. 
It seems you also took your time when adding yourself to AUTHORS.rst. Happens to the best. :smile: 


AUTHORS should be updated for even a typo fix :)


I was saving my addition for my first major contribution to requests (and coincidentally OSS as a whole). ;)
Fix ssl_wrap_socket AttributeError on python2

Importing requests on python2 raises an AttributeError in pyopenssl.py

```
Python 2.7.5 (default, Sep  6 2013, 09:55:21) 
[GCC 4.8.1 20130725 (prerelease)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import requests
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/site-packages/requests/__init__.py", line 53, in <module>
    from .packages.urllib3.contrib import pyopenssl
  File "/usr/lib/python2.7/site-packages/requests/packages/urllib3/contrib/pyopenssl.py", line 55, in <module>
    orig_connectionpool_ssl_wrap_socket = connectionpool.ssl_wrap_socket
AttributeError: 'module' object has no attribute 'ssl_wrap_socket'
```


Thanks for this! Unfortunately, this bug is in [urllib3](https://github.com/shazow/urllib3). We include an exact copy of urllib3 in Requests, so we don't accept fixes for it here. Can you propose this fix over there please?


Sorry, it seems to have been fixed urllib3 already and was reported in #1705 (my search did not bring it up as it did contain a screenshot instead of the textual representation of the AttributeError).

The only thing needed is to pull an updated version of urllib3.
Fixed #1723

I'm unsure that using unicode to `method` is expectable behavior or not.
but this pull request will fix #1723


Unicode to method is expected behaviour in Python 3. This fix looks good to me. :+1:


@daftshady please use pytest's capabilities to add a test that should only ever fail on Python 2.x please.


Also, please move this logic into the Session object's `request` method. Someone could just as easily create a session instance and do the same thing.


@sigmavirus24 I moved logic and added test case. Please review this. Thanks.


:sparkles: :cake: :sparkles:


:cake: Thanks @daftshady