Query params are not properly sanitized

Hey,

One of our developers (Randy Tobias) encountered a problem today when attempting to query a URL which is collected from the 'Location' header.

The following code will fail:

```

_url = "http://www.example.com/?hello=hello world"
requests.get(_url)

```

The only way to make this work is to patch up the query string with urlencode - therefore I am submitting this patch for consideration. I believe that enough websites send back broken "Location" headers for this to be included in the core.

Without it, we get the following exception:

```

webapp.libs._requests.exceptions.ConnectionError: HTTPSConnectionPool(host=u'online2.statefarm.com', port=443): Max retries exceeded with url: /apps/iarqapply/launch.redirect?postalCode=07017&productId=F1MRW45Q500&effectiveDate=20120214&applicationName=Auto&stateAbbreviation=NJ&regionalOffice=17&endEffectiveDate=20120315&city=East Orange


```

Here is the patch:

```
# models.py

def patch_url(i):
    import urlparse
    import urllib
    o = urlparse.urlparse(i)

    _n = urlparse.ParseResult(
        scheme = o.scheme,
        netloc = o.netloc,
        path = o.path,
        params = o.params,
        query = urllib.urlencode(urlparse.parse_qsl(o.query)),
        fragment = o.fragment,
    )

    return _n.geturl()

class Request(object):
    """The :class:`Request <Request>` object. It carries out all functionality of
    Requests. Recommended interface is with the Requests functions.
    """

    def __init__(self,
        url=None,
        headers=dict(),
        files=None,
        method=None,
        data=dict(),
        params=dict(),
        auth=None,
        cookies=None,
        timeout=None,
        redirect=False,
        allow_redirects=False,
        proxies=None,
        hooks=None,
        config=None,
        _poolmanager=None,
        verify=None,
        session=None):

        #: Float describes the timeout of the request.
        #  (Use socket.setdefaulttimeout() as fallback)
        self.timeout = timeout

        #: Request URL.

        # if isinstance(url, str):
            # url = url.encode('utf-8')
            # print(dir(url))

        self.url = patch_url(url)

```

```

Before patch: https://online2.statefarm.com/apps/iarqapply/launch.redirect?postalCode=07017&productId=F1MRW45Q500&effectiveDate=20120214&applicationName=Auto&stateAbbreviation=NJ&regionalOffice=17&endEffectiveDate=20120315&city=East Orange

```

```

After patch: https://online2.statefarm.com/apps/iarqapply/launch.redirect?postalCode=07017&productId=F1MRW45Q500&effectiveDate=20120214&applicationName=Auto&stateAbbreviation=NJ&regionalOffice=17&endEffectiveDate=20120315&city=East+Orange

```

Many thanks

Cal Leeming
Simplicity Media Ltd
- Randy Tobias


See issue #369. Once changes made by @mgiuca are merged your problem should be fixed as well.


I don't understand exactly what the problem is. It looks like you sent a request to the URL:

```
http://www.example.com/?hello=hello world
```

And then you report an exception involving this URL:

```
/apps/iarqapply/launch.redirect?postalCode=07017&productId=F1MRW45Q500&effectiveDate=20120214&applicationName=Auto&stateAbbreviation=NJ&regionalOffice=17&endEffectiveDate=20120315&city=East Orange
```

What do those two URLs have to do with each other? They seem to both have in common a space in the query string.

Let's pick a more useful test case: a URL on Kenneth's nice HTTP test site that won't redirect (unlike example.com):

```
http://httpbin.org/get?hello=hello world
```

My test code:

```
>>> _url = "http://httpbin.org/get?hello=hello world"
>>> r = requests.get(_url)
>>> r.url
u'http://httpbin.org/get?hello=hello world'
```

(I'm not sure why it became a Unicode string.)

Indeed, this is a malformed query string, because it has a space in it. Note that this is AFTER my patch was merged. My patch deliberately only dealt with the path component (because I didn't want to mess with the behaviour too much; requests was already munging the path but not the query). If we want requests to automatically fix this, it should fix it as follows:

```
http://httpbin.org/get?hello=hello%20world
```

It should NOT be encoding the space as a '+'. Space is NOT equivalent to + in the URI syntax, and doing so would break existing correct code. Space is correctly encoded as %20. The replacement of space with '+' is not part of the general URI syntax; it is part of the HTML specification as [defined by the W3C](http://www.w3.org/TR/REC-html40/interact/forms.html#form-content-type) -- the content type application/x-www-form-urlencoded defines '+' as equivalent to space. Any URI that is not the result of an HTML form submission should not consider those equivalent.

So the fixes I made on Issue #369 were actually relevant here, it's just that they were only applied to the path. I have pushed a new branch, [uri-cleanup-nonpath](https://github.com/mgiuca/requests/tree/uri-cleanup-nonpath) which extends the application of `requote_path` to the entire URI once it is reconstructed, which means that all parts of the URI will now benefit from cleaning. This gives the following result to the above test:

```
>>> r.url
'http://httpbin.org/get?hello=hello%20world'
```

I have added full tests of the sanitisation of the query part of the URI, and I think that my patch would resolve this issue. I'll put in a pull request.

P.S. When you want to send a patch, please create a Git branch and commit it, then push to GitHub and create a Pull Request. That will make it much easier for people to see what changes you intend to submit, rather than simply pasting code into a bug report.


I should add, one of our developers (James Cleary) found that our previously suggested patch causes other unit tests to break, YMMV.


encode_uri configuration has been added.


Nice! Thank you very much for this Kenneth :)


Also @mgiuca, thank you very much for raising the issue about the + and %20 stuff, our patch was failing on several tests due to this problem - so you've saved us a lot of time :) Thank you again.


No worries :) I'm a bit obsessed with URI encoding issues so I'm happy to help out.


@mgiuca good to know :)


Thanks from me as well! Ran into the issue yesterday, and today it's fixed.


@mgiuca we could really use your help with a new collaboration with @mitsuhiko to merge requests and werkzeug. Would you like to help?


I'd like to but I'm moving house and starting a new job this week, sorry. I might look at it in a few days.


@mgiuca It'll likely be over the next month or two :)


> (...) a new collaboration with @mitsuhiko to merge requests and werkzeug. 

Was the intention of merging published somewhere?


@piotr-dobrogost I'm planning on doing a blog post on it today


:tophat: