From 0997cc1c58e84217d8727ae3583a1beade45a55a Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 27 Oct 2010 10:30:51 +0200 Subject: [PATCH 01/12] Proof of concept: Password authentication We need to at least support the 'commands' command when not authenticated. --- mopidy/frontends/mpd/session.py | 17 +++++++++++++++++ mopidy/settings.py | 5 +++++ 2 files changed, 22 insertions(+) diff --git a/mopidy/frontends/mpd/session.py b/mopidy/frontends/mpd/session.py index 580b5905..3ad80c92 100644 --- a/mopidy/frontends/mpd/session.py +++ b/mopidy/frontends/mpd/session.py @@ -2,6 +2,7 @@ import asynchat import logging import multiprocessing +from mopidy import settings from mopidy.frontends.mpd.protocol import ENCODING, LINE_TERMINATOR, VERSION from mopidy.utils.log import indent from mopidy.utils.process import pickle_connection @@ -22,6 +23,7 @@ class MpdSession(asynchat.async_chat): self.client_port = client_socket_address[1] self.core_queue = core_queue self.input_buffer = [] + self.authenticated = False self.set_terminator(LINE_TERMINATOR.encode(ENCODING)) def start(self): @@ -46,6 +48,9 @@ class MpdSession(asynchat.async_chat): def handle_request(self, request): """Handle request by sending it to the MPD frontend.""" + if not self.authenticated: + self.authenticated = self.check_password(request) + return my_end, other_end = multiprocessing.Pipe() self.core_queue.put({ 'to': 'frontend', @@ -69,3 +74,15 @@ class MpdSession(asynchat.async_chat): output = u'%s%s' % (output, LINE_TERMINATOR) data = output.encode(ENCODING) self.push(data) + + def check_password(self, request): + if not settings.MPD_SERVER_PASSWORD: + return True + if request == 'password "%s"' % settings.MPD_SERVER_PASSWORD: + self.send_response('OK') + return True + command = request.split(' ')[0] + self.send_response( + "ACK [4@0] {%s} " % command + + "you don't have permission for \"%s\"" % command) + return False diff --git a/mopidy/settings.py b/mopidy/settings.py index c9d7b9fc..da08584e 100644 --- a/mopidy/settings.py +++ b/mopidy/settings.py @@ -164,6 +164,11 @@ OUTPUT = u'mopidy.outputs.gstreamer.GStreamerOutput' #: Listens on all interfaces, both IPv4 and IPv6. MPD_SERVER_HOSTNAME = u'127.0.0.1' +#: Password required for connecting to the MPD server. +#: +#: Default: :class:`False`, which means no password required. +MPD_SERVER_PASSWORD = False + #: Which TCP port Mopidy's MPD server should listen to. #: #: Default: 6600 From 8f0e00e1d781b60b2bb5cb2f6c850ff2627eb5ce Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 00:17:01 +0100 Subject: [PATCH 02/12] Implement 'password' command which will be reached post-auth --- mopidy/frontends/mpd/exceptions.py | 5 +++++ mopidy/frontends/mpd/protocol/connection.py | 9 +++++++-- tests/frontends/mpd/connection_test.py | 19 +++++++++++++++++-- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/mopidy/frontends/mpd/exceptions.py b/mopidy/frontends/mpd/exceptions.py index 2a18b2f3..53a31dd9 100644 --- a/mopidy/frontends/mpd/exceptions.py +++ b/mopidy/frontends/mpd/exceptions.py @@ -39,6 +39,11 @@ class MpdArgError(MpdAckError): super(MpdArgError, self).__init__(*args, **kwargs) self.error_code = 2 # ACK_ERROR_ARG +class MpdPasswordError(MpdAckError): + def __init__(self, *args, **kwargs): + super(MpdPasswordError, self).__init__(*args, **kwargs) + self.error_code = 3 # ACK_ERROR_PASSWORD + class MpdUnknownCommand(MpdAckError): def __init__(self, *args, **kwargs): super(MpdUnknownCommand, self).__init__(*args, **kwargs) diff --git a/mopidy/frontends/mpd/protocol/connection.py b/mopidy/frontends/mpd/protocol/connection.py index 0ce3ef51..65811d09 100644 --- a/mopidy/frontends/mpd/protocol/connection.py +++ b/mopidy/frontends/mpd/protocol/connection.py @@ -1,5 +1,6 @@ +from mopidy import settings from mopidy.frontends.mpd.protocol import handle_pattern -from mopidy.frontends.mpd.exceptions import MpdNotImplemented +from mopidy.frontends.mpd.exceptions import MpdPasswordError @handle_pattern(r'^close$') def close(frontend): @@ -33,7 +34,11 @@ def password_(frontend, password): This is used for authentication with the server. ``PASSWORD`` is simply the plaintext password. """ - raise MpdNotImplemented # TODO + # You will not get to this code without being authenticated. This is for + # when you are already authenticated, and are sending additional 'password' + # requests. + if settings.MPD_SERVER_PASSWORD != password: + raise MpdPasswordError(u'incorrect password', command=u'password') @handle_pattern(r'^ping$') def ping(frontend): diff --git a/tests/frontends/mpd/connection_test.py b/tests/frontends/mpd/connection_test.py index 21753054..e7bb74de 100644 --- a/tests/frontends/mpd/connection_test.py +++ b/tests/frontends/mpd/connection_test.py @@ -1,5 +1,6 @@ import unittest +from mopidy import settings from mopidy.backends.dummy import DummyBackend from mopidy.frontends.mpd import dispatcher from mopidy.mixers.dummy import DummyMixer @@ -9,6 +10,9 @@ class ConnectionHandlerTest(unittest.TestCase): self.b = DummyBackend(mixer_class=DummyMixer) self.h = dispatcher.MpdDispatcher(backend=self.b) + def tearDown(self): + settings.runtime.clear() + def test_close(self): result = self.h.handle_request(u'close') self.assert_(u'OK' in result) @@ -21,9 +25,20 @@ class ConnectionHandlerTest(unittest.TestCase): result = self.h.handle_request(u'kill') self.assert_(u'OK' in result) - def test_password(self): + def test_valid_password_is_accepted(self): + settings.MPD_SERVER_PASSWORD = u'topsecret' + result = self.h.handle_request(u'password "topsecret"') + self.assert_(u'OK' in result) + + def test_invalid_password_is_not_accepted(self): + settings.MPD_SERVER_PASSWORD = u'topsecret' result = self.h.handle_request(u'password "secret"') - self.assert_(u'ACK [0@0] {} Not implemented' in result) + self.assert_(u'ACK [3@0] {password} incorrect password' in result) + + def test_any_password_is_not_accepted_when_password_check_turned_off(self): + settings.MPD_SERVER_PASSWORD = False + result = self.h.handle_request(u'password "secret"') + self.assert_(u'ACK [3@0] {password} incorrect password' in result) def test_ping(self): result = self.h.handle_request(u'ping') From af0f80d2385001b52560cb0ec9d31e85c4a891bf Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 00:17:30 +0100 Subject: [PATCH 03/12] Add MPD_SERVER_PASSWORD to list of relevant frontend settings --- mopidy/frontends/mpd/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index ce9abc6d..2f87088c 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -14,6 +14,7 @@ class MpdFrontend(BaseFrontend): **Settings:** - :attr:`mopidy.settings.MPD_SERVER_HOSTNAME` + - :attr:`mopidy.settings.MPD_SERVER_PASSWORD` - :attr:`mopidy.settings.MPD_SERVER_PORT` """ From 00897e90240fb80d1697979ddec3fa7032c29e6e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 00:18:04 +0100 Subject: [PATCH 04/12] Improve MPD_SERVER_PASSWORD description --- mopidy/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/settings.py b/mopidy/settings.py index b2ee5b12..add32428 100644 --- a/mopidy/settings.py +++ b/mopidy/settings.py @@ -164,7 +164,7 @@ OUTPUT = u'mopidy.outputs.gstreamer.GStreamerOutput' #: Listens on all interfaces, both IPv4 and IPv6. MPD_SERVER_HOSTNAME = u'127.0.0.1' -#: Password required for connecting to the MPD server. +#: The password required for connecting to the MPD server. #: #: Default: :class:`False`, which means no password required. MPD_SERVER_PASSWORD = False From 775ec649763f77bd869d203e5de7f9cfe1697e5b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 02:15:27 +0100 Subject: [PATCH 05/12] Test and improve password handling in MpdSession --- mopidy/frontends/mpd/session.py | 32 ++++++++++++------ tests/frontends/mpd/server_test.py | 53 ++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 10 deletions(-) diff --git a/mopidy/frontends/mpd/session.py b/mopidy/frontends/mpd/session.py index 3ad80c92..b67e7f13 100644 --- a/mopidy/frontends/mpd/session.py +++ b/mopidy/frontends/mpd/session.py @@ -49,8 +49,10 @@ class MpdSession(asynchat.async_chat): def handle_request(self, request): """Handle request by sending it to the MPD frontend.""" if not self.authenticated: - self.authenticated = self.check_password(request) - return + (self.authenticated, response) = self.check_password(request) + if response is not None: + self.send_response(response) + return my_end, other_end = multiprocessing.Pipe() self.core_queue.put({ 'to': 'frontend', @@ -76,13 +78,23 @@ class MpdSession(asynchat.async_chat): self.push(data) def check_password(self, request): + """ + Takes any request and tries to authenticate the client using it. + + :rtype: a two-tuple containing (is_authenticated, response_message). If + the response_message is :class:`None`, normal processing should + continue, even though the client may not be authenticated. + """ if not settings.MPD_SERVER_PASSWORD: - return True - if request == 'password "%s"' % settings.MPD_SERVER_PASSWORD: - self.send_response('OK') - return True + return (True, None) command = request.split(' ')[0] - self.send_response( - "ACK [4@0] {%s} " % command + - "you don't have permission for \"%s\"" % command) - return False + if command == 'password': + if request == 'password "%s"' % settings.MPD_SERVER_PASSWORD: + return (True, u'OK') + else: + return (False, u'ACK [3@0] {password} incorrect password') + if command in ('close', 'commands', 'notcommands', 'ping'): + return (False, None) + else: + return (False, "ACK [4@0] {%s} " % command + + "you don't have permission for \"%s\"" % command) diff --git a/tests/frontends/mpd/server_test.py b/tests/frontends/mpd/server_test.py index 9d006eb3..9d877ed5 100644 --- a/tests/frontends/mpd/server_test.py +++ b/tests/frontends/mpd/server_test.py @@ -1,5 +1,6 @@ import unittest +from mopidy import settings from mopidy.frontends.mpd import server class MpdServerTest(unittest.TestCase): @@ -21,8 +22,60 @@ class MpdSessionTest(unittest.TestCase): def setUp(self): self.session = server.MpdSession(None, None, (None, None), None) + def tearDown(self): + settings.runtime.clear() + def test_found_terminator_catches_decode_error(self): # Pressing Ctrl+C in a telnet session sends a 0xff byte to the server. self.session.input_buffer = ['\xff'] self.session.found_terminator() self.assertEqual(len(self.session.input_buffer), 0) + + def test_authentication_with_valid_password_is_accepted(self): + settings.MPD_SERVER_PASSWORD = u'topsecret' + authed, response = self.session.check_password(u'password "topsecret"') + self.assertTrue(authed) + self.assertEqual(u'OK', response) + + def test_authentication_with_invalid_password_is_not_accepted(self): + settings.MPD_SERVER_PASSWORD = u'topsecret' + authed, response = self.session.check_password(u'password "secret"') + self.assertFalse(authed) + self.assertEqual(u'ACK [3@0] {password} incorrect password', response) + + def test_authentication_with_anything_when_password_check_turned_off(self): + settings.MPD_SERVER_PASSWORD = False + authed, response = self.session.check_password(u'any request at all') + self.assertTrue(authed) + self.assertEqual(None, response) + + def test_anything_when_not_authenticated_should_fail(self): + settings.MPD_SERVER_PASSWORD = u'topsecret' + authed, response = self.session.check_password(u'any request at all') + self.assertFalse(authed) + self.assertEqual( + u'ACK [4@0] {any} you don\'t have permission for "any"', response) + + def test_close_is_allowed_without_authentication(self): + settings.MPD_SERVER_PASSWORD = u'topsecret' + authed, response = self.session.check_password(u'close') + self.assertFalse(authed) + self.assertEqual(None, response) + + def test_commands_is_allowed_without_authentication(self): + settings.MPD_SERVER_PASSWORD = u'topsecret' + authed, response = self.session.check_password(u'commands') + self.assertFalse(authed) + self.assertEqual(None, response) + + def test_notcommands_is_allowed_without_authentication(self): + settings.MPD_SERVER_PASSWORD = u'topsecret' + authed, response = self.session.check_password(u'notcommands') + self.assertFalse(authed) + self.assertEqual(None, response) + + def test_ping_is_allowed_without_authentication(self): + settings.MPD_SERVER_PASSWORD = u'topsecret' + authed, response = self.session.check_password(u'ping') + self.assertFalse(authed) + self.assertEqual(None, response) From 4953a80cce16a75e3802889dddb0586980c8ffa3 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 02:20:07 +0100 Subject: [PATCH 06/12] Add FIXME's to 'commands' and 'notcommands', as they currently do not care if the client is authenticated or not --- mopidy/frontends/mpd/protocol/reflection.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/mopidy/frontends/mpd/protocol/reflection.py b/mopidy/frontends/mpd/protocol/reflection.py index d2c9c599..83efdd94 100644 --- a/mopidy/frontends/mpd/protocol/reflection.py +++ b/mopidy/frontends/mpd/protocol/reflection.py @@ -9,9 +9,12 @@ def commands(frontend): ``commands`` Shows which commands the current user has access to. - - As permissions is not implemented, any user has access to all commands. """ + # FIXME When password auth is turned on and the client is not + # authenticated, 'commands' should list only the commands the client does + # have access to. To implement this we need access to the session object to + # check if the client is authenticated or not. + sorted_commands = sorted(list(mpd_commands)) # Not shown by MPD in its command list @@ -51,9 +54,11 @@ def notcommands(frontend): ``notcommands`` Shows which commands the current user does not have access to. - - As permissions is not implemented, any user has access to all commands. """ + # FIXME When password auth is turned on and the client is not + # authenticated, 'notcommands' should list all the commands the client does + # not have access to. To implement this we need access to the session + # object to check if the client is authenticated or not. pass @handle_pattern(r'^tagtypes$') From c85292ab5fd8516ac4d3f02301ac5fe88ed7feea Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 02:26:33 +0100 Subject: [PATCH 07/12] docs: Add section on how to use MPD_SERVER_PASSWORD --- docs/settings.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/settings.rst b/docs/settings.rst index 532f52cf..9adbe76b 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -95,6 +95,13 @@ As a secure default, Mopidy only accepts connections from ``localhost``. If you want to open it for connections from other machines on your network, see the documentation for :attr:`mopidy.settings.MPD_SERVER_HOSTNAME`. +If you open up Mopidy for your local network, you should consider turning on +MPD password authentication by setting +:attr:`mopidy.settings.MPD_SERVER_PASSWORD` to the password you want to use. +If the password is set, Mopidy will require MPD clients to provide the password +before they can do anything else. Mopidy only supports a single password, and +do not support different permission schemes like the original MPD server. + Scrobbling tracks to Last.fm ============================ From bf46b73b646920c0db4ce842fdc8f7337ef0a2e1 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 16:05:04 +0100 Subject: [PATCH 08/12] Fix typo in variable name --- tests/utils/settings_test.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/utils/settings_test.py b/tests/utils/settings_test.py index cef0069d..f7a95eaa 100644 --- a/tests/utils/settings_test.py +++ b/tests/utils/settings_test.py @@ -69,34 +69,34 @@ class SettingsProxyTest(unittest.TestCase): def test_value_ending_in_path_is_expanded(self): self.settings.TEST_PATH = '~/test' - acctual = self.settings.TEST_PATH + actual = self.settings.TEST_PATH expected = os.path.expanduser('~/test') - self.assertEqual(acctual, expected) + self.assertEqual(actual, expected) def test_value_ending_in_path_is_absolute(self): self.settings.TEST_PATH = './test' - acctual = self.settings.TEST_PATH + actual = self.settings.TEST_PATH expected = os.path.abspath('./test') - self.assertEqual(acctual, expected) + self.assertEqual(actual, expected) def test_value_ending_in_file_is_expanded(self): self.settings.TEST_FILE = '~/test' - acctual = self.settings.TEST_FILE + actual = self.settings.TEST_FILE expected = os.path.expanduser('~/test') - self.assertEqual(acctual, expected) + self.assertEqual(actual, expected) def test_value_ending_in_file_is_absolute(self): self.settings.TEST_FILE = './test' - acctual = self.settings.TEST_FILE + actual = self.settings.TEST_FILE expected = os.path.abspath('./test') - self.assertEqual(acctual, expected) + self.assertEqual(actual, expected) def test_value_not_ending_in_path_or_file_is_not_expanded(self): self.settings.TEST = '~/test' - acctual = self.settings.TEST - self.assertEqual(acctual, '~/test') + actual = self.settings.TEST + self.assertEqual(actual, '~/test') def test_value_not_ending_in_path_or_file_is_not_absolute(self): self.settings.TEST = './test' - acctual = self.settings.TEST - self.assertEqual(acctual, './test') + actual = self.settings.TEST + self.assertEqual(actual, './test') From 7f4ce3be8aa3eea5d3785891ed4f19443e2f41b2 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 16:29:04 +0100 Subject: [PATCH 09/12] Fix SettingsProxy.__getattr__ to support settings that are None or 0. --- mopidy/utils/settings.py | 2 +- tests/utils/settings_test.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/mopidy/utils/settings.py b/mopidy/utils/settings.py index 2ec0f716..7715721e 100644 --- a/mopidy/utils/settings.py +++ b/mopidy/utils/settings.py @@ -49,7 +49,7 @@ class SettingsProxy(object): if attr not in self.current: raise SettingsError(u'Setting "%s" is not set.' % attr) value = self.current[attr] - if type(value) != bool and not value: + if isinstance(value, basestring) and len(value) == 0: raise SettingsError(u'Setting "%s" is empty.' % attr) if attr.endswith('_PATH') or attr.endswith('_FILE'): value = os.path.expanduser(value) diff --git a/tests/utils/settings_test.py b/tests/utils/settings_test.py index f7a95eaa..8e2575b9 100644 --- a/tests/utils/settings_test.py +++ b/tests/utils/settings_test.py @@ -1,7 +1,7 @@ import os import unittest -from mopidy import settings as default_settings_module +from mopidy import settings as default_settings_module, SettingsError from mopidy.utils.settings import validate_settings, SettingsProxy class ValidateSettingsTest(unittest.TestCase): @@ -55,6 +55,33 @@ class SettingsProxyTest(unittest.TestCase): self.settings.TEST = 'test' self.assertEqual(self.settings.TEST, 'test') + def test_getattr_raises_error_on_missing_setting(self): + try: + test = self.settings.TEST + self.fail(u'Should raise exception') + except SettingsError as e: + self.assertEqual(u'Setting "TEST" is not set.', e.message) + + def test_getattr_raises_error_on_empty_setting(self): + self.settings.TEST = u'' + try: + test = self.settings.TEST + self.fail(u'Should raise exception') + except SettingsError as e: + self.assertEqual(u'Setting "TEST" is empty.', e.message) + + def test_getattr_does_not_raise_error_if_setting_is_false(self): + self.settings.TEST = False + self.assertEqual(False, self.settings.TEST) + + def test_getattr_does_not_raise_error_if_setting_is_none(self): + self.settings.TEST = None + self.assertEqual(None, self.settings.TEST) + + def test_getattr_does_not_raise_error_if_setting_is_zero(self): + self.settings.TEST = 0 + self.assertEqual(0, self.settings.TEST) + def test_setattr_updates_runtime_settings(self): self.settings.TEST = 'test' self.assert_('TEST' in self.settings.runtime) From e1197ed84c4fc6667ea6d44725951164fd768021 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 16:31:35 +0100 Subject: [PATCH 10/12] Change default value of MPD_SERVER_PASSWORD from False to None --- mopidy/frontends/mpd/session.py | 2 +- mopidy/settings.py | 4 ++-- tests/frontends/mpd/connection_test.py | 2 +- tests/frontends/mpd/server_test.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/mopidy/frontends/mpd/session.py b/mopidy/frontends/mpd/session.py index b67e7f13..1161dac5 100644 --- a/mopidy/frontends/mpd/session.py +++ b/mopidy/frontends/mpd/session.py @@ -85,7 +85,7 @@ class MpdSession(asynchat.async_chat): the response_message is :class:`None`, normal processing should continue, even though the client may not be authenticated. """ - if not settings.MPD_SERVER_PASSWORD: + if settings.MPD_SERVER_PASSWORD is None: return (True, None) command = request.split(' ')[0] if command == 'password': diff --git a/mopidy/settings.py b/mopidy/settings.py index add32428..6e33ffaa 100644 --- a/mopidy/settings.py +++ b/mopidy/settings.py @@ -166,8 +166,8 @@ MPD_SERVER_HOSTNAME = u'127.0.0.1' #: The password required for connecting to the MPD server. #: -#: Default: :class:`False`, which means no password required. -MPD_SERVER_PASSWORD = False +#: Default: :class:`None`, which means no password required. +MPD_SERVER_PASSWORD = None #: Which TCP port Mopidy's MPD server should listen to. #: diff --git a/tests/frontends/mpd/connection_test.py b/tests/frontends/mpd/connection_test.py index e7bb74de..44ce78ca 100644 --- a/tests/frontends/mpd/connection_test.py +++ b/tests/frontends/mpd/connection_test.py @@ -36,7 +36,7 @@ class ConnectionHandlerTest(unittest.TestCase): self.assert_(u'ACK [3@0] {password} incorrect password' in result) def test_any_password_is_not_accepted_when_password_check_turned_off(self): - settings.MPD_SERVER_PASSWORD = False + settings.MPD_SERVER_PASSWORD = None result = self.h.handle_request(u'password "secret"') self.assert_(u'ACK [3@0] {password} incorrect password' in result) diff --git a/tests/frontends/mpd/server_test.py b/tests/frontends/mpd/server_test.py index 9d877ed5..48c7e790 100644 --- a/tests/frontends/mpd/server_test.py +++ b/tests/frontends/mpd/server_test.py @@ -44,7 +44,7 @@ class MpdSessionTest(unittest.TestCase): self.assertEqual(u'ACK [3@0] {password} incorrect password', response) def test_authentication_with_anything_when_password_check_turned_off(self): - settings.MPD_SERVER_PASSWORD = False + settings.MPD_SERVER_PASSWORD = None authed, response = self.session.check_password(u'any request at all') self.assertTrue(authed) self.assertEqual(None, response) From 55b32065a8dd32c5d3fa6660916c5f63fbd1a201 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 16:38:16 +0100 Subject: [PATCH 11/12] Expose MPD error codes as constants on the MpdAckError exception, to avoid use of magic values in the other exceptions --- mopidy/frontends/mpd/exceptions.py | 36 ++++++++++++++---------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/mopidy/frontends/mpd/exceptions.py b/mopidy/frontends/mpd/exceptions.py index 53a31dd9..faf4ce2f 100644 --- a/mopidy/frontends/mpd/exceptions.py +++ b/mopidy/frontends/mpd/exceptions.py @@ -1,22 +1,20 @@ from mopidy import MopidyException class MpdAckError(MopidyException): - """ - Available MPD error codes:: + """See fields on this class for available MPD error codes""" - ACK_ERROR_NOT_LIST = 1 - ACK_ERROR_ARG = 2 - ACK_ERROR_PASSWORD = 3 - ACK_ERROR_PERMISSION = 4 - ACK_ERROR_UNKNOWN = 5 - ACK_ERROR_NO_EXIST = 50 - ACK_ERROR_PLAYLIST_MAX = 51 - ACK_ERROR_SYSTEM = 52 - ACK_ERROR_PLAYLIST_LOAD = 53 - ACK_ERROR_UPDATE_ALREADY = 54 - ACK_ERROR_PLAYER_SYNC = 55 - ACK_ERROR_EXIST = 56 - """ + ACK_ERROR_NOT_LIST = 1 + ACK_ERROR_ARG = 2 + ACK_ERROR_PASSWORD = 3 + ACK_ERROR_PERMISSION = 4 + ACK_ERROR_UNKNOWN = 5 + ACK_ERROR_NO_EXIST = 50 + ACK_ERROR_PLAYLIST_MAX = 51 + ACK_ERROR_SYSTEM = 52 + ACK_ERROR_PLAYLIST_LOAD = 53 + ACK_ERROR_UPDATE_ALREADY = 54 + ACK_ERROR_PLAYER_SYNC = 55 + ACK_ERROR_EXIST = 56 def __init__(self, message=u'', error_code=0, index=0, command=u''): super(MpdAckError, self).__init__(message, error_code, index, command) @@ -37,24 +35,24 @@ class MpdAckError(MopidyException): class MpdArgError(MpdAckError): def __init__(self, *args, **kwargs): super(MpdArgError, self).__init__(*args, **kwargs) - self.error_code = 2 # ACK_ERROR_ARG + self.error_code = MpdAckError.ACK_ERROR_ARG class MpdPasswordError(MpdAckError): def __init__(self, *args, **kwargs): super(MpdPasswordError, self).__init__(*args, **kwargs) - self.error_code = 3 # ACK_ERROR_PASSWORD + self.error_code = MpdAckError.ACK_ERROR_PASSWORD class MpdUnknownCommand(MpdAckError): def __init__(self, *args, **kwargs): super(MpdUnknownCommand, self).__init__(*args, **kwargs) self.message = u'unknown command "%s"' % self.command self.command = u'' - self.error_code = 5 # ACK_ERROR_UNKNOWN + self.error_code = MpdAckError.ACK_ERROR_UNKNOWN class MpdNoExistError(MpdAckError): def __init__(self, *args, **kwargs): super(MpdNoExistError, self).__init__(*args, **kwargs) - self.error_code = 50 # ACK_ERROR_NO_EXIST + self.error_code = MpdAckError.ACK_ERROR_NO_EXIST class MpdNotImplemented(MpdAckError): def __init__(self, *args, **kwargs): From 1625c1e08022f856c2581262c094d5fdea843282 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Jan 2011 16:44:24 +0100 Subject: [PATCH 12/12] Be consistent in use of unicode string --- mopidy/frontends/mpd/session.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mopidy/frontends/mpd/session.py b/mopidy/frontends/mpd/session.py index 1161dac5..e8e3291d 100644 --- a/mopidy/frontends/mpd/session.py +++ b/mopidy/frontends/mpd/session.py @@ -96,5 +96,6 @@ class MpdSession(asynchat.async_chat): if command in ('close', 'commands', 'notcommands', 'ping'): return (False, None) else: - return (False, "ACK [4@0] {%s} " % command + - "you don't have permission for \"%s\"" % command) + return (False, + u'ACK [4@0] {%(c)s} you don\'t have permission for "%(c)s"' % + {'c': command})