From ee0c31ba6a3b00a671a8c2af1b5934af89d2f3df Mon Sep 17 00:00:00 2001
From: Carl Suster <carl@contraflo.ws>
Date: Thu, 4 Apr 2019 17:50:21 +1100
Subject: [PATCH] bpd: track and log client session details

Keep track of a list of currently-connected clients.

Use `socket.getpeername()` to get an identifier for each connection and
include this in each log message. This function is documented as not
being available on all systems, but it's unclear which systems this
involves.

Also log a message on client connect and disconnect events. If the
disconnection reason is because the client sent a blank line, match MPD
by returning a protocol error then hanging up. Escape curly braces.
---
 beetsplug/bpd/__init__.py | 39 +++++++++++++++++++++++++++++++++------
 test/test_player.py       |  5 +++++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py
index dc7f64db7..153fd675e 100644
--- a/beetsplug/bpd/__init__.py
+++ b/beetsplug/bpd/__init__.py
@@ -187,9 +187,22 @@ class BaseServer(object):
         self.paused = False
         self.error = None
 
+        # Current connections
+        self.connections = set()
+
         # Object for random numbers generation
         self.random_obj = random.Random()
 
+    def connect(self, conn):
+        """A new client has connected.
+        """
+        self.connections.add(conn)
+
+    def disconnect(self, conn):
+        """Client has disconnected; clean up residual state.
+        """
+        self.connections.remove(conn)
+
     def run(self):
         """Block and start listening for connections from clients. An
         interrupt (^C) closes the server.
@@ -643,6 +656,7 @@ class Connection(object):
         self.server = server
         self.sock = sock
         self.authenticated = False
+        self.address = u'{}:{}'.format(*sock.sock.getpeername())
 
     def send(self, lines):
         """Send lines, which which is either a single string or an
@@ -653,9 +667,9 @@ class Connection(object):
         if isinstance(lines, six.string_types):
             lines = [lines]
         out = NEWLINE.join(lines) + NEWLINE
-        # Don't log trailing newline:
-        message = out[:-1].replace(u'\n', u'\n' + u' ' * 13)
-        self.server._log.debug('server: {}', message)
+        session = u'>[{}]: '.format(self.address)
+        for l in out.split(NEWLINE)[:-1]:
+            self.server._log.debug('{}', session + l)
         if isinstance(out, six.text_type):
             out = out.encode('utf-8')
         return self.sock.sendall(out)
@@ -672,24 +686,36 @@ class Connection(object):
             # Send success code.
             yield self.send(RESP_OK)
 
+    def disconnect(self):
+        """The connection has closed for any reason.
+        """
+        self.server.disconnect(self)
+        self.server._log.debug(u'*[{}]: disconnected', self.address)
+
     def run(self):
         """Send a greeting to the client and begin processing commands
         as they arrive.
         """
-        self.server._log.debug('New client connected')
+        self.server._log.debug(u'*[{}]: connected', self.address)
+        self.server.connect(self)
         yield self.send(HELLO)
 
+        session = u'<[{}]: '.format(self.address)
         clist = None  # Initially, no command list is being constructed.
         while True:
             line = yield self.sock.readline()
             if not line:
+                self.disconnect()  # Client disappeared.
                 break
             line = line.strip()
             if not line:
+                err = BPDError(ERROR_UNKNOWN, u'No command given')
+                yield self.send(err.response())
+                self.disconnect()  # Client sent a blank line.
                 break
             line = line.decode('utf8')  # MPD protocol uses UTF-8.
-            message = line.replace(u'\n', u'\n' + u' ' * 13)
-            self.server._log.debug(u'client: {}', message)
+            for l in line.split(NEWLINE):
+                self.server._log.debug('{}', session + l)
 
             if clist is not None:
                 # Command list already opened.
@@ -710,6 +736,7 @@ class Connection(object):
                 except BPDClose:
                     # Command indicates that the conn should close.
                     self.sock.close()
+                    self.disconnect()  # Client explicitly closed.
                     return
 
     @classmethod
diff --git a/test/test_player.py b/test/test_player.py
index aa3c3d6a8..baf1ddfdb 100644
--- a/test/test_player.py
+++ b/test/test_player.py
@@ -367,6 +367,11 @@ class BPDTest(BPDTestHelper):
             response = client.send_command('crash_TypeError')
         self._assert_failed(response, bpd.ERROR_SYSTEM)
 
+    def test_empty_request(self):
+        with self.run_bpd() as client:
+            response = client.send_command('')
+        self._assert_failed(response, bpd.ERROR_UNKNOWN)
+
 
 class BPDQueryTest(BPDTestHelper):
     test_implements_query = implements({