eonpatapon / mpDris2

MPRIS V2.1 support for mpd
GNU General Public License v3.0
211 stars 46 forks source link

Protect access to last_currentsong #145

Closed ferdnyc closed 2 years ago

ferdnyc commented 2 years ago

I confess I'm not sure when or how it would happen, myself, but we received a crashreporter log for the Fedora package indicating that the value of self._currentsong could in certain circumstances be a boolean value. (False, one presumes.):

Version-Release number of selected component:
mpdris2-0.8-6.20200205git491588a.fc34

Additional info:
reporter:       libreport-2.14.0
cgroup:         0::/user.slice/user-1000.slice/user@1000.service/app.slice/app-gnome-mpdris2-47029.scope
cmdline:        /usr/bin/python3 /usr/bin/mpDris2
crash_function: last_currentsong
exception_type: AttributeError
executable:     /usr/bin/mpDris2
interpreter:    python3-3.9.5-2.fc34.x86_64
kernel:         5.11.21-300.fc34.x86_64
runlevel:       N 5
type:           Python3
uid:            1000

Truncated backtrace:
mpDris2:499:last_currentsong:AttributeError: 'bool' object has no attribute 'copy'

Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/gi/overrides/GLib.py", line 671, in <lambda>
    func_fdtransform = lambda _, cond, *data: callback(channel, cond, *data)
  File "/usr/bin/mpDris2", line 475, in socket_callback
    self._update_properties(force=True)
  File "/usr/bin/mpDris2", line 761, in _update_properties
    self.update_metadata()
  File "/usr/bin/mpDris2", line 511, in update_metadata
    mpd_meta = self.last_currentsong()
  File "/usr/bin/mpDris2", line 499, in last_currentsong
    return self._currentsong.copy()
AttributeError: 'bool' object has no attribute 'copy'

Local variables in innermost frame:
self: <__main__.MPDWrapper object at 0x7fd8681c93a0>

(This was against 0.8 — I'm building packages of the latest version now — but there have been absolutely no changes in the source between 0.8 and the current HEAD that would affect this bug at all... if it was present, it's still present.)

So, this PR protects all attempts to use the return from self.last_currentsong() with truthiness checks, and changes last_currentsong() itself to check that self._currentsong isn't False, before attempting to call its copy() method.