Raibek/requests-github-issues huggingface dataset:
Automatically executes script in working folder if its name "cgi"

Requests version - 2.10.0
As soon as the working folder folder contains script with name "cgi.py" (it was part of my application), your package automatically executes it. In my case - I fixed that myself by renaming my script. For another users - probability of such case is not zero. (In my case it was about 1 hour of fighting with strange behavior of my application). To reproduce it, it is possible to create 2 simple scripts, e.g.: 

**test.py**:
_import requests
print "Hello from test"_
**cgi.py**:
_print "Hello from CGI!"_

Possible solutions: to reflect such behavior in documentation or to fix it.

P.S. Very nice package, it saved me much more time this lost hour!


This is not a bug: this is unfortunately the way Python works. Python puts the current working directory at the top of the import namespace, which means that when we try to import standard library modules, if you have a script in the working directory with the same name, we will accidentally get that instead. 

To prevent your script from executing in these circumstances, you will want to use the [`if **name** == '**main**':](http://stackoverflow.com/questions/419163/what-does-if-name-main-do) pattern. That would change this error to either an ImportError or an AttributeError, which are better failure modes. 

Otherwise, try to avoid naming your scripts after stdlib modules. (You'd have similar problems with, for example, naming a script 'urllib.py'.)
updating documentation to reflect decision of #3386

These are a couple of minor doc tweaks to document the decision to enforce strings in header values. Since requests previously operated with non-string header values, I figured it was best to make note that an exception will now be thrown in the event of a non-string value.

There may be some formatting and wording changes needed, this iteration is mainly to get the conversation going.


Cool, LGTM! :sparkles: @sigmavirus24?


While you're here @sigmavirus24 and @Lukasa, I threw together [another commit](https://github.com/nateprewitt/requests/commit/be31a90906deb5553c2e703fb05cf6964ee23ed5) related to this to encapsulate the type error thrown by `re` when non-strings are passed to `check_header_validity`. This is to make it clear that the behaviour is to be expected, but may be overkill with this documentation now. Any thoughts on if this would be worth opening? 


Thanks @nateprewitt 🍮 🍰 ☕
fixing #3366 for non-str header values

This will fix issue #3386 with non-string/buffer header values, but there may be some more discussion to be had here. This is a passthrough for non-string/buffers which means all other datatypes will skip the regex check. I wasn't able to create the header split issue with most of the other standard datatypes I checked (lists, sets, dicts), so this _should_ be safe with the passthrough. 

I think an alternative, and possibly better solution is to cast everything that isn't a byte string as a `str`, whether that be standard, or `encode('latin-1')`. This will ensure the check still runs and doesn't modify the value in the actual `Requests.headers` value. This is probably closer to what @sigmavirus24 already did [here](https://github.com/kennethreitz/requests/pull/866/files#diff-5956087d5835a57d9ef6fff974f6fd9bR273).


@nateprewitt that pull request was _rejected_ precisely because integers are not defined behaviour within requests and haven't been for _years_.


Sorry, @sigmavirus24, I was suggesting something along this line for the check since it wouldn't be transforming the value in this case. I was trying to use this as a template example. I realize the documentation says that headers must be strings, and that was the assumption I was operating off of when I wrote #3366. I submitted the passthrough as the initial pull request here because it avoids dealing with any defining behaviour by ignoring non-strings.

I just wanted to make sure both options were at least briefly discussed.


Yeah, so the question boils down to whether we handle non-string headers. Clearly we've oscillated around for a while: they didn't work, and then they did, and now they don't again.

However, what I failed to note is Kenneth's original response in #865. With that in mind, I'm inclined to want to continue to respect his wishes and say that non-string headers are not acceptable.

Sorry for having you do this work @nateprewitt!


No problem, glad to have the question resolved.
The keywords headers must be string or buffer in new version 2.10.0?

The keywords `headers` value must be string or buffer?
If the  keywords `dnt` and `upgrade-insecure-requests`  value is  **string** or **buffer**, it's right,but the value is int,it's wrong.Why is that?
Right:

```
   'dnt': '1',
    'upgrade-insecure-requests': '1',
```

Wrong:

```
   'dnt': 1,
    'upgrade-insecure-requests': 1,
```

Why?


@wut0n9 This behavioural change is not in v2.10.0, it's in the current master branch.

However, that's a real bug: #3366 has regressed this. @nateprewitt, are you interested in trying to update with a fix for this?


Headers should always have been string. I don't think this is a significant regression if a regression at all. We've always documented that header values should be strings.


Regardless of what we documented, this used to work and now it doesn't. We'll break a _lot_ of code if we don't change this back.


Yeah, I'll get right on this @Lukasa.


See also https://github.com/kennethreitz/requests/issues/865 and https://github.com/kennethreitz/requests/pull/866

I don't know how much a "lot" actually is in this case.


@sigmavirus24 has linked to the relevant earlier opinions, which suggest that actually we don't allow non-string header values. So that means this is not a bug: it was us making a revision that is within the scope of the API definition.


@Lukasa right, I'm frankly surprised this didn't break earlier. The meaning of non-bytes/str as a header value is undefined as far as I'm concerned.
Support responses like `HTTP/1.1 404 öööööööö`

Currently this code raises UnicodeDecodeError:

`requests.get('http://www.voice.fi//lampaan-kiveksia-ja-coca-cola-hulluutta-5-asiaa-joita-et-tiennyt-islannista-122015?utm_source=feed&utm_medium=feed&utm_content=article_link&utm_campaign=feed').ok`

Here is their status:
`HTTP/1.1 404 Komponenttia ei löydy`

This pull request allow module to handle URLs like this correctly


Generally speaking I'm not too interested in supporting this kind of logic: servers are really not supposed to be sending non-ASCII-bytes in the reason phrase, and choosing to assume that UTF-8 is being used is really fairly optimistic.

However, in the case of `raise_for_status`, we're exploding in a situation that we don't need to. RFC 7230 says we should ignore the reason phrase, and we're choosing not to do that, so the least we should do is _not_ explode if it's weird. For that reason, I'm in favour of a patch like this, though I'd like us to change the decode logic to make sure that we will _always_ successfully decode the reason phrase, even if we decode it to gibberish.


@Lukasa is `reason.decode('utf-8', 'ignore')` ok?


`'ignore'` is fine.


@Lukasa done, updated PR


@Lukasa done


Cool, I'm happy with this as-is. I'd like @sigmavirus24 to ACK as well, if possible.


I have a question or two inline. Otherwise, I agree we shouldn't have this exception raised, even if the server is in the wrong.


@gugu just use `u''`. Further, stop importing six. It's not a dependency of requests.


six is a _test_ dependency of requests though, it's acceptable to use there.


> six is a test dependency of requests though, it's acceptable to use there.

Sure but there's little sense to using `six.u` in the tests when we already use `u''` in the tests.


@sigmavirus24 @lucasa done, updated PR


Looks good to me. @Lukasa, the changes are minimal from when you gave your LGTM, so I'm merging. :)


Thanks @gugu! :sparkles: :cake: :sparkles: :fireworks: 


y'all emoji-chain thieves!


Is there any plan to make a new Pypi release? We would rather bump `requests`' version in our dependencies instead of referencing a GitHub commit.

(We're affected because of the [`raven` library calling `response.ok` on all our `requests` calls](https://github.com/getsentry/raven-python/blob/6b5044ab72040aa5678f9c522dacb7099cbe8829/raven/breadcrumbs.py#L275))


We're currently tentatively planning a release for next week. =)


Great


Thank you all for v2.11!
Will requests bundle with future version of python?

Will requests bundle with future version of python like pathlib? 

I think urllib is not intuitive.
I hope that requests replace urllib as a intuitive high level http request module.


Thanks for this question! The answer I'm afraid is "no". Requests is security-critical for most users, which means we need to be able to react quickly to security vulnerabilities and ensure that they reach all users. That's not possible if we're in the standard library because of its requirements on backward compatibility and long release cycles.

Additionally, it's never been easier to get a copy of Requests for your Python. Modern Pythons ship with pip, which means that Requests is a simple `pip install requests` away: one small command line. This means that the convenience advantage of bundling with the standard library is pretty minimal.

For this reason, we will not be part of the standard library: we can do better work by being outside it.


https://speakerdeck.com/kennethreitz/python-requests-and-the-standard-library


The official decision was made for the official Python documentation to recommend Requests, instead. It's now prominent in the urllib documentation.
Change exception and variable names so that tests will run.

@Lukasa @sigmavirus24 

I tried running the tests on the proposed/3.0.0 branch, and found that they did not run. So I changed a couple of variable names to fix the obvious errors. If you prefer InvalidSchema and MissingSchema I can change those back, I changed the imports to match the names of the exceptions as they were defined.

After these changes, there is still one failing test. I'll try to take a look and see if I can debug it. But this was low hanging fruit.


Looks good to me, thanks!
updating iter_content docstring to match functionality

Sorry to be bombarding you guys with PRs. This one is pretty minor but explains what I would say was non-obvious behaviour of the function before PR #3368 and PR #3365.


So this isn't quite right. In particular, `iter_content` with `stream=True` and a chunk encoded response will iterate in chunk size. So it would be better for this to say that it iterates in chunks of undefined size. 


Looks like I failed to set stream=True in my test case. I was misinterpreting the call to `r.content` as the stream read. Thanks @Lukasa, string updated.


Cool, I'm happy with this! Thanks @nateprewitt! :sparkles: :cake: :sparkles:
adding in slice_length fix and test for chunk_size=None

This fixes the issue discussed in #3369 but may not be the best way. It adds a certain amount of additional complexity to iter_slices that may be better solved by using a try/except. If we throw an exception requiring an int for slice_length, we could pass `chunk_size or len(self._content)` [here](https://github.com/kennethreitz/requests/blob/master/requests/models.py#L691) and solve the issue that way as well.


I'm happy with this, but would like @sigmavirus24 to take a look as well.


Actually one quick note on this, it looks like we actually should have an integer > 0 as a requirement. slice_length <= 0 produces an infinite generator. I can modify the check if that's something you think should be caught (I do).


Sure thing =)


Ok, I'm still happy, but would still like @sigmavirus24 to weigh in. 


:+1: Looks fine to me. Thanks @nateprewitt!
TypeError in iter_slices() when slice_length=None

An unintended side effect of using `chunk_size=None` (as referenced in PR #3368) is a failure in `iter_slices()`. The two code snippets below will both trigger [`reused_chunks` being chosen](https://github.com/kennethreitz/requests/blob/master/requests/models.py#L695) as the return value of `iter_content`. The generator is produced by `iter_slices` which currently requires the chunk_size to be an integer, otherwise it raises a TypeError when [trying to add an int and NoneType](https://github.com/kennethreitz/requests/blob/master/requests/utils.py#L388).

Perhaps these are obscure enough edge cases, that it's not an issue. I just wanted to make a quick note of it.
#### Examples

```
def test_stream():
    s = requests.Session()
    req = requests.Request('GET', 'https://httpbin.org/get').prepare()
    resp = s.send(req, stream=False)
    next(resp.iter_content(None))

def test_decode():
    r = requests.Response()
    r.raw = io.BytesIO(b'the content')
    all_content = r.content
    decoded_chunk = next(r.iter_content(None, True))
```


Why does `iter_slices` with `slice_length = None` not trigger a `TypeError`?


Sorry, I'm not sure I understand the question. I'm currently seeing the following error returned with both python2.7 and python3.5.

`TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'`

A more direct code snippet for reproduction would be:

`next(requests.utils.iter_slices('my string', None))`


Ok, that's good, I just didn't read the report clearly enough I guess. =)
Allow None value for chunk_size again

Passing in `chunk_size=None` used to be fine, but started raising a `TypeError` following the addition of the type check (#3365)


Thanks @joyzheng! :cake: 


It may also be worth updating the `iter_content` docstring to reflect the "readall" behaviour with `iter_content(None)`.


Yeah, good point. I'm not actually sure about all the behavior cases for `iter_content(None)`, since it doesn't always result in reading the entire input; the primary one I've used it for is chunked encoding with `stream=True`, where setting `chunk_size=None` iterates chunk-by-chunk.
Add let's encrypt ca certificates to trusted certs

IMO requests should trust let's encrypt issued certs: www.letsencrypt.org


Do you have evidence that we don't?

## If so, can you share the URL and version of requests, Python, and openssl?

Sent from my Android device with K-9 Mail. Please excuse my brevity.


Requests does trust LE: LE uses a cross signed root that we have chained up to for years. 


MMh, then why do I get an SSL error when I request https://auxdb.app.tu-dortmund.de? It is using a letsencrypt certificate and all browsers I tried trust this certificate.

`requests.get('https://auxdb.app.tu-dortmund.de')`

I added the 4 intermediate certificates of letsencrypt to my systems ca-certificates:
https://letsencrypt.org/certificates/

This works:

```
requests.get('https://auxdb.app.tu-dortmund.de', verify='/etc/ssl/certs/ca-certificates.crt')
```


What version of requests do you have installed, what packages on your system do you have installed, what OS are you on, and what OpenSSL version do you have?


Are you saying that you can request the given url without ssl error? 


@MaxNoe The server does not server the intermediate certificate(s) like it should (browser cache them from other sites, so it may work)
Compare:

``` sh
$ openssl s_client -connect auxdb.app.tu-dortmund.de:443 -CAfile /etc/ssl/certs/ca-certificates.crt
...
Certificate chain
 0 s:/CN=git.e5.physik.tu-dortmund.de
   i:/C=US/O=Let's Encrypt/CN=Let's Encrypt Authority X3
...
Verify return code: 21 (unable to verify the first certificate)
```

``` sh
$ openssl s_client -connect t-8ch.de:443 -CAfile /etc/ssl/certs/ca-certificates.crt
...
Certificate chain
 0 s:/CN=t-8ch.de
   i:/C=US/O=Let's Encrypt/CN=Let's Encrypt Authority X3
 1 s:/C=US/O=Let's Encrypt/CN=Let's Encrypt Authority X3
   i:/O=Digital Signature Trust Co./CN=DST Root CA X3
...
Verify return code: 0 (ok)
```

Certbot saves the chain to `chain.pem` and `fullchain.pem` (including the leaf cert).


Mhh, ok. So it's a problem on the other hand. Sorry to bother.


Ok, got it. I use a python script to upload the certificates obtained by letsencrypt into the rancher api. I got the keyword for the chain cert wrong. Everything works now.


Glad to hear that Requests is of a level of quality that it accidentally led you to fix your improperly configured server.

We should start selling a security audit edition license for corporations :)
Allow sending an unprepared request via session.

This can be considered a request in the form of a PR because it was easier to make the code change than explain it. 

I have multiple places where i am passing around method, url, json etc before it makes it to the requests level. Now these can all be bundled up into a single request object which is much easier to pass around, but then i have to manually call

prep = session.prepare_request(req)
settings = session.merge_environment_settings(....) 
settings['timeout'] = timeout
session.send(prep, **settings)

Now it's not super hard, but it's something that happens by default doing request() and it's certainly not intuitive. If we add a send_request() which does exactly the same as request() just on an existing request object it makes it much easier to correctly use Request objects.

Note: If you like the idea but not the implementation and have something equivalent - please just do it your way. I don't care if this particular PR gets accepted or not if the idea gets up. I proposed this against 3.0 thinking it was going to be a more breaking change, but it seems pretty easy if you'd prefer i propose against master.

Thanks


PreparedRequests should be suitable for accomplishing what you're trying to accomplish.

I see where you're coming from, and thank you for your contribution. However, in short, no. :)

@sigmavirus24 & @Lukasa are welcome to add additional thoughts, if desired.


I once thought about this too. Then I realized that you're introducing yet another way of doing things when there already exist more than one way to do it. You can do:

``` py
#: 1
requests.get(...)
#: 2
requests.request('GET', ...)
#: 3
my_session.get(...)
#: 4
my_session.request('GET', ...)
#: 5
request = requests.Request(method='GET', ...)
prepared = my_session.prepare_request(request)
settings = my_session.merge_environment_settings(....) 
# modify settings
my_session.send(prepared, **settings)
```

We really don't need yet another way of doing roughly the same thing. That's too much API surface area for very little extra benefit. If you're using the PreparedRequest flow, your proposal is only saving a tiny fraction of that workflow and diverging the typical code-paths further. Frankly, I don't think it's worth the extra surface area. In my experience, people who think the PreparedRequest flow is too complicated or kludgy, don't actually need it.
Custom ResponseClass

A common pattern in webob is allowing applications to define there own request and response class. This makes it easier to work with and expose attributes and methods that you will commonly use in your application. For example you can add properties for headers that are commonly used in your application. 

Theoretically you could do the request side of this today subclassing requests.Request, however the more useful side here would be being able to specify a response class (it can be asserted that custom response class is a subclass of requests.Response).


Thank you for the idea, but again, in short brevity: no. :)


> For example you can add properties for headers that are commonly used in your application.

You could also do this with a separate response class (that takes a requests.Response) as a parameter and uses descriptors for that. OpenStack already has a lot of "session" classes that use requests.Session instances under the covers. I don't understand why they couldn't have their own Response object that uses descriptors as intended to achieve a similar goal.
Do not attempt to decode to unicode twice

It's possible I'm missing something here, but it seems like we should not be trying to decode via the underlying urllib3 method since `iter_content` is responsible for the decision of whether or not to decode.


Thanks for this!

The urllib3 decoding is about decoding compressed content-encodings like gzip and deflate. We do always want that on. =) iter_content's decoding is about decoding to unicode, an unrelated concern.


Ah, thank you for the explanation :)
'True' payload in get cause problem

``` python
payload = {'key1': 'value1', 'key2': True}
r = requests.get('http://httpbin.org/get', params=payload)
print(r.url)
# 'http://httpbin.org/get?key1=value1&key2=True'
```

With `key2` I wanna get a boolean param, but requests gives me 'True'. 'True' will be considered as a string in many frameworks.


Is it a bug, or designed to be so?


@lufeihaidao HTTP query parameters are not typed: they are _all_ strings. There is no such thing as a boolean parameter in a query string. What do you think a boolean parameter is, in this context?


@Lukasa Sorry, in my app, `key2` is supposed to be a boolean field. And many frameworks' boolean validations only check whether the `key2` string is '1' or 'true',rather than 'True'. So when the client assembles params with 'True', it will not pass the server's boolean validator.


In this case, I need to modify the server's boolean validator to support 'True'?


@lufeihaidao You could do that, or you could just pass the string `true` rather than `True`.

``` python
payload = {'key1': 'value1', 'key2': 'true'}
```


@Lukasa OK, thanks.


No problem.
Failed to establish a new connection: [Errno 65] No route to host

hi, I'm trying to

``` python
import requests
req = requests.Session()
headers = {'Referer':'https://archive.org/','User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:47.0) Gecko/20100101 Firefox/47.0'}
req.headers.update(headers)
res = req.get('https://archive.org/', headers=headers, timeout=20, proxies={'http': '52.192.0.225:8080'}, verify=False)
```

Then got ---------------------------------------------------------------------------
ConnectionError                           Traceback (most recent call last)
<ipython-input-23-1ddf77e531ff> in <module>()
      3 headers = {'Referer':'https://archive.org/','User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:47.0) Gecko/20100101 Firefox/47.0'}
      4 req.headers.update(headers)
----> 5 res = req.get('https://archive.org/', headers=headers, timeout=20, proxies={'http': '52.192.0.225:8080'}, verify=False)

/Users/test/env/lib/python2.7/site-packages/requests/sessions.pyc in get(self, url, *_kwargs)
    485 
    486         kwargs.setdefault('allow_redirects', True)
--> 487         return self.request('GET', url, *_kwargs)
    488 
    489     def options(self, url, **kwargs):

/Users/test/env/lib/python2.7/site-packages/requests/sessions.pyc in request(self, method, url, params, data, headers, cookies, files, auth, timeout, allow_redirects, proxies, hooks, stream, verify, cert, json)
    473         }
    474         send_kwargs.update(settings)
--> 475         resp = self.send(prep, **send_kwargs)
    476 
    477         return resp

/Users/test/env/lib/python2.7/site-packages/requests/sessions.pyc in send(self, request, *_kwargs)
    583 
    584         # Send the request
--> 585         r = adapter.send(request, *_kwargs)
    586 
    587         # Total elapsed time of the request (approximately)

/Users/test/env/lib/python2.7/site-packages/requests/adapters.pyc in send(self, request, stream, timeout, verify, cert, proxies)
    465                 raise ProxyError(e, request=request)
    466 
--> 467             raise ConnectionError(e, request=request)
    468 
    469         except ClosedPoolError as e:

ConnectionError: HTTPSConnectionPool(host='archive.org', port=443): Max retries exceeded with url: / (Caused by NewConnectionError('<requests.packages.urllib3.connection.VerifiedHTTPSConnection object at 0x105ea5fd0>: Failed to establish a new connection: [Errno 65] No route to host',))

I've tried to use proxies, set down the verify, but that does not work. Thank you 


Your proxy configuration appears to be wrong: you've set up a proxy for only HTTP schemed addresses. Do you want to try changing your `req.get` request to be:

``` python
res = req.get('https://archive.org/', headers=headers, timeout=20, proxies={'http': 'http://52.192.0.225:8080', 'https': 'http://52.192.0.225:8080'})
```


@Lukasa  OK. thank you .
fix PUT of empty (0 byte) stream

I found #3051 but I'm offering this anyway. It's a simple fix for what appears to be a simple oversight in the code.


Thanks for this!

This is not an oversight, it's entirely deliberate. We do this to handle the case that we have a body with indeterminate length (e.g. stdin) which reports its length as zero or throws errors when we try to work out how long it is. In this case, defaulting to transfer encoding chunked is sensible.

All servers should be able to tolerate this. Have you encountered an actual problem with this?


Thanks for the quick response. I was still looking into the test failures when I saw it.

It seems like the length should be `None` rather than zero in case of a body with indeterminate length, don't you think? :) 

I do have a real world problem with this. The archive.org s3-like api doesn't support chunked uploads and responds to a PUT without content-length with a "411 Length Required". 
https://archive.org/help/abouts3.txt

For example:

```
$ curl -LsSv --request PUT --upload-file /dev/null --header x-archive-meta-collection:test_collection  --header "authorization: LOW xxx:xxx" https://s3.us.archive.org/nlevitt-test-2016-06-17/
*   Trying 207.241.224.50...
* Connected to s3.us.archive.org (207.241.224.50) port 443 (#0)
* TLS 1.2 connection using TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
* Server certificate: *.s3.us.archive.org
* Server certificate: Go Daddy Secure Certificate Authority - G2
* Server certificate: Go Daddy Root Certificate Authority - G2
> PUT /nlevitt-test-2016-06-17/null HTTP/1.1
> Host: s3.us.archive.org
> User-Agent: curl/7.43.0
> Accept: */*
> Transfer-Encoding: chunked
> x-archive-meta-collection:test_collection
> authorization: LOW xxx:xxx
> Expect: 100-continue
> 
< HTTP/1.1 411 Length Required
< Date: Fri, 17 Jun 2016 19:34:15 GMT
< Server: Apache/2.4.7 (Ubuntu)
< Content-Length: 320
< Connection: close
< Content-Type: text/html; charset=iso-8859-1
< 
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>411 Length Required</title>
</head><body>
<h1>Length Required</h1>
<p>A request of the requested method PUT requires a valid Content-length.<br />
</p>
<hr>
<address>Apache/2.4.7 (Ubuntu) Server at s3.us.archive.org Port 80</address>
</body></html>
* Closing connection 0
```


The reason we fall back to zero is a side effect of the way lengths are calculated. If you dive back into bug reports you'll see that we had bugs where we sent bodies with Content-Length: 0: this behaviour is much nicer. =D

Archive.org in this case is wildly violating the HTTP RFCs, but you can force the correct behaviour by using the prepared request flow and setting up the headers and body as you need them. 


Ok. Thanks.


The archive.org s3-like API is extremely important, despite the fact that it is poorly implemented. Personally, I have utilized it successfully with Boto (which uses Requests internally, for some things). You may find the use of that library more appropriate. 

PreparedRequest is indeed the solution, if not, though.
use the paypal in mac os x has the problem about sslv3

requests.exceptions.SSLError: [SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:600)


I have change the terminal's openssl version,

kingwangdeMini:icampuslist_web kingwang$ openssl version
OpenSSL 1.0.2d 9 Jul 2015

but my python's openssl version is still old version：

kingwangdeMini:icampuslist_web kingwang$ python -c "import ssl; print(ssl.OPENSSL_VERSION)"
OpenSSL 0.9.8zg 14 July 2015

I know that [SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] 's bug is because of the version error.

I want to know that how to change my python's openssl version~
thanks!


Your Python's OpenSSL version is what it is compiled against. In this case, for Mac OS X, you'll need to compile yourself a new Python. The easiest way to do that is to use [Homebrew](http://brew.sh/): `brew install python` will provide you with a new Python that has a more modern OpenSSL.

You can also fix the bug for yourself by running `pip install pyasn1 ndg-httpsclient pyopenssl`, which will also give you a newer OpenSSL that only Requests can use.


This is my first use github to solve issue.
@Lukasa 
Thank you! And my bug solved now.

I use the pyenv to run the python. and I uninstall the python, then install again. It's working!   So happy~~

By the way, I tried the second method firstly, but It not working.  Then I tried the first one.I didn't know the reason. Maybe I had already installed that packages. 

Finally, Thanks very much!
Use seek from end rather than getvalue

Avoids having the (potentially large) object in memory twice.


Thanks for this patch!

As composed, however, this patch doesn't make sense. The code block tests for `getvalue()` but then uses two unrelated methods.

It might be better to have a separate block that tests for `seek()` and `tell()` and then use those in that block, considering that a higher priority than `getvalue()`. Does that sound sensible?


`seek` and `tell` are necessary but not sufficient conditions for it being a BytesIO object, no? That would also capture the file-objects that are the last thing checked for and use the filesystem.

The whole thing strikes me as a bit funny, but I didn't have enough context here to make wholesale changes. Why not just use `isinstance` be explicit about what's what? Python 2/3 issues? `getvalue` is a sufficient condition for BytesIO though.

Thoughts?


The main reason not to use `isinstance` is because we want to support duck-typed objects: things that behave _like_ a BytesIO object but are not BytesIO objects, for example. This is particularly important with file-like objects, as there is _technically_ a protocol to which they conform but _in practice_ many file-like objects support only part of the API.

In this case, we may be able to resolve the problem by changing the logic. Rather than have one gigantic `if...elif` chain, we could change it to be more like this:

``` python
total_length = None
if x:
    total_length = whatever()

if total_length is None and y:
    total_length = whatever2()

if total_length is None and z:
    total_length = whatever3()
```

That would allow us to re-order things to do the `fileno` block _first_ and then, if we drop out of the `fileno` block, fall back on seek() and tell(). How does that sound?


Makes sense. Let me give it another go.


See what you think about this. Added some code comments about why it's implemented this way.


I suppose we could run into a case where `current_position` becomes None in the 'tell' block, but I'm not sure under what circumstances.


Well, looks like that happens on Python 2.


🍝 


I'm not going to have time to see this through. Closing in favor of other intrepid souls. Happy to have a look at subsequent work.


Thanks for that @jseabold, it's helpful when people come back to tidy up when they no longer have the time to dedicate to the patch. All the best with whatever you're spending your time on, and I hope you hang around!


Thanks for contributing to Requests @jseabold!
Refactor prepare body

PR for proposed/3.0 with refactoring of `prepare_body` method. For discussion see #3184.


I can take a look tonight and see if I can fix the conflict.


@Lukasa @nateprewitt I sort of forgot that this PR has been hanging out - is this still something we'd like to merge in? I can see about getting the conflicts fixed. Sorry I sort of lost momentum working on requests, I switched jobs and don't get time at work for open source projects any more.


@davidsoncasey, no worries, thanks for taking the time to check back in! 😀

 @Lukasa may have more on this but I think moving [these tests](https://github.com/davidsoncasey/requests/blob/60e0349ce265378b127ae51e1853533b57eaac38/tests/test_requests.py#L1241-L1270) into a separate PR against master will be the most immediate benefit. This will show that the issue is currently fixed on master and allow us to close out #3066.

The work you did for `prepare_body` and `prepare_content_length` simplifies a lot of the logic and would be great for proposed/3.0.0. We'll need to merge master into the proposed/3.0.0 branch in this repo and then have you rebase your commits on top of it. I did some local testing and got this patch merged and working relatively pain free.

I'm also happy to help with any of the reshuffling of your commits if needed, just let us know.


Hey @davidsoncasey, just checking back in. If you don't have any qualms with this, I'd like to open a PR rebasing 1a01007 and lines 1264-1270 in tests/test_requests.py (e5f0993) onto master next week. This will verify the work for #3066 is finished there and then we can revisit the `prepare_body` consolidation here whenever you're ready.

@davidsoncasey, checking in again. It looks like @kennethreitz may want to start cleaning some of these older PRs up. There's still a lot of useful stuff in here that didn't make it into master but should be in 3.0.0. Would you be interested in bringing this branch up to date and fixing a few things? If not, I'll tidy up your commits and wrap this up.

needs a rebase

@nateprewitt feel free to go ahead and tidy up and get this into master. I haven't had a chance to work on this project recently, and it would take me a little to get back up to speed on it.

Thanks for checking back in @davidsoncasey, I'll move forward with rebasing this into a new PR then. All the best in whatever you're currently working on :)

Closing in favor of #3897
500 error when using requests.get()

I am new to requests and python I'm trying to run the following code but getting 500 Server Error.

import requests
url = 'https://developers.zomato.com/api/v2.1/categories'
r = requests.get(url,headers={'user-key':'apikey'})
print(r)
print(r.content)
print(r.headers)


The issue is with zomato api. Adding the user agent made it work.
Scrape  issue 

I tried to scrape http://www.nbcnews.com/health/health-news/brain-study-helps-explain-some-veterans-agony-n589916 .Then i got the below issue.

404 Client Error: Not Found for url: http://www.nbcnews.com/health/health-news/brain-study-helps-explain-some-veterans-agony-n589916%20 


You left a space at the end of your URL. You need to remove it: requests will not automatically do that for you.


Thanks for your reply. But i did't add any spaces to the url, i also saw that '%20' end of the url. how is it happened?


That %20 is the urlencoding for a space, which is how I determined what happened. Where did your URL come from?


This discussion belongs on [StackOverflow](https://stackoverflow.com/questions/tagged/python-requests).
Note how HTTPErrors are raised

After looking through the code line, `requests.exceptions.HTTPError`s are only raised by `raise_for_status` but the docs made it seem like that error could happen through other means. This PR makes the docs explicit.


I believe the original text is indeed correct, that error can occur when an HTTPError (not related to status code) is raised. I'm not sure, off the top of my head, when that could be anymore. It's possible that's a remnant from when requests ran on urllib2. 

Regardless, the _additions_ to the text are most certainly welcome.

This PR may be perfect as-is, but will need double checking, given the notes in my first paragraph.


Urllib3 has an exception with the same name, but it is not actually related to `requests.exceptions.HTTPError`. I [searched this repository](https://github.com/kennethreitz/requests/search?utf8=%E2%9C%93&q=httperror) (well, I used grep) for HTTPError and the only place I can see where requests itself uses it is with `raise_for_status`.

I had a discussion with @Lukasa briefly on IRC about this. As far as I can tell, `requests.exceptions.HTTPError` is only used for this purpose. However, if you think you'll use it for other purposes, another alternative is to have something like an `HTTPStatusCodeError` (a subclass of HTTPError) that is specifically for `raise_for_status`.


Okay then, that is, in fact, a remnant from running on urllib2, and your improved wording is welcome. Thanks for the due diligence :)


`HTTPStatusCodeError` would be a better name for this, but it is too late to change it now. Behavior was a bit different when this was originally crafted :)
Performance issues while working behind a proxy where no proxy is needed.

Hi, plz have a look at this [issue #116](https://github.com/bulkan/robotframework-requests/issues/116)  with [robotframework-requests](https://github.com/bulkan/robotframework-requests) library, take particular notice of my [second comment](https://github.com/bulkan/robotframework-requests/issues/116#issuecomment-224537034), it might be related to 'requests'.

Cheers
Tset


This is a known issue: it's a duplicate of #2988.
session ignores POST data when caching redirects

I was scraping a site where the search form makes a POST request which is redirected with 301 code. That code being permanent, the redirect is cached in session.redirect_cache by POST URL alone, so for all further searches the session gets the same page. I suggest that redirect_cache should use POST body as a key also, it's quite typical for sites to redirect POSTs to different URLs based on data.


Hey @panda-34,

Thanks for pointing this out. This was something I hadn't seen before (or considered). Tthe redirect cache is meant to very simplistic. The default cache can be disabled like as described [here](https://github.com/kennethreitz/requests/pull/2095#issuecomment-45977320). You can then use a real cache system, like [cachecontrol](/ionrock/cachecontrol) to do this kind of work.


POST requests aren't really supposed to be cacheable, redirects or not. Bypassing the POST and jumping immediately to the result (even with correct URL) might have some very nasty consequences.


Just on the topic of disabling the cache - Is it possible to disable the
cache for specific calls, or does it need to be done globally?

On Tue, Jun 7, 2016 at 6:51 AM panda-34 notifications@github.com wrote:

> POST requests aren't really supposed to be cacheable, redirects or not.
> Bypassing the POST and jumping immediately to the result (even with correct
> URL) might have some very nasty consequences.
> 
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> https://github.com/kennethreitz/requests/issues/3291#issuecomment-224084122,
> or mute the thread
> https://github.com/notifications/unsubscribe/AGk_7dwxUktz7CDneIGjxlnQ4Edgsty8ks5qJIhEgaJpZM4IvRXW
> .


Once again, I would vastly prefer that rather than us making the 301 cache increasingly more complex we simply _remove_ it. It frequently behaves incorrectly and leads to subtle confusion. I was in opposition to its merging back when it was proposed and I remain opposed to its function now.


@Lukasa I'm in agreement. Let's do that in 3.0


+1, I don't like it.
Test verify parameter


I've removed the `SSLError` tests, and session tests due to what you've explained in #3288, and focused on the `SubjectAltNameWarning` which should always occur with `pytest-httpbin`.

This is actually better at showing the problem in #3287 , as it needs to occur in all combinations of dependencies and Python versions.


This looks reasonable to me aside from my one note about warning spam. @sigmavirus24, what do you think?


This is a good start. I think we can mitigate the spam.


There are also other bugs in pytests handling of warnings (see https://github.com/pytest-dev/pytest/issues/840)
An alternative approach to address the warnings problem is to redirect them through the logging layer for the test suite, using `captureWarnings` and backports or some pytest extension which provides something similar.  py.test itself doesnt [appear](https://pytest.org/latest/recwarn.html) to have anything particularly fancy for managing warnings in the global test context.  There are pytest extensions for the logging layer.  Let me know if you want me to do a little analysis down that path.


Any objections to the warnings init moving into `tests.__init__` like so...
https://github.com/jayvdb/requests/blob/unbundling_walkthrough-v2/tests/__init__.py ? It makes the results more stable.


I'm ok with it.


Sorry for the delay; been recovering from hard disk loss


Ok I'm good with this, leaving it up to @sigmavirus24 to merge when he's happy.


@sigmavirus24 , ping ... ?


Sorry. Was distracted with other things. This looks fine to me.
Minor session object documentation issues

I suspect this is a documentation issue, for expected and desired behaviour, rather than an unintended bug...

http://docs.python-requests.org/en/master/user/advanced/#session-objects states that 

> The Session object ... will use urllib3's connection pooling. So if you're making several requests to the same host, the underlying TCP connection will be reused, which can result in a significant performance increase (see HTTP persistent connection).
> ...
> Note, however, that method-level parameters will not be persisted across requests, even if using a session. This example will only send the cookies with the first request, but not the second: ..."

There is a minor conflict between those, in that exceptions to the rule include the `verify` parameter and likely other connection related parameters (`cert`, `timeout`, ...?), which do in fact persist across the requests.  This is strongly implied by the opening paragraph, which says that the connection is reused.

Also "several requests to the same host" is not quite correct, I believe, as different protocols will result in different connections, naturally.


Note #3289 shows the `verify` parameter being persistent between requests in the same session.


What would be interesting to test is whether a connection reset of some sort causes the `verify` parameter in the session to the same protocol/host to be lost.  Maybe there is a test in `urllib3` that already covers that scenario.


This is a known issue (see #2863) and will be fixed in an upcoming release, having had the underlying urllib3 issue fixed (shazow/urllib3#830).
Load all unbundled urllib3 modules


I am _extremely_ nervous about applying further upstream patches to handle downstream unbundling behaviours. In particular, it's not clear to me what exact problem you're experiencing in #3287.

For the moment however I'm inclined to call this a Debian bug unless told otherwise. 


The problem is that pyopenssl injection doesnt work, despite having all of the necessary dependencies installed, and despite the pyopenssl injection function appearing to be successful (i.e. no `ImportError` is raised).  i.e. Debian's requests 2.10 doesnt have OpenSSL & SNI support, at least in the combinations I've tested using.

It looks like there is no tests covering this unbundling support code, so I'll set up unit tests to verify this isnt a Debian only problem.

fwiw, I have no objection to the unbundling support code being removed from `requests.packages.__init__`, but it is currently there, and doesn't work, but has lots of documentation in the module that gives packagers the false expectation that it works.


So pip does [something similar](https://github.com/pypa/pip/blob/master/pip/_vendor/__init__.py#L79) but in a far more intelligent way than what is happening here.

That said, I don't think we have evidence that what pip is doing works either.

I also don't agree with your all or nothing mentality @jayvdb. It's not productive.


Hello,
sorry for the late reply, I was on trip and once returned I had to work on the backport of betamax for the stable release due the sheduled upload on OpenStack (I don't remember which one).

I definitively agree with @Lukasa here: the problem is on the Debian side.
So, as soon I complete the backport of betamax I will work on this.
This is my plan:
1. use the same pip's `vendored` function to patch requests.packages.**init**;
2. add some tests on Debian CI infrastructure over this specific issue;
3. upload this new revision to experimental suite;
4. make a call for test;

@jayvdb can you share your tests about OpenSSL & SNI support on Debian? Thanks!


@jayvdb never mind, you were talking about https://github.com/kennethreitz/requests/issues/3287.


I agree with @Lukasa — this part of the codebase is absolutely abhorrent and should not even exist — however, we have chosen to do so to improve the user experience of our very unfortunate distro-installed users. 

We've already done more than I think we should have. Doing even more, without extremely strong beyond-a-reasonable-doubt purpose, is out of the question.
Try to allow the socks patch to use http_no_tunnel

re #3238


Thanks for this. Unfortunately, we can't merge this patch: it is entirely in urllib3 (a third-party library that we don't merge patches for). It's also not correct: it uses a constant that doesn't exist and handles a weirdly scoped URL rather than looking for environment variables. 


Thank you. I'll see if I can find another way to add this functionality.
Setting Proxy-Authorization Header Manually

I know you guys have addressed a variety of concerns relating to configuring proxies and that proxying is functionally complex, but I'd like to raise one issue I was having today. We have a new proxy in our environment that requires auth with a yubikey rather than a password. We can simply set an environment variable to a token returned from a yubikey auth endpoint that expires every 12 hours and pass it along with the Proxy-Authorization header in tools like curl, wget, etc... However, it gets a little trickier with requests, as the default api seems to only support including the proxy-user:proxy-pass in the proxy-url.

In our case, we can't do this, as a yubikey token is only good once. As a temporary workaround, I've subclassed HTTPAdapter and modified the proxy_headers method:

```
class CustomHTTPAdapter(HTTPAdapter):
    def proxy_headers(self, proxy):
        return {
            'Proxy-Authorization': 'Basic ' + environ.get('proxy_token', str())
        }  
```

and then mount it on a Session:

```
import requests

from mypkgs.adapters import CustomHTTPAdapter


s = requests.Session()
s.mount('https://', CustomHTTPAdapter())
s.get('https://someurl')
```

It would be incredibly useful to simply be able to do something like `requests.get(uri, proxy_headers=proxy_headers)`. Am I barking up the wrong tree here or do you have any ideas or feedback?


Generally speaking we're disinclined to add a separate `proxy_headers` kwarg. The vast majority of users don't need it, and those that do are relatively well-served by the `proxy_headers` override of the HTTPAdapter.

Requests is extremely reluctant to add new kwargs to the top-level API if it can possibly be avoided, and in this case I think it can.

Sorry! =(


Thanks for the feedback! In that case, is there an easy way to mount a custom HTTPAdapter on the main requests api rather than on a session? I didn't note any in the documentation, but maybe I missed it. 


Nope: the requests API manufactures a new Session on each request to avoid having any implicit global state that can lead to all kinds of weird bugs.