Raibek/requests-github-issues huggingface dataset:
Response as iterator

Either: 

``` python
r = requests.get('http://bigsite.com/')

for line in r:
    # Do stuff.
```

or:

``` python
r = requests.get('http://bigsite.com/')

for chunk in r:
    # Do stuff.
```

See #1141.


This breeds ambiguity I think. On the one hand iterating over the lines would work in the above example, but how should the user expect the following to behave?

``` python
r = requests.get('https://s3.amazon.com/big_download.tar.gz', stream=True)

for data in r:
    # Do stuff.
```

For this reason alone I'm partial to using `iter_content`, with a reasonable chunk size if this is implemented at all. (And this would be extraordinarily easy to do.)


I'm happy to use `iter_content` instead of `iter_lines`. =)


``` python
import requests

r = requests.get('http://www.google.com/', stream=True)

for chunk in r:
    print chunk
```

All done, and we even have a test. =)


Ugh, Python 3 changed the behaviour of generators. Need to have a think about this.


Ah yeah, that's what I was thinking of. `next(iter(r))`


Already on it. =D


Ugh, same test failures as in #1119. Totally convinced this isn't my fault.


I'm almost tempted to just mock it with a fake response so that it always passes. I'm going to see if I can reproduce it here though.


OH DUH! I know why that test is failing. The API hits it's limit from the Travis IP addresses so quickly and bizarrely that you're getting a 411 (or I forget which they use to signify you're over the rate-limit). It's why the errors don't reproduce locally. I'll send a patch with the mocked response.

Otherwise :+1: on this PR

_Edit_ And by "DUH" I meant, "I should have realized this sooner". I have seen this far too many times and I never realize it as quickly as I should.


I don't need the patch, the test is already in the repo. May as well just open a PR with the fix. =)


Yeah, I pushed to master instead of to this pull request branch. 


I give up trying to push to here. I'll just open the PR from my fork.


Thanks @sigmavirus24!


let me know when i was ready to merge


@kennethreitz: Rebased. =)


:cake:
Remove default Content-Length from GET requests.

Fix #1051.


Great will fix


We need PATCH in that tuple as well.


Let's do the opposite:

if not self.method in ('GET', 'HEAD')


Yeah, I should have done that from the start :).


Does anyone know if I merge these locally and force push, will the pull request on Github figure it out? Most of these commits I feel like I should have just put in a single commit. Normally this is a terrible idea, but the Push Request model seems to work well for this. Otherwise, I'll just push up another commit.


nah it doesn't matter, just add a new commit :)


Cool :)


> If I merge these locally and force push, will the pull request still work 
> out. Most of these commits I feel like I should have just put in a single 
> commit. Otherwise, I'll just push up another one.

Squashing should work just fine. :)


Ah, @kennethreitz beat you to it. MOAR COMMITS!


:sparkles: :cake: :sparkles:
[RFC] Response as a string

In some cases it could be useful, that response would act as a string:

```
>>> response = requests.get('https://api.thing.somewhere/size/blah', auth=('user', 'pass'))
>>> response.status_code
200
>>> "Blah is %d big" % response
Blah is 3242 big
>>> response.headers['content-type']
'text/plain'
```

is it contrary to pythonic ideas?


It's not un-pythonic, but I would argue it violates the principle of least surprise. I would not say that a Response's string representation is its Content Length.


yeah, i don't think we want to go this route.

A better question is should responses be iterable?


I will say yes if you can tell me what word goes in place of 'X' in the following code:

``` python
for X in response:
    # Do stuff
```


There are plenty of times where we can not pre-determine the length of a 
response so your example only works half the time. (The issue of getting the 
length of a response has been discussed ad nauseum as well.)


@Lukasa line or chunk


Ooh, I like that. I'm +1 on it.


So let's close this issue (or rename it) and open one for using the response object as an iterator?
Test for cookie parameters.

I wasn't sure if setting some parameters on cookies was working or not, so I wrote a unit test for it.

The test passes; turns out I was just using it wrong in my other module.


:+1:


The test failure lies in unittest in python 2.6 not having the `assertIn` method. I'll work from @ralphbean's pr and add a work around tomorrow.


Oh.. I can push a fix commit.  One moment.


There.  :)


:+1:

Looks good to me.


yay!
HTTPDigestAuth et al don't preserve stream, proxies etc.

`HTTPDigestAuth.handle_401()` replaces the original response with another by doing `r.connection.send(r.request)`. This ignores the arguments originally passed to `Session.request()` such as `stream`, `proxies`, `timeout`, `verify` and `cert`.

`HTTPDigestAuth` (and other hooks that behave similarly) need a way to perform additional requests in the same context as the original. This could be done by having `request.Session` set the relevant instance attributes in `__init__()` and then doing `resp.session = self` after `self.send()`.

It's possible that `requests.request()` et al should also pass those arguments to the `Session.__init__()`, not `Session.request()`.


Hmm, I don't think this is the right approach currently. We'll have to reevaluate soon.
Time each request

Pretty simple - adds calls to time.time() around send() and a new attribute, Response.time_taken, to tell how long the last request took.


I like this, it's cool for simple metrics (needed something like this sometime ago). But wouldn't be better to put the result in the returned `Response` object?

That way we can use it like the code below and keep track of the request time for every request made.

``` python
import requests
response = requests.get('https://google.com')
print response.elapsed()
```

Also, `elapsed` is the best name for this? Property vs method? (I'm not familiar with requests internal code standards).

:+1: 


This has been requested/proposed a few times before and shot down each time. As it is, a method would not be the preferable way of doing this. You would simply time it either on the adapter or session level, and set an attribute named `elapsed` as the value. There's no need to store two separate attributes and recalculate it each time. It isn't going to change since the request has been completed. (Remember: methods have overhead, attributes don't.)

Also, this is inaccurate because streaming requests don't technically complete when the response object is returned, so there is no accurate way of timing those responses since you're ostensibly doing something with each chunk you read which would affect the timing.


@sigmavirus24 Your point about streaming responses is correct; I'm not sure that a simple 'end - start' timer is useful for streaming responses _anyway_, but it IS useful for everything else (which is what I need).

As far as whether it's better as a method or a property - I honestly don't care. It's such a small change that if it gets rewritten to be a single property instead of a method, hooray! All I want is the ability to _get_ this information, from requests, without having to patch it myself every time I use it. @kennethreitz didn't seem to be convinced that this functionality even _belongs_ in requests, so I'm hoping that even if this particular commit doesn't get accepted, the general idea is considered to be worthwhile and some kind of implementation makes its way in.


Btw, @kennethreitz, as we discussed on IRC, :+1:. :shipit: 


I hope this gets added in


+1, this would be super useful


+1 for utility


get rid of start and finish and i'll merge :)


Eh, I'll update it.


gah, need to rebase


@kennethreitz which start and finish are you talking about? The locals in the adapters file?
introduce a test for link headers.

This test is kinda bad, submitting for feedback, fixup, and eventual inclusion.

There's currently no test to to check that the `Link` header parsing actually works, so I wrote one.

It pings GitHub, so that's bad, obviously.

You mentioned on IRC that you could use `PreparedRequest`; should that be what I'm doing here?

:heart: :sparkling_heart: :heart:


Hmm, we need a mocked Response connection adapter.

I'm on it!


Also, I wanted to use httpbin, but it doesn't have a way to return them. Would that be useful to add to it?


Well, we're going to remove it completely. Give me a few, I'll give you a mock response which will be awesome.


:metal:
remove silly assert

C'mon bro. :wink:


one must question everything


:metal:

It's true, you never know, someone may have monkey-patched `assert` to always return true!

Oh wait, this isn't Ruby. :wink: 

Hopefully actually meaningful commits coming soon.


DON'T DO IT :-1: 


```
>>> True = False
```


```
  File "<stdin>", line 1
SyntaxError: assignment to keyword
```


Python 3?
`timeout` should be settable on session

It would be nice to have a default timeout for all requests within a session. Currently, this isn't possible because timeout is only taken as an argument to `Session.request`. As a workaround, I am using something similar to the following:

```
class TimeoutSession(requests.Session):
    def __init__(self, timeout=None):
        self.timeout = timeout

    def request(self, *args, **kwargs):
        kwargs.setdefault('timeout', self.timeout)
        return super(TimeoutSession, self).request(*args, **kwargs)
```

This allows the timeout to be overridden on individual requests, but uses the session timeout if its not specified.


Timeout is a parameter of the act of sending a request now.

Basically, session-level timeouts down't make sense when we have connection adapters.


What is the suggested method for setting the same timeout for a set of requests? Do you suggest we override the Adapter?


Also, do you mind letting this issue breath a little before immediately closing it?


Correct.

Normally, I'd let it breathe for a while, but this was an explicit decision I made recently. Timeouts used to to be configurable.


Thanks, Kenneth.

Keep up the awesome work!
New download() method on Response model for saving response content to local file...

...without reading whole response content into machine memory.

Includes 1 new test case.


Firstly, thanks for the pull request! It's always great to have contributions. =)

Secondly, and being up-front, I don't think this is likely to get accepted into the library. _Do not close this PR based on that comment_ (some people have done in the past), that decision isn't mine, but I suspect this PR won't get accepted. I think this for two reasons:
1. It adds to the API, which is generally not what we're looking to do.
2. It isn't hard for the end user to do themselves.

To show an example of point 2:

``` python
import requests
r = requests.get('http://google.com', stream=True)

with open('/tmp/data.html') as f:
    for chunk in r.iter_content(chunk_size=4096):
        f.write(chunk)
```

Point 2 doesn't matter much if it's a very common use case (see the `Response.json()` method), but I don't particularly think this is. If it only takes 3 lines of code for the user to do it themselves, it's rare that we'll add to the complexity of the library to do it. =)


Well said, @Lukasa. 

Thanks for the pull request, but I won't be accepting this at this time. You're not the first person to send this, actually :)


OK, I'm gonna try fixing some open bugs instead.


:+1: go get 'em @alanhamlett
AttributeError when // is missing (eg. 'http:www.guardian.co.uk')

Requests fail with an AttributeError instead of a LocationParseError when the url misses the '//'. Also LocationParseError maybe should be a top level exception of Requests, not buried inside urllib3.

the complete error:
AttributeError: 'NoneType' object has no attribute 'encode'


This is a better traceback:

```
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python2.6/site-packages/requests/api.py", line 55, in g
    return request('get', url, **kwargs)
  File "/usr/lib64/python2.6/site-packages/requests/api.py", line 44, in r
    return session.request(method=method, url=url, **kwargs)
  File "/usr/lib64/python2.6/site-packages/requests/sessions.py", line 279equest
    resp = self.send(prep, stream=stream, timeout=timeout, verify=verify, ert, proxies=proxies)
  File "/usr/lib64/python2.6/site-packages/requests/sessions.py", line 374end
    r = adapter.send(request, **kwargs)
  File "/usr/lib64/python2.6/site-packages/requests/adapters.py", line 174end
    timeout=timeout
  File "/usr/lib64/python2.6/site-packages/requests/packages/urllib3/conneool.py", line 422, in urlopen
    body=body, headers=headers)
  File "/usr/lib64/python2.6/site-packages/requests/packages/urllib3/conneool.py", line 274, in _make_request
    conn.request(method, url, **httplib_request_kw)
  File "/usr/lib64/python2.6/httplib.py", line 914, in request
    self._send_request(method, url, body, headers)
  File "/usr/lib64/python2.6/httplib.py", line 931, in _send_request
    self.putrequest(method, url, **skips)
  File "/usr/lib64/python2.6/httplib.py", line 858, in putrequest
    host_enc = self.host.encode("ascii")
AttributeError: 'NoneType' object has no attribute 'encode'
```

The reason for this is that `urlparse` detects the scheme but not the `netloc`. I'm pretty sure this could easily be fixed by checking `if not (scheme and netloc)` the line after the url is parsed.


Resolved by #1127. Thanks for raising the issue! =)
HTTPDigestAuth: missing algorithm field

While I was testing a digest auth provider I found that the algorithm field was missing at the Authorization Request Header.

Following the [RFC](http://pretty-rfc.herokuapp.com/RFC2617#the.authorization.request.header):

> The values of the opaque and algorithm fields must be those supplied in the WWW-Authenticate response header...


@sigmavirus24, all good?


Seems it to me. I'm convinced it does what he says.

Kenneth Reitz notifications@github.com wrote:

> @sigmavirus24, all good?
> 
> ---
> 
> Reply to this email directly or view it on GitHub:
> https://github.com/kennethreitz/requests/pull/1125#issuecomment-12579122


:cake:
Correct type of Content-Length headers.

This should resolve #1113. A test is included, but the test relies on StringIO - if you want to avoid that import, I can remove the test.

As a side note, the type of the Content-Length header is now always unicode, which feels pretty weird. Should I add in a call to `encode('ascii')`?


Updated this because the test doesn't (and can't) run on Python3. Stupid new StringIO.


@Lukasa just use the `io` module. It has `StringIO` in it in python 2.6-3.3


@sigmavirus24:

``` python
Python 3.3.0 (default, Oct 20 2012, 09:48:02) 
[GCC 4.2.1 Compatible Apple Clang 4.0 ((tags/Apple/clang-421.0.60))] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from io import StringIO
>>> a = StringIO('abc')
>>> a.fileno()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
io.UnsupportedOperation: fileno
```

The problem isn't in my code, it's that stupid StringIO _does_ have the method, but unconditionally throws exceptions. Given that Requests tries to use it, that's a problem. =P


Should have read the test I guess :P. Kind of distracted by trying to fix my 
app on Heroku.


thanks!


@Lukasa 

As to type of this header, I agree, it should not be unicode. More generally no header should be unicode. Actually, thinking of it, no header in prepared request should be unicode; there could be some value in leaving unicode in unprepared request.


also keep python3 in mind


@kennethreitz Yeah, I was wondering about that. It seems like headers on the `Request` object should be free to be either unicode or bytes. With that said, somewhere before urllib3 we should get rid of any unicode headers, in order to avoid problems like #1082. Maybe `prepare_headers` should turn them into bytes?
Type of Content-Length header changes when type of data changes

```
import requests
import StringIO

req1 = requests.Request(url='http://example.com', data=StringIO.StringIO('abc')).prepare()
req2 = requests.Request(url='http://example.com', data='abc').prepare()
assert type(req1.headers['content-length']) == type(req2.headers['content-length'])
```


This is a feature not a bug.

http://docs.python-requests.org/en/latest/user/advanced/#chunk-encoded-requests


I don't think using different transfer encoding should change **Python's type** of some header's value.


Ah. It should always be a string.


Resolved by #1116.
Keep-alive doesn't work over http proxy (again)

Proposed solution:

``` diff
diff --git a/requests/adapters.py b/requests/adapters.py
index 5f9d9c7..099a001 100644
--- a/requests/adapters.py
+++ b/requests/adapters.py
@@ -11,7 +11,7 @@ and maintain connections.
 import socket

 from .models import Response
-from .packages.urllib3.poolmanager import PoolManager, proxy_from_url
+from .packages.urllib3.poolmanager import PoolManager, ProxyManager
 from .packages.urllib3.response import HTTPResponse
 from .hooks import dispatch_hook
 from .compat import urlparse, basestring, urldefrag
@@ -120,7 +120,7 @@ class HTTPAdapter(BaseAdapter):

         if proxy:
             proxy = prepend_scheme_if_needed(proxy, urlparse(url).scheme)
-            conn = proxy_from_url(proxy)
+            conn = ProxyManager(self.poolmanager.connection_from_url(proxy))
         else:
             conn = self.poolmanager.connection_from_url(url)
```

BTW I once implemented this. How could it disappear? What happened with the tests covering this feature?


the entire codebase was re-written. send a pull request.
Licensing questions

The openSUSE legal team stumbled upon some licensing questions: https://bugzilla.novell.com/show_bug.cgi?id=799119

Distributions have to be exact here, so maybe can comment and clarify the situation.


I am not going to create an account.

Can you share a screenshot or transcript or something?


Uhm, damn legal bugs aren't public, sorry... Here's the report:

The NOTICE file shows that the package uses various components under MIT,
BSD-3-Clause and LGPL-2.1+. The LICENSE file states:

Copyright 2013 Kenneth Reitz

   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
   You may obtain a copy of the License at

```
   http://www.apache.org/licenses/LICENSE-2.0
```

   Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.

The Apache-2.0 license seems to be confirmed by the setup.py file.

However, our spec file states ISC. The Fedora spec file also states MIT and ISC

Please confirm...


The project was previously released under MIT. It has since changed Apache 2. 

See NOTICES file for clarification for embedded Python modules.
add file scheme support

Adds file://... support.  It's unnaturally complete for what it does.
- methods for GET / HEAD / PUT / POST / DELETE
- turns symlinks into redirects
- appropriate HTTP errors for access-denied, missing file, etc.
- streaming support for large files
- guesses content-types / encodings from filenames
- date, last-modified, content-length, and other headers

Since this is potentially dangerous, it's not enabled by default.  A few
rudimentary tests are provided.

For those who are wondering why anyone would want this, I use it for
bootstrapping some of our HTTP clients (i.e. I have a list of URLs for
fall-back, with a file://... URL at the end) and of testing (i.e. it's easier
to just change the API base URI to file: than it is to mock out requests
everywhere).


First off, thanks for the contribution. Very bold :)

That being said: I will never accept this. This is one of the biggest conceptual flaws in urllib2. This also adds a tremendous amount of code to the codebase, which should aways be avoided if possible.

However, this is awesome, and exactly the type of thing that I was hoping people would make with Connection Adapters. It just doesn't belong in Requests itself, but another module.


Perhaps 'requests-fs' or something ;)


But... but... I disabled it by default!  And it doesn't touch any core code at all!  And I wrote tests!

![Okay...](http://alltheragefaces.com/img/faces/png/okay-okay.png)


pip install requests-fs. it'll be awesome :)


@kennethreitz : I just came across this while looking for a way to do this.  Can you explain why allowing requests to open file scheme URLs would be a "conceptual flaw"?


That's easy: because requests is a HTTP library!

Requests is not about opening arbitrary URLs, it's about making HTTP requests. File URLs don't involve HTTP at all. They are simply out of scope for the library. Adding support for them adds complexity and mess to a codebase that simply doesn't need it. 


They are also a big security risk for people that expecting to be able to trust that this is an HTTP library :)

Many thanks, @Lukasa :)
Correct broken cookies example

The cookies example in the docs is wrong. This fixes it so it demonstrates correct behaviour, though it is no longer something you can type into your own shell to see. If we need that functionality, we might need to adjust httpbin's behaviour. Let me know if you'd rather do that.

Addresses #1069 (a whole month later).


What change in httpbin do you envision?


Have httpbin return the cookies on the 200 after the redirect.
Result of 'response' hook is stored in session.response and not returned

https://github.com/kennethreitz/requests/blob/master/requests/sessions.py#L298

Should the result of `dispatch_hook` actually be returned? Seems to make no sense to store it as a property of the session object.

The documentation states:

> "If the callback function returns a value, it is assumed that it is to replace the data that was passed in. If the function doesn’t return anything, nothing else is effected."

Another thing I've noticed, and don't quite understand, is that [hooks are executed by the adapter as well](https://github.com/kennethreitz/requests/blob/master/requests/adapters.py#L113), and that hooks are not preserved when the `PreparedRequest` is being generated.


This pull request is very relevant: https://github.com/kennethreitz/requests/pull/1099


I'll fix this in a bit.
refresh CA certificates

In order to resolve the TURKTRUST compromise, and to catch any other
issues that may have fallen through the cracks, I'm changing
the certificate bundle to match the /etc/pki/tls/certs/ca-bundle.crt
on my system, from Fedora 18's package ca-certificates-2012.87-1.fc18.noarch.

See #1102 for some discussion.

As per `yum info`, the certificate file is public-domain:

$ sudo yum info ca-certificates
Loaded plugins: changelog, langpacks, presto, refresh-packagekit
Installed Packages
Name        : ca-certificates
Arch        : noarch
Version     : 2012.87
Release     : 1.fc18
Size        : 1.6 M
Repo        : installed
From repo   : updates
Summary     : The Mozilla CA root certificate bundle
URL         : http://www.mozilla.org/
License     : Public Domain
Description : This package contains the set of CA certificates chosen by the
            : Mozilla Foundation for use with the Internet PKI.


The file appears to have grown from 210K to 697K :-\


I'd prefer to stick with our existing CA bundle.


Cool, if you're sure we have all the relevant revocations covered. This diff just removes the TURKTRUST roots.

Does certifi require an analogous pull request? Is certifi being end-of-lifed now that it's vendored here?


Yeah, I'll move it to my archive account. Thanks for this!


https://github.com/kennethreitz-archive/certifi
Support for custom http pool sizes removed

The following worked previously

config = {'pool_maxsize':1000}
session = requests.session(config=config)

Obviously the interface has changed slightly. However there is no way to modify the default mounted http adapter to have a higher connection pool size. This should be possible because a limit of 10 is extremely small if you are using it in a proxy connecting to a single server.

If there is a preferred way to do this I am open to suggestions


``` python
from requests import HTTPAdapter

adapter = HTTPAdapter(max_poolsize=1000)

s = session()
s.mount("http://", adapter)
s.mount("https://", adapter)
```


What I did was

```
session = requests.Session()
session.mount('http://', requests.adapters.HTTPAdapter(pool_connections=1000, pool_maxsize=1000))
session.mount('https://', requests.adapters.HTTPAdapter(pool_connections=1000, pool_maxsize=1000))
```


@c00w if you fixed it, close this issue please?
Restore the ability to use a list of 2-tuples for the data keyword argument with requests.post.

As of [commit ef8563a](https://github.com/kennethreitz/requests/commit/ef8563ab36c6b52834ee9c35f6f75a424cd9ceef) the ability to pass a keyword argument like `data=[('key', 'value'), ('key', 'value2')]` was broken because the `is_stream` detection in `PreparedRequest.prepare_body(self, data, files)` fails to look for a list of 2-tuples.

Just in case this is deemed to be an unnecessary change (because the calling code could switch to using a dict), Il'l make note here that this is required in order to pass POST parameters where the key is the same, but values are different. 

```
form_data = [('item', 1), ('item', 2), ('item', 3)]
resp = requests.post('http://www.example.com', data=form_data)
```


Actually, the proper syntax for that is slightly different.

``` python
form_data = {'item': [1, 2, 3]}
resp = requests.post('http://www.example.com', data=form_data)
```


I see the syntactical niceness of that example for simple cases(thanks for showing it, I didn't realize I could do that), but I'd still like to make my case for the way that it worked previously.

I'm currently using Werkzeug `OrderedMultiDict` to hold the parameters that eventually make it into the `data` argument.  Some of my uses can have quite a few parameters and, since it's ordered, the order of the parameters does matter in the POST data (working with a legacy system).

WIth a list of 2-tuples I can do: `data=ordered_multi_dict_instance.items(multi=true)`. So the 2-tuple is quite easy to create from a dict-like object that iterates.  I may be wrong, but this new syntax deviates in such a way that I'd have to marshall all the data into it and maintain the order at the same time. 

I also face an issue with the fact that this legacy system I'm working with requires not just multiple keys but ordered multiple keys.  Something like: `Item=1&Value=10&OtherValue=100&Item=2&Value=200&OtherValue=200&Item=3&Value=30&OtherValue=300`.  Presumably, that would result in `form_data = {'Item': [1, 2, 3], 'Value': [10, 20, 30], 'OtherValue': [100, 200, 300]}` in the new syntax, which would likely re-order the parameters based on the names of Item, Value and OtherValue, and not in the repeating ordered sequence: Item, Value, OtherValue.


Related: issue #522 


We need to support MultiDict. This is long overdue.


Accepting MultiDict for the `data=` keyword would be acceptable for me.


So, moving this forward.  

As it stands [commit ef8563a](https://github.com/kennethreitz/requests/commit/ef8563ab36c6b52834ee9c35f6f75a424cd9ceef) is a breaking change from 1.0X to 1.10 for my particular use case.  The workaround I have is:

```
form_data = RequestEncodingMixin._encode_params(request.values.items(multi=True))
resp = requests.post('http://www.example.com', data=form_data)
```

Is this particular pull request viable as a stop-gap between now and MultiDict support or is passing a 2-tuple list into `data=` a no-go for the future?


Merged.
address compromised TURKTRUST certificates

It seems likely that `requests/cacerts.pem` is affected by the recent compromise of TURKTRUST certificates. However, I'm not sure what exactly we need to do to fix this. Removing both of the root certs we currently have in there is probably wrong.

Maybe @saschpe can advise?


Hmm, I tried to follow the instructions here: [https://gist.github.com/996292]

but the URL given there for certdata.txt points to a file dated December 29, 2012, and it doesn't change the status of any TURKTRUST certificates. I'll ask around.


Hmm, OK. The December 29th file ([https://mxr.mozilla.org/mozilla/source/security/nss/lib/ckfw/builtins/certdata.txt?raw=1] ) has the changes we need at the very end, but they take the form of blacklisting the affected intermediate certificates rather than removing the root certificates. The process of converting this to a .pem seems to lose this information.


yeah, i always remove them by hand. Care to send a PR with the CA removed?


My current understanding is that Python's ssl module lacks support for blacklisting intermediate certificates, it can only whitelist roots. Thus the file I'm submitting in the pull request removes the TURKTRUST roots entirely.


Please note that most browsers still trust the TURKTRUST root CA, so this patch will cause some mismatch to what browsers do.
Mozilla, for example, has only blacklisted the intermediate certificates that TURKTRUST wrongly issued. See https://blog.mozilla.org/security/2013/01/03/revoking-trust-in-two-turktrust-certficates/


Agreed, this is suboptimal. We're doing this because Python's ssl module doesn't seem to support this kind of sophisticated blacklisting behavior.

Python's ssl module is weird; there's a baseline of documented functionality, and then some platforms appear to have undocumented extensions. See #629 for an issue of this kind.
Session object no longer pickleable

After upgrading to 1.1.0, I now get this error when trying to pickle the Session object:

File "/Library/Python/2.6/site-packages/requests/sessions.py", line 397, in **getstate**
    return dict((attr, getattr(self, attr, None)) for attr in self.**attrs**)
AttributeError: 'Session' object has no attribute '**attrs**'


Duplicate of issue #1088 


Quite right. =) Let's move all discussion to there.
Call hooks when following redirects

See https://github.com/kennethreitz/requests/pull/965 . However, this no longer appears to be working in Requests 1.1. This pull request is against 1.1.


This should be working now after #1099. Can you test from the repo and confirm @jimothyGator?


Confirmed:

``` python
import requests

def print_respone(resp):
    print resp.status_code, resp.url

hooks = dict(response=print_respone)

requests.get('http://httpbin.org/redirect/3', hooks=hooks)
```

Output:

```
302 http://httpbin.org/redirect/3
302 http://httpbin.org/redirect/2
302 http://httpbin.org/redirect/1
200 http://httpbin.org/get
```


Awesome. Thanks for your fix as well.
Make hooks work with prepared requests

:heart: 


:+1: This is a prerequisite for #1117 (or co-requisite)


@kennethreitz this is definitely a pull request you should merge. It complements #1117 which you already merged.


@sprt can you update this? It's a bit out of date now :)


Unfortunately, this isn't going to work because we made incompatible changes as I relied on the fact that the adapter would call `dispatch_hook()`.

I'm not sure how to handle this; when using sessions `Session.send()` should call `dispatch_hook()` and when using connection adapters directly, `BaseAdapter.send()` should.


@sprt, I'll work on this.


@kennethreitz, @sprt makes a valid point. My concern is dispatching a hook more than once. Someone using requests in @sprt's case has a valid reason to believe the hook should be called which makes it the responsibility of either the adapter's `send` or the session's `send` method. The former also catches the case where a user just instantiates the adapter and uses that directly (which is a bad idea). The latter would allow the hook to be dispatched once, and would remove it from the `Session.request` and `<Adapter>.send`. This I see as being the preferable option, but I wanted to make sure this is okay.

The problem, of course, is that only `request` has the hooks passed by the user. Naturally, this shouldn't be a problem because the prepared request has those hooks and the auth hooks generated by the authentication handler, so we can just rely on that. No where in between are hooks removed. A rogue adapter may do this in its `send` method, but that would be author's problem, not ours.


The changes are in [sigmavirus24/requests@pr/1099](https://github.com/sigmavirus24/requests/tree/pr/1099)


@sprt those changes pass with your tests. If you like them merge them into your branch, they'll be added to this pull and @kennethreitz could accept it.


Done, sorry for the FF

What about adapters though? 


Sorry, I meant merge it from the branch I specified above (`git pull git://github.com/sigmavirus24/requests pr/1099`).

My patch places the dispatch after the request is returned from the adapter in `Session.send`. This satisfies your test and regular hooks as well.


Merged!


Ah, I missed the merge. Disregard my comment above.


@sigmavirus24 was this not ready? send another pr :)


So what's the consensus on instantiating the adapter and using that directly?  Because this is still broken.  Should hooks be dispatched in the adapter's `send()` method too or should users be told to use sessions?


Users _should_ uses sessions. If they want custom adapters they should mount 
them on the session.

If the user wants to use Adapters directly, and they're writing their own, 
they can use `dispatch_hook` themselves.


Ah indeed, forgot about mounting!

Then I suggest @kennethreitz edit [his blog post](http://www.kennethreitz.com/announcing-requests-v100.html) (cf. "Connection Adapters") :)
"timeout" not a Session option anymore?

I checked the document and sessions.py in source repo, found it is specifically not merged into Request object. Is this a deliberate left out?


this is an argument used when sending a request. Not a connection-wide timeout.


OK, I could understand the principal. But the thing is, I used "timeout", "cookies" and "config" parameter a lot in my current code, with these important parameters suddenly deprecated, what suggestions do you have for situations like me? Thanks.
OAuth 1 requests with BODY signature_type send no body data and set Content-Length: 0 

Expected: OAuth params in the body and corresponding non-zero Content-Length header.

Actual: No body in the request and Content-Length: 0.

Notes:
1. As of kennethreitz/requests@8f86ff633a2646442f2d8976046b5c0025e772c1 (2012-11-28) the Content-Length was computed after applying auth. In 34917618d80580fafec98641b213c453d7f3fb39 (2013-01-11), `prepare_body` computes Content-Length before `prepare_auth` computes the OAuth parameters.
2. requests/requests-oauthlib@ba2ea5938dd4da30c963158a33f1a83c3e7d362c assigns the body data to a `data` attribute, but kennethreitz/requests@8f86ff633a2646442f2d8976046b5c0025e772c1 expects it in a `body` attribute.


Here's the requests-oauthlib portion of the fix: https://github.com/requests/requests-oauthlib/pull/13
Add support for generators as a body of the request.

The code assumes that we have only file like objects or strings as body of the request.
With this change also generators can be set as a body of the request.


It's already done - see issue #952. As there's no info which commit it was done in you'll have to find this yourself.


If I had to guess, I'd say that it was ef8563a. Just a guess. :wink:

For what it's worth, this PR would have needed updating anyway. If you're not supplying the `Content-Length` header you really need to supply `Transfer Encoding`, which this PR does not.

Thanks for the contribution though! :smile:
Add a simple test for #1092

Also, some minor PEP 8 fixes.


these aren't pep8 fixes :)


huh? why not?

``` shell
~/requests (master) > pep8 --version
1.4.1
~/requests (master) > pep8 test_requests.py  
test_requests.py:21:1: E302 expected 2 blank lines, found 1
...
test_requests.py:64:5: E303 too many blank lines (2)
...
test_requests.py:71:13: E128 continuation line under-indented for visual indent
...
```


I disagree with '" E128 continuation line under-indented for visual indent", and will defend it until the day i die :)


haha, fair enough :)


There are ways around it (spoken as a pep8 contributor and flake8 maintainer).
Make merge_kwargs case-insensitive when looking up keys.

This GET request spuriously returns a 400

``` python
>>> import requests
>>> url = "http://www.stackoverflow.com"
>>> headers ={"User-agent": "Mozilla/5.0"}
>>> response = requests.get(url, headers=headers)
>>> print response.status_code
400
```

This is because ever since a776e7cb3e57e56a2d9974b730e6f5e7776751a4,

``` python
headers = merge_kwargs(headers, self.headers)
```

causes the LHS to have two keys for user-agent when the user-supplied version is incorrectly capitalized:

``` python
>>> headers.keys()
['Accept-Encoding', 'Accept', 'User-Agent', 'User-agent']
```

A solution is to, instead of using `dict.update`, iterate through each key and update manually using a case-insensitive key lookup. This feels kinda hacky, so please let me know if there is a cleaner way to fix this.


@kennethreitz Are you closing/reopening in order to restart the Travis build?


Yes :(


Passes! :cake: Thanks :)


:tada:
Session pickling support is broken and tests for it are removed

The commit 42b029552190f6639642d0f62d27abcd1ceed51e removes the `__attrs__` attribute of the `Session` class, which is used in the pickle protocol's `__getstate__` method.

The tests that are testing this functionality (functions `test_session_pickling` and `test_unpickled_session_requests` in the once present `tests/test_requests.py`) are also removed.

The commit messages don't seem to indicate any reason for this, and I can't find anything searching in the issues.

If it is intended that pickling of Session objects not be supported, could you give the reason? And may be the `__getstate__` and `__setstate__` methods should be removed too, as they might send a wrong message.

If this is unintended (which is what I think is the case), I can work on a pull request to fix this. Please confirm.

Thank you.


I am also getting a `AttributeError: 'Session' object has no attribute '__attrs__'` traceback when trying to pickle Session objects after upgrading to requests ver 1.0.*.

Would love to have this functionality back.


In general, making `Session` objects pickle-able is good. Bear in mind that you will likely need to be able to pickle the transport adapters too.


Forgot that this was taken care of.
Encodings from content

Requests has a `get_encodings_from_content()` function, but it doesn't seem to be used anywhere -- only `get_encoding_from_headers()` is used.

Any reason why? I'd think on most pages, trying for the meta tag encoding declaration first will produce better results. See The Verge, for one example (the meta tag declares encoding as utf-8, but Requests detects as ISO-8895-1).


At first I thought this was an issue with [charade](/sigmavirus24/charade), but it seems you're right that this is taking place in requests proper. From what I can tell, The Verge might not be setting the encoding in a way requests expects to see it. Could you post the headers from your response for better debugging? (Alternatively, post the URL you used.) For the sake of satiating my curiousity, could you also use: `apparent_encoding` and see what that returns? (It is an attribute, not a method)


Related #156, especially [this](https://github.com/kennethreitz/requests/issues/156#issuecomment-4856384) comment.


@sigmavirus24 I noticed the issue with the title of this article:

http://www.theverge.com/2013/1/4/3836944/robot-band-compressorhead-plays-motorhead-ace-of-spades

a few days ago, it was reporting an encoding of `iso-8859-1`. I don't see the issue now, however. It would be a little weird if they just happened to fix the issue since I last looked... but I don't have a better explanation. Content-Type is now:

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


Hm. I'll try to reproduce it later. The server just could have been 
misbehaving and they identified and fixed it. _shrug_

Thanks for following up though, it's very helpful


We do not parse HTML. We did in the past. That's why the function exists, for those who feel like they need it.


@kennethreitz understood after reviewing your comments in #156. A mention of the function in the docs would be useful.
Cookie example doesn't work

Hi, I noticed that after I update requests to version 1.0.4, the [simple example](http://docs.python-requests.org/en/latest/user/quickstart/#cookies) on requests site doesn't work.

```
>>> import requests
>>> requests.__version__
'1.0.4'
>>> url = 'http://httpbin.org/cookies/set/requests-is/awesome'
>>> r = requests.get(url)
>>> r.cookies['requests-is']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Library/Python/2.7/site-packages/requests-1.0.4-py2.7.egg/requests/cookies.py", line 243, in   __getitem__
    return self._find_no_duplicates(name)
  File "/Library/Python/2.7/site-packages/requests-1.0.4-py2.7.egg/requests/cookies.py", line 284, in     _find_no_duplicates
    raise KeyError('name=%r, domain=%r, path=%r' % (name, domain, path))
KeyError: "name='requests-is', domain=None, path=None"
>>> 
```


This is directly related to #1069, so we'll try to keep the discussion in there. Thanks for raising the issue! =D
Skip otherwise mandatory headers

This adds support for skip_host and skip_accept_encoding. I bet there are a lot 
of ways this could be refactored to not be completely all over the place. This
enables a feature in httplib and http.client from all the way back in py2.4 to
enable/disable the "Host" and "Accept-Encoding" headers.

This breaks a test in urllib3 and the same problem appears in the requests 
tests. Hopefully someone can help look into why this test is failing in both
cases.


This is related to shazow/urllib3#138 where I have submitted the underlying urllib3 changes.


Thanks for the pull request, but this is unnecessary. 

If you really want to enable this functinality for yourself, you can create a new connection adapter to behave exactly as you expect.
Support x.y.z.w/prefix form in no_proxy

CIDR format is usually used in no_proxy environment varilable.
Now requests modules supports only domain suffix form (.mydomain.example.net) for no_proxy.
I believe it is useful to support CIDR format in no_proxy.

After this commit, we can use no_proxy such as "no_proxy=127.0.0.0/8,192.168.0.0/16,10.0.0.0/8,.mydomain.example.net"

This patch uses 'netaddr' module. I am not sure using a third party module is acceptable in requests module policy.

In addition, I cannot find unit tests in the repo.

Any feedback will be appreciated.
Thanks,
Akihiro


Personally, I'm -1 on adding `netparse` as a requirement. Not sure what @kennethreitz will think though.


Correct.


@kennethreitz 
Let me clarify the reason In my understanding the reason is my patch has a dependency on "netaddr" module. Is it correct? If so this is the policy of this module and I am happy to honor that.

Lastly, do you have a plan to support x.y.z.w/prefix form in no_proxy?
Stop connection

Is there a way to cancel current request?

I'm testing a bunch of URLs that streams video, and I need to output to console the ones that aren't working.  
I can catch the `status_code` but it's just returned once connection ended and data is received.  

I can catch the code using a hook, but is there a way to kill the requests instance from the hook method?


You might be able to stream the requests in this instance.


Yes, but I just don't want to stream the request, just need to know the video stream is OK, and continue testing the rest of the URLs from my list.  
And as I'm doing now, when it founds a valid stream, it keeps connected to it until I cancel it.

I would like to close the stream and continue.


Related: shazow/urllib3/issues/132


@lardissone, @piotr-dobrogost is correct. You can use what I did in that pull request to close it yourself. There just isn't a clean API for it in requests unfortunately.

To recapitulate, look into `stream=True` and then after checking the status_code use `Response.raw` and work from there to incorporate the changes in the PR @piotr-dobrogost referenced above.


Awesome. Thanks!

I'll give it a try later.


Hm, something similar is confusing me. Is partially reading responses by chunks somehow interfering with keeping the session alive?

This is what I am trying to do:

```
response = session.get(url, stream=True)
while True:
    chunk = response.raw.read(1024)
    if not chunk:
        break
    parser.feed(chunk)
    if parser.image:
        response.raw._fp.close()
        return parser.image.size
```

I want to only read enough of the image to be able to parse it using PIL and detect its size - and then cancel the current request, but keep the session alive, because I need to do it for all images on a website. Still, each time I call this snippet (with the same session) a new SSL handshake is established. Did I understand @piotr-dobrogost's PR wrong?


Nope, just what you're doing there doesn't return the connection to the pool. You should be doing this:

``` python
response = session.get(url, stream=True)
while True:
    chunk = response.raw.read(1024)
    if not chunk:
        break
    parser.feed(chunk)
    if parser.image:
        response.close()
        return parser.image.size
```

That will handle returning the connection to the pool.


Ah, OK, great. Thanks for the help!
1.0.4 cannot access request params from response

Notice that params is not in the list in 1.0.4 but is in 1.0.3

1.0.3:

> > > import requests
> > > r = requests.get('http://www.google.com', params={'foo': 'bar'})
> > > dir(r.request)
> > > ['**class**', '**delattr**', '**dict**', '**doc**', '**format**', '**getattribute**', '**hash**', '**init**', '**module**', '**new**', '**reduce**', '**reduce_ex**', '**repr**', '**setattr**', '**sizeof**', '**str**', '**subclasshook**', '**weakref**', '_encode_files', '_encode_params', 'allow_redirects', 'auth', 'body', 'deregister_hook', 'headers', 'hooks', 'method', 'params', 'path_url', 'prepare_auth', 'prepare_body', 'prepare_cookies', 'prepare_headers', 'prepare_method', 'prepare_url', 'proxies', 'register_hook', 'url']

1.0.4:

> > > import requests
> > > r = requests.get('http://www.google.com', params={'foo': 'bar'})
> > > dir(r.request)
> > > ['**class**', '**delattr**', '**dict**', '**doc**', '**format**', '**getattribute**', '**hash**', '**init**', '**module**', '**new**', '**reduce**', '**reduce_ex**', '**repr**', '**setattr**', '**sizeof**', '**str**', '**subclasshook**', '**weakref**', '_encode_files', '_encode_params', 'body', 'deregister_hook', 'headers', 'hooks', 'method', 'path_url', 'prepare_auth', 'prepare_body', 'prepare_cookies', 'prepare_headers', 'prepare_method', 'prepare_url', 'register_hook', 'url']


This is intentional. As far as a PreparedRequest is concerned, params are just part of the URL.
On redirect (302) params are doubled

When the servers redirects (302) and the redirect url already has params in it, requests just adds the params once more, doubling them:

session.py: 82

``` python
    url = resp.headers['location']
```

it already contains the params in it:

session.py: 111

``` python
    resp = self.request(
            url=url,
            method=method,
            headers=headers,
            params=req.params,
            ...
```

now it glues them again on the url.


Yeah, I'm not sure if we need to re-pass the parameters. This is a good catch @alefnula. If you want to ready a commit that removes this, feel free to. I'm sure @Lukasa or @kennethreitz will weigh in after the holidays.


Eh, this is weird.

In principle, the server can put whatever query parameters it likes in the url. This leads to three possible behaviours:
1. Keep the server's query parameters and drop our own (assumes server persists the parameters).
2. Throw away the server's ones and add our own (assumes server is stupid).
3. Merge both ours and the server's. (Still assumes the server is stupid, but less so).


I'm mostly erring on the side of doing (1).


Let's do #1


done