diff --git a/docs/changelog.rst b/docs/changelog.rst index 988a0fcd..e1082b09 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -84,6 +84,11 @@ v0.20.0 (UNRELEASED) - Share a single mapping between names and URIs across all MPD sessions. (Fixes: :issue:`934`, PR: :issue:`968`) +- Add support for blacklisting MPD commands. This is used to prevent clients + from using ``listall`` and ``listallinfo`` which recursively lookup the entire + "database". If you insist on using a client that needs these commands change + :confval:`mpd/command_blacklist`. + **Audio** - Deprecated :meth:`mopidy.audio.Audio.emit_end_of_stream`. Pass a diff --git a/docs/ext/mpd.rst b/docs/ext/mpd.rst index ecfab949..b02226a2 100644 --- a/docs/ext/mpd.rst +++ b/docs/ext/mpd.rst @@ -99,3 +99,10 @@ See :ref:`config` for general help on configuring Mopidy. ``$hostname`` and ``$port`` can be used in the name. Set to an empty string to disable Zeroconf for MPD. + +.. confval:: mpd/command_blacklist + + List of MPD commands which are disabled by the server. By default this + setting blacklists ``listall`` and ``listallinfo``. These commands don't + fit well with many of Mopidy's backends and are better left disabled unless + you know what you are doing. diff --git a/mopidy/mpd/__init__.py b/mopidy/mpd/__init__.py index 05c83baa..b2438b07 100644 --- a/mopidy/mpd/__init__.py +++ b/mopidy/mpd/__init__.py @@ -24,6 +24,7 @@ class Extension(ext.Extension): schema['max_connections'] = config.Integer(minimum=1) schema['connection_timeout'] = config.Integer(minimum=1) schema['zeroconf'] = config.String(optional=True) + schema['command_blacklist'] = config.List(optional=True) return schema def validate_environment(self): diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py index b1b2db77..eece86d9 100644 --- a/mopidy/mpd/dispatcher.py +++ b/mopidy/mpd/dispatcher.py @@ -163,6 +163,11 @@ class MpdDispatcher(object): def _call_handler(self, request): tokens = tokenize.split(request) + # TODO: check that blacklist items are valid commands? + blacklist = self.config['mpd'].get('command_blacklist', []) + if tokens and tokens[0] in blacklist: + logger.warning('Client sent us blacklisted command: %s', tokens[0]) + raise exceptions.MpdDisabled(command=tokens[0]) try: return protocol.commands.call(tokens, context=self.context) except exceptions.MpdAckError as exc: diff --git a/mopidy/mpd/exceptions.py b/mopidy/mpd/exceptions.py index e7ab0068..6fc925a3 100644 --- a/mopidy/mpd/exceptions.py +++ b/mopidy/mpd/exceptions.py @@ -87,3 +87,13 @@ class MpdNotImplemented(MpdAckError): def __init__(self, *args, **kwargs): super(MpdNotImplemented, self).__init__(*args, **kwargs) self.message = 'Not implemented' + + +class MpdDisabled(MpdAckError): + # NOTE: This is a custom error for Mopidy that does not exist in MPD. + error_code = 0 + + def __init__(self, *args, **kwargs): + super(MpdDisabled, self).__init__(*args, **kwargs) + assert self.command is not None, 'command must be given explicitly' + self.message = '"%s" has been disabled in the server' % self.command diff --git a/mopidy/mpd/ext.conf b/mopidy/mpd/ext.conf index c62c37ef..fe9a0494 100644 --- a/mopidy/mpd/ext.conf +++ b/mopidy/mpd/ext.conf @@ -6,3 +6,4 @@ password = max_connections = 20 connection_timeout = 60 zeroconf = Mopidy MPD server on $hostname +command_blacklist = listall,listallinfo diff --git a/tests/mpd/test_dispatcher.py b/tests/mpd/test_dispatcher.py index 63981668..d6b11e43 100644 --- a/tests/mpd/test_dispatcher.py +++ b/tests/mpd/test_dispatcher.py @@ -16,6 +16,7 @@ class MpdDispatcherTest(unittest.TestCase): config = { 'mpd': { 'password': None, + 'command_blacklist': ['disabled'], } } self.backend = dummy_backend.create_proxy() @@ -26,14 +27,18 @@ class MpdDispatcherTest(unittest.TestCase): pykka.ActorRegistry.stop_all() def test_call_handler_for_unknown_command_raises_exception(self): - try: + with self.assertRaises(MpdAckError) as cm: self.dispatcher._call_handler('an_unknown_command with args') - self.fail('Should raise exception') - except MpdAckError as e: - self.assertEqual( - e.get_mpd_ack(), - 'ACK [5@0] {} unknown command "an_unknown_command"') + + self.assertEqual( + cm.exception.get_mpd_ack(), + 'ACK [5@0] {} unknown command "an_unknown_command"') def test_handling_unknown_request_yields_error(self): result = self.dispatcher.handle_request('an unhandled request') self.assertEqual(result[0], 'ACK [5@0] {} unknown command "an"') + + def test_handling_blacklisted_command(self): + result = self.dispatcher.handle_request('disabled') + self.assertEqual(result[0], 'ACK [0@0] {disabled} "disabled" has been ' + 'disabled in the server')