keepalive.HTTPResponse.read decodes response body as UTF-8

[uholzer]

I would expect that the keepalive.HTTPResponse.read (overriding httplib.HTTPResponse.read) method returns str objects. Instead, it returns unicode objects. In fact, it decodes the body as UTF-8 in all cases. It shouldn't do that, because the encoding of the body may be different or the body may not even be text.

[bgw]

Good point. However, it shouldn't return a str on Python 3. it should return a bytes object, which is an alias for str on Python 2.

[wikier]

That's exactly the background of the issue, see commit d202ed8

[wikier]

I'd prefer not to have version-specific code there; so any idea, @uholzer or @bgw?

[joernhees]

as hmm, can't we simply revert d202ed8 and replace all self._buf = '' with self._buf = b'' (similar with data = "" below d202ed8)?

[uholzer]

Python 2.7 does know the b prefix for string literals. Python 2.6 does not seem to do so.

[uholzer]

If you are very clever, you probably can do without any literals:

        s = self._raw_read(amt) if self._rbuf is None else self._rbuf + self._raw_read(amt)
        self._rbuf = None

Once you do have a s of the correct type, you can also do magic like type(s)().

Any more concrete ideas?

[uholzer]

Or you can put some version-specific code at the top to get the right types:

bytes_type = bytes if PYTHON3 else str
unicode_type = str if PYTHON3 else bytes

where PYTHON3 is something Idon't know.

Then use them in the code:

        s = self._rbuf + self._raw_read(amt)
        self._rbuf = bytes_type()

[wikier]

@uholzer, personally I'd prefer to avoid such version specific code there. So I like more @joernhees' s proposal of using self._buf = b'' and raising to 2.7 the minimum version. I'm not aware of many people using SPARQLWrapper in 2.6 anyway; @joernhees would be that a problem for RDFLib?

[wikier]

Patch available as 0.5.dev0; could you please make your tests, @uholzer and @joernhees? In the meantime I'll see what we can do in RDFLib/sparqlwrapper#70.

[joernhees]

python 2.6 supports the b"bla" strings: https://docs.python.org/2/whatsnew/2.6.html#pep-3112-byte-literals

Python 2.6.9 (unknown, Jul 14 2015, 19:46:31)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> b'foobar'
'foobar'

so that will only restrict to >py2.5 and still supporting 2.5 seems like madness anyhow...

that said i updated RDFLib/rdflib#550 to use the most recent keepalive package from github: https://travis-ci.org/RDFLib/rdflib/builds/92929930 (fixes py2 but still breaks something on py3)

[joernhees]

i think it fails on py3 as headers etc. inserted shouldn't be unicode strings either: e569c31#diff-f1079ce199a23a37972b8b644516f133R311

[wikier]

@joernhees you test faced just by luck another issue (see commit 018f118 for further context) with POST requests 😞

Since it fetches the HEAD from github, I restarted the build https://travis-ci.org/RDFLib/rdflib/builds/92929930 and now it looks to work for all python versions.

So it looks to be ready. But I would prefer to have @uholzer also verifying the fix before getting out a new release.

[joernhees]

👍 thanks for digging through all this :-/