Avoid converting to bytes the Python 3 str header values

[dulacp]

the method clean_headers was turning python 3 str instances into bytes, which was causing the httplib2 module to raise an exception.

Cheers

[craigcitro]

can you paste a traceback for the httplib2 exception? (in particular, i confirmed that the cases i thought were likely would work with httplib2 when i made this change, but if there are more we should add tests and find the right fix.)

[methane]

@craigcitro I'm suffered same error.

  File "google.py", line 93, in get_email
    (resp, content) = http.request("https://www.googleapis.com/oauth2/v2/userinfo")
  File "/venv/lib/python3.4/site-packages/oauth2client/util.py", line 135, in positional_wrapper
    return wrapped(*args, **kwargs)
  File "/venv/lib/python3.4/site-packages/oauth2client/client.py", line 547, in new_request
    redirections, connection_type)
  File "/venv/lib/python3.4/site-packages/httplib2/__init__.py", line 1139, in request
    headers = self._normalize_headers(headers)
  File "/venv/lib/python3.4/site-packages/httplib2/__init__.py", line 1107, in _normalize_headers
    return _normalize_headers(headers)
  File "/venv/lib/python3.4/site-packages/httplib2/__init__.py", line 195, in _normalize_headers
    return dict([ (key.lower(), NORMALIZE_SPACE.sub(value, ' ').strip())  for (key, value) in headers.items()])
  File "/venv/lib/python3.4/site-packages/httplib2/__init__.py", line 195, in <listcomp>
    return dict([ (key.lower(), NORMALIZE_SPACE.sub(value, ' ').strip())  for (key, value) in headers.items()])
TypeError: sequence item 0: expected str instance, bytes found

[dulacp]

Finally got it ! I wasn't able to understand why for a while, then it hit me.
In the unit test, an HttpMock is used instead of an httplib2.Http instance.

So the unit test never test the behavior inherited from the actual httplib2.Http instance, which contains the above _normalize_headers method that led to the crash.

Now that the problem is identified I can work on a fix.

Cheers

[dulacp]

I first restore your original code, then patch the HttpMock class to reproduce the header normalization step.
So now, we have to clean failing tests that we can fix, with the same traceback as above TypeError: sequence item 0: expected str instance, bytes found

[dulacp]

The only fix I'm thinking for now, is to fix the httplib2 project.
Like replacing the following one-liner :

def _normalize_headers(headers):
    return dict([ (key.lower(), NORMALIZE_SPACE.sub(value, ' ').strip())  for (key, value) in headers.items()])

with

def _normalize_headers(headers):
    normalized_headers = {}
    for key, value in headers.items():
        if isinstance(value, bytes):
            value = value.decode('utf8')
            value = NORMALIZE_SPACE.sub(value, ' ').strip()
            value = value.encode('utf8')
        else:
            value = NORMALIZE_SPACE.sub(value, ' ').strip()
        normalized_headers[key.lower()] = value
    return normalized_headers

If you have another suggestion @craigcitro, don't hesitate to stop me.
There seems to be a PR, jcgregorio/httplib2#291, that looks quite similar to my patch. Unfortunately, the httplib2 project doesn't seem to be very actively maintained...

[nathanielmanistaatgoogle]

@dulaccc: if the best way to solve the problem you've described here is with a change to httplib2, we'd like to exhaust that possibility before looking further at this pull request changing oauth2client. @jcgregorio has asked for unit test coverage in jcgregorio/httplib2#291 so it looks like that's the way to move all this forward?

[nathanielmanistaatgoogle]

Please reassess this pull request now that jcgregorio/httplib2#291 has been accepted?

[nathanielmanistaatgoogle]

Friendly ping - what work remains to be done here?

[nathanielmanistaatgoogle]

Also, please correct me if I'm wrong, but it looks like you haven't yet signed Google's Contributor License Agreement. You must do so before we can accept your contribution.

[dhermes]

@nathanielmanistaatgoogle This can be closed. These issues were address upstream in jcgregorio/httplib2#291 and jcgregorio/httplib2#296

[nathanielmanistaatgoogle]

I thought so, but was leaving it open on the off chance that there was something we might want to do here with respect to httplib2 not yet having cut a new release.

The more I think about it, the more that's a problem outside the scope of this pull request.