Review all use of byte literals

[jodal]

Before the final release of Mopidy 3 we should review all use of byte literals (b'foobar'). Most of these are probably remnants from Python 2 and are now likely sources of bugs.

[girst]

Here's an annotated grep for all byte literals in /mopidy, generated from grep -rn "[^+']\\bb['\"]" mopidy/

looks to me like there are only two problems, in mopidy/internal/playlists.py lines 37 and 51. there, a 1-tuple of a bytes object is passed to xml.etree.ElementTree.iterparse(), which should be either a tuple of strings or a list of strings. I've seen both in code around the web, but the former seems more common.

i've assumed that anything that works on data from a network or disk is working on raw bytes. all my notes below.

mpd/protocol/__init__.py:21:LINE_TERMINATOR = b"\n"
	looks fine
internal/xdg.py:53:    data = b"[XDG_USER_DIRS]\n" + data
	see below
internal/xdg.py:54:    data = data.replace(b"$HOME", bytes(pathlib.Path.home()))
	see below
internal/xdg.py:55:    data = data.replace(b'"', b"")
	data is a bytes object, so that's fine. is decoded afterwards.
internal/playlists.py:23:    return data[0:7].upper() == b"#EXTM3U"
	operates on bytes, afaict. fine.
internal/playlists.py:27:    return data[0:10].lower() == b"[playlist]"
	ditto, fine.
internal/playlists.py:32:    if b"xspf" not in data.lower():
	ditto, fine.
internal/playlists.py:37:        for _event, element in elementtree.iterparse(data, events=(b"start",)):
	XXX: docs say should be a "sequence of strings" -- why a tuple?
internal/playlists.py:46:    if b"asx" not in data.lower():
	operates on bytes, fine.
internal/playlists.py:51:        for _event, element in elementtree.iterparse(data, events=(b"start",)):
	XXX: docs say should be a "sequence of strings"
internal/playlists.py:62:        if found_header or line.startswith(b"#EXTM3U"):
	operates on bytes, fine.
internal/playlists.py:67:        if not line.strip() or line.startswith(b"#"):
	ditto, fine.
internal/playlists.py:124:        if not line.strip() or line.startswith(b"#"):
	ditto, fine.
internal/network.py:215:        self.send_buffer = b""
	socket.send() operates on bytes; fine
internal/network.py:268:            return b""
	ditto, fine.
internal/network.py:389:    terminator = b"\n"
	passed to re.compile(), but then used to split bytes (pydoc says that's to spec). so fine.
internal/network.py:402:        self.recv_buffer = b""
	socket; fine.
internal/http.py:57:    return b"".join(content)
	is http response bytes (should be)? if so, then fine.
config/types.py:61:            return b""
	assuming serialize() should return bytes, then fine.
config/types.py:99:            return b""
	ditto, fine
config/types.py:118:            return b"********"
	ditto, fine
config/types.py:174:            return b"true"
	ditto, fine
config/types.py:176:            return b"false"
	ditto, fine
config/types.py:203:            return b""
	ditto, fine
config/types.py:204:        return b"\n  " + b"\n  ".join(encode(v) for v in value if v)
	ditto, fine
config/types.py:216:        return b""
	ditto, fine
config/types.py:245:        return b""
	ditto, fine

[jodal]

Thank you for the review!

I agree that passing bytes to xml.etree is probably wrong. Well-formed XML documents always have an encoding defined, so I would think that xml.etree should work well if we use text throughout our interactions with it.

I think we might want to try changing ConfigValue.serialize() to return text. I think I left a comment on that in the source while porting to Python 3.

[girst]

On Sun, Dec 08, 2019 at 01:02:42PM -0800, Stein Magnus Jodal wrote: I agree that passing bytes to `xml.etree` is probably wrong. Well-formed XML documents always have an encoding defined, so I would

these are special string constants, not anything from the document, as far as i can tell.

think that `xml.etree` should work well if we use text throughout our interactions with it. I think we might want to try changing `ConfigValue.serialize()` to return text. I think I left a comment on that in the source while porting to Python 3.

I think a serialisation format should return what actually gets put on the disk, not python's intermediate string format. keep it bytes!