Non-ascii (iso-8859-1) location headers are handled incorrectly on handling a redirect

  Hi,

Akkording to the RFC (2616, https://www.ietf.org/rfc/rfc2616.txt) the location header MAY contain non-ascii characters if encoded with iso-8859-1. However, this case is handled incorrectly by requests. 

httplib will correctly parse the header as iso-8859-1 (see parse_headers in http/client.py, around line 214 for python 3.5), but the resulting string, when following the redirect, is quoted without specifying an encoding by requests (see requote_uri in requests/utils.py):
```
def requote_uri(uri):
    """Re-quote the given URI.

    This function passes the given URI through an unquote/quote cycle to
    ensure that it is fully and consistently quoted.

    :rtype: str
    """
    safe_with_percent = "!#$%&'()*+,/:;=?@[]~"
    safe_without_percent = "!#$&'()*+,/:;=?@[]~"
    try:
        # Unquote only the unreserved characters
        # Then quote only illegal characters (do not quote reserved,
        # unreserved, or '%')
        print('Yo')
        return quote(unquote_unreserved(uri), safe=safe_with_percent)
    except InvalidURL:
        # We couldn't unquote the given URI, so let's try quoting it, but
        # there may be unquoted '%'s in the URI. We need to make sure they're
        # properly quoted so they do not cause issues elsewhere.
        return quote(uri, safe=safe_without_percent)
```

The calls to quote should have ```encoding='iso-8859-1'``` as a parameter otherwise quote will assume utf-8 and that will result in a wrong url. I.e.:

```
return quote(unquote_unreserved(uri), encoding='iso-8859-1', safe=safe_with_percent)
```

I have looked into fixing it, which I'm willing to do, but not 100% sure where to add the test. My best guess is doing it in test_lowlevel.py using the test_server (httpbin can't be used for this as it quotes the non-ascii characters).

Hi there @atleta,

First, RFC 2616 has been made obsolete by RFCs [7230][], [7231][], [7232][], [7233][], [7234][], and [7235][].

As you'll see when you familiarize yourself with those RFCs, the *default* encoding for Headers is no longer ISO-8859-1 (a.k.a., Latin-1).

I suspect that if you have a specific encoding you want to do you should disallow redirects and take control over URL encoding yourself. @Lukasa may disagree, though.

[7230]: https://tools.ietf.org/html/rfc7230
[7231]: https://tools.ietf.org/html/rfc7231
[7232]: https://tools.ietf.org/html/rfc7232
[7233]: https://tools.ietf.org/html/rfc7233
[7234]: https://tools.ietf.org/html/rfc7234
[7235]: https://tools.ietf.org/html/rfc7235

Thanks for this report!

Firstly, RFC 2616 is dead. It has been superseded by RFCs 723[0-3] for HTTP/1.1. Secondly, ISO-8859-1 is not unambiguously the correct encoding for header values. In practice, no HTTP specification has ever unequivocally required that ISO-8859-1 be used for header values, and it is quite common to encounter non-ISO-8859-1 header values. The only requirement RFC 7230 places on the receiver is to parse the HTTP message using an encoding that is a superset of US-ASCII, which allows both ISO-8859-1 and UTF-8.

However, I agree that there is a problem: it's just not been characterised quite right. The *real* problem is that we should be operating on the location header as a set of bytes that are a encoded in some random encoding that is a superset of US-ASCII. Because of httplib's decision to decode using ISO-8859-1 (a *bad* decision, but there we are), we need to *re-encode* using ISO-8859-1 as well to get back to the "random sequence of bytes" place.

So, I agree that this is an issue, but I'm not entirely confident it's a real bad one. I'd be open to a fix, but I'd definitely want to see a test that reproduces a real problem. You're right that `test_lowlevel.py` is the right place, so if you can write a test there that demonstrates the issue I'll be open to considering a fix for this (and working out whether it is likely to break something or not).

Also, worthy of note, in 7230 on [Page 26](https://tools.ietf.org/html/rfc7230#page-26)

>   Historically, HTTP has allowed field content with text in the
>   ISO-8859-1 charset [ISO-8859-1], supporting other charsets only
>   through use of [RFC2047] encoding.  In practice, most HTTP header
>   field values use only a subset of the US-ASCII charset [USASCII].
>   Newly defined header fields SHOULD limit their field values to
>   US-ASCII octets.  A recipient SHOULD treat other octets in field
>   content (obs-text) as opaque data.


I believe this has also been discussed as an issue to fix in urllib3 in shazow/urllib3#952, which may be the more appropriate location to handle this.

Haha, obviously one shouldn't believe everything they read on stackoverflow. I suspected that the rfc might be out of date but didn't put in enough efforts to find the final version :) (As a side note, the quote from 7230 seems to refer to newly defined fields, which Location isn't. But will check it out, as I also reported this as a bug towards the operators of the service.)

But indeed how I ran into the issue was a server that sent the location header utf-8 encoded and after some debugging I found that http lib parses it as iso-8859-1 and as a result re-encoding it as utf-8 will cause problems with any non-ascii location header.

@nateprewitt Nah, I think these are different issues. This is because requests *does* urlencode the redirect header, but it also *character* encodes it, wrongly.

But yeah, I'm prepared to believe that that example is a good example of how we get it wrong. Happily, ISO-8859-1 is round-trippable.

Hi all, 

After digging into requests, urllib3 and httplib regarding response headers and location encoding lately (on #3963), I am willing to try and solve this issue :smile: .

# The Problem
AFAICS, the issue starts with the difference between python2 and python3 httplib implementation of reading headers.

in python3 the code that parse the headers is like this (from [here](https://github.com/python/cpython/blob/master/Lib/http/client.py#L213)):

```python
def parse_headers(fp, _class=HTTPMessage):
    """Parses only RFC2822 headers from a file pointer.
    email Parser wants to see strings rather than bytes.
    But a TextIOWrapper around self.rfile would buffer too many bytes
    from the stream, bytes which we later need to read as bytes.
    So we read the correct bytes here, as bytes, for email Parser
    to parse.
    """
    headers = []
    while True:
        line = fp.readline(_MAXLINE + 1)
        if len(line) > _MAXLINE:
            raise LineTooLong("header line")
        headers.append(line)
        if len(headers) > _MAXHEADERS:
            raise HTTPException("got more than %d headers" % _MAXHEADERS)
        if line in (b'\r\n', b'\n', b''):
            break
    hstring = b''.join(headers).decode('iso-8859-1')
    return email.parser.Parser(_class=_class).parsestr(hstring)
```

**we can clearly see it decodes into iso-8859-1 (aka latin1)**

now on python2 the code is completely different (from [here](https://github.com/python/cpython/blob/2.7/Lib/httplib.py#L271)):

```python
    def readheaders(self):
...........
        self.headers = hlist = []
        self.status = ''
        headerseen = ""
        firstline = 1
        tell = None
        if not hasattr(self.fp, 'unread') and self.seekable:
            tell = self.fp.tell
        while True:
            if len(hlist) > _MAXHEADERS:
                raise HTTPException("got more than %d headers" % _MAXHEADERS)
...........
            if headerseen and line[0] in ' \t':
                # XXX Not sure if continuation lines are handled properly
                # for http and/or for repeating headers
                # It's a continuation line.
                hlist.append(line)
                self.addcontinue(headerseen, line.strip())
                continue
...........
            if headerseen:
                # It's a legal header line, save it.
                hlist.append(line)
                self.addheader(headerseen, line[len(headerseen)+1:].strip())
 ..........
```

**on python2 it returns the data encoded (i.e. python2 str)**

This inconsistency affect requests (and probably urllib3 - issue https://github.com/shazow/urllib3/issues/952).

# Possible Solutions
1. solve this on requests:
change `SessionRedirectMixin.get_redirect_target()` to something like this:

```python
    def get_redirect_target(self, resp):
        """Receives a Response. Returns a redirect URI or ``None``"""
        if resp.is_redirect:
            location = resp.headers['location']
            # issue with httplib blbablabla
            if is_py3:
                location = location.encode('latin1')
            return to_native_string(location, 'utf8')
        return None
```

2. solve this on urllib3

please advise if any of the solutions (or any other solution) is desired and I can write some PR for it.

Thanks,
Shmulik

This is solved in the v2 branch of urllib3 so we don't need to worry about it right now. Instead, let's focus on Requests and the here-and-now.

As discussed above, I think the thing to do is to do something similar to your suggestion. However, it's vitally important that we have a failing test *first* to validate that this works the way we want it to.

I am also unsure whether or not this is a breaking change yet. It feels like it might be, given that it changes the return value of `get_redirect_target`.

ok lets do this, I'll try write a PR for fixing it. 

Can you provide more info regarding the API changes?
the code above returns same type (unicode on python3, bytes on python2), its only a fixing the wrong encoding. what am I missing?

Oh good spot. So the only risk is that we'll break anyone who is already working around this issue in their code, but I think that's ok. We'll be doing them a favour in the long term. ;)

can we close this issue now? anything else that I can help with?

It looks like this is wrapped up now with #3969. Thanks for all your work @shmuelamar!