Merge pull request #990 from adamcik/feature/mpd-blacklist-commands

mpd: Support blacklisting MPD commands in the server.

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

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.

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):

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:

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

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

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')