Quote responses. Unfortunately this is a major protocol change. I've

added a protocol generation field, to allow future such changes to be
handled more gracefuly, but 2.1 will therefore require all clients to
be upgraded too.

diff --git a/doc/disorder_protocol.5.in b/doc/disorder_protocol.5.in
index 8cbbde7a5960714fd99d4b0c77f9e76b7d3a1679..fa98a8144751c888e20cc688be0f78b498af16aa 100644 (file)
--- a/doc/disorder_protocol.5.in
+++ b/doc/disorder_protocol.5.in
@@ -339,10 +339,15 @@ part is commentary.
 .B 9
 The text part is just commentary (but would normally be a response for this
 command) e.g. \fBplaying\fR.
+.PP
+Result strings (not bodies) intended for machine parsing (i.e. xx1 and xx2
+responses) are quoted.
 .SH AUTHENTICATION
 When a connection is made the server sends a \fB231\fR response before any
-command is received.  This contains an algorithm name and a challenge encoded
-in hex.
+command is received.  This contains a protocol generation, an algorithm name
+and a challenge encoded in hex, all separated by whitespace.
+.PP
+The current protocol generation is \fB2\fR.
 .PP
 The possible algorithms are (currently) \fBsha1\fR, \fBsha256\fR, \fBsha384\fR
 and \fBsha512\fR.  \fBSHA1\fR etc work as synonyms.

diff --git a/lib/client.c b/lib/client.c
index e4792db0853760a512b6c5832cced4b93b3723b9..739fc20e1cac4a07878f04184c8b11d1ab90171d 100644 (file)
--- a/lib/client.c
+++ b/lib/client.c
@@ -217,7 +217,7 @@ int disorder_connect_sock(disorder_client *c,
   size_t nl;
   const char *res;
   char *r, **rvec;
-  const char *algo = "SHA1";
+  const char *protocol, *algorithm, *challenge;
 
   if(!password) {
     error(0, "no password found");
@@ -251,15 +251,20 @@ int disorder_connect_sock(disorder_client *c,
     return -1;
   if(!(rvec = split(r, &nrvec, SPLIT_QUOTES, 0, 0)))
     return -1;
-  if(nrvec > 1) {
-    algo = *rvec++;
-    --nrvec;
+  if(nrvec != 3) {
+    error(0, "cannot parse server greeting %s", r);
+    return -1;
   }
-  if(!nrvec)
+  protocol = *rvec++;
+  if(strcmp(protocol, "2")) {
+    error(0, "unknown protocol version: %s", protocol);
     return -1;
-  if(!(nonce = unhex(*rvec, &nl)))
+  }
+  algorithm = *rvec++;
+  challenge = *rvec++;
+  if(!(nonce = unhex(challenge, &nl)))
     return -1;
-  if(!(res = authhash(nonce, nl, password, algo))) goto error;
+  if(!(res = authhash(nonce, nl, password, algorithm))) goto error;
   if(disorder_simple(c, 0, "user", username, res, (char *)0))
     return -1;
   c->user = xstrdup(username);
@@ -337,8 +342,20 @@ int disorder_rescan(disorder_client *c) {
   return disorder_simple(c, 0, "rescan", (char *)0);
 }
 
+static int dequote(int rc, char **rp) {
+  char **rr;
+  if(!rc){
+    if((rr = split(*rp, 0, SPLIT_QUOTES, 0, 0)) && *rr) {
+      *rp = *rr;
+      return 0;
+    }
+    error(0, "invalid reply: %s", *rp);
+  }
+  return -1;
+}
+
 int disorder_version(disorder_client *c, char **rp) {
-  return disorder_simple(c, rp, "version", (char *)0);
+  return dequote(disorder_simple(c, rp, "version", (char *)0), rp);
 }
 
 static void client_error(const char *msg,
@@ -462,7 +479,8 @@ int disorder_unset(disorder_client *c, const char *track,
 
 int disorder_get(disorder_client *c,
                 const char *track, const char *key, char **valuep) {
-  return disorder_simple(c, valuep, "get", track, key, (char *)0);
+  return dequote(disorder_simple(c, valuep, "get", track, key, (char *)0),
+                valuep);
 }
 
 static void pref_error_handler(const char *msg,
@@ -585,11 +603,13 @@ int disorder_log(disorder_client *c, struct sink *s) {
 
 int disorder_part(disorder_client *c, char **partp,
                  const char *track, const char *context, const char *part) {
-  return disorder_simple(c, partp, "part", track, context, part, (char *)0);
+  return dequote(disorder_simple(c, partp, "part",
+                                track, context, part, (char *)0), partp);
 }
 
 int disorder_resolve(disorder_client *c, char **trackp, const char *track) {
-  return disorder_simple(c, trackp, "resolve", track, (char *)0);
+  return dequote(disorder_simple(c, trackp, "resolve", track, (char *)0),
+                trackp);
 }
 
 int disorder_pause(disorder_client *c) {
@@ -636,7 +656,8 @@ int disorder_unset_global(disorder_client *c, const char *key) {
 }
 
 int disorder_get_global(disorder_client *c, const char *key, char **valuep) {
-  return disorder_simple(c, valuep, "get-global", key, (char *)0);
+  return dequote(disorder_simple(c, valuep, "get-global", key, (char *)0),
+                valuep);
 }
 
 int disorder_rtp_address(disorder_client *c, char **addressp, char **portp) {
@@ -667,7 +688,8 @@ int disorder_deluser(disorder_client *c, const char *user) {
 
 int disorder_userinfo(disorder_client *c, const char *user, const char *key,
                      char **valuep) {
-  return disorder_simple(c, valuep, "userinfo", user, key, (char *)0);
+  return dequote(disorder_simple(c, valuep, "userinfo", user, key, (char *)0),
+                valuep);
 }
 
 int disorder_edituser(disorder_client *c, const char *user,

diff --git a/lib/eclient.c b/lib/eclient.c
index 2bf86e9dff3192863aa72212c2dced66b7e1c733..2d85fb92233696dcb3122f72e9d169670b3a9c3e 100644 (file)
--- a/lib/eclient.c
+++ b/lib/eclient.c
@@ -819,13 +819,19 @@ static void stash_command(disorder_eclient *c,
 
 /* Command support ***********************************************************/
 
-/* for commands with a simple string response */ 
+/* for commands with a quoted string response */ 
 static void string_response_opcallback(disorder_eclient *c,
                                        struct operation *op) {
   D(("string_response_callback"));
   if(c->rc / 100 == 2) {
-    if(op->completed)
-      ((disorder_eclient_string_response *)op->completed)(op->v, c->line + 4);
+    if(op->completed) {
+      char **rr = split(c->line + 4, 0, SPLIT_QUOTES, 0, 0);
+
+      if(rr && *rr)
+        ((disorder_eclient_string_response *)op->completed)(op->v, *rr);
+      else
+        protocol_error(c, op, c->rc, "%s: %s", c->ident, c->line);
+    }
   } else
     protocol_error(c, op, c->rc, "%s: %s", c->ident, c->line);
 }

diff --git a/python/disorder.py.in b/python/disorder.py.in
index 7b888b5455bed96505e268c9f4c66949a39cc46a..650671d506c818ee248034ae2c5f922e2af49860 100644 (file)
--- a/python/disorder.py.in
+++ b/python/disorder.py.in
@@ -375,8 +375,11 @@ class client:
           s.connect(self.who)
         self.w = s.makefile("wb")
         self.r = s.makefile("rb")
-        (res, challenge_and_algo) = self._simple()
-        (algo, challenge) = _split(challenge_and_algo)
+        (res, details) = self._simple()
+        (protocol, algo, challenge) = _split(details)
+        if protocol != '2':
+          raise communicationError(self.who,
+                                   "unknown protocol version %s" % protocol)
         if cookie is None:
           if self.user is None:
             user = self.config['username']
@@ -488,7 +491,7 @@ class client:
 
   def version(self):
     """Return the server's version number."""
-    return self._simple("version")[1]
+    return _split(self._simple("version")[1])[0]
 
   def playing(self):
     """Return the currently playing track.
@@ -619,7 +622,7 @@ class client:
     if ret == 555:
       return None
     else:
-      return details
+      return _split(details)[0]
 
   def prefs(self, track):
     """Get all the preferences for a track.
@@ -810,7 +813,7 @@ class client:
     The return value is the preference 
     """
     ret, details = self._simple("part", track, context, part)
-    return details
+    return _split(details)[0]
 
   def setglobal(self, key, value):
     """Set a global preference value.
@@ -841,7 +844,7 @@ class client:
     if ret == 555:
       return None
     else:
-      return details
+      return _split(details)[0]
 
   def make_cookie(self):
     """Create a login cookie"""

diff --git a/server/server.c b/server/server.c
index b2045131c0f8ea5357b8a6ca75e7ea7226b378ae..7c0d3b39cd6908f9e7320200ef363d2e080dc135 100644 (file)
--- a/server/server.c
+++ b/server/server.c
@@ -607,7 +607,7 @@ static int c_get(struct conn *c,
   const char *v;
 
   if(vec[1][0] != '_' && (v = trackdb_get(vec[0], vec[1])))
-    sink_printf(ev_writer_sink(c->w), "252 %s\n", v);
+    sink_printf(ev_writer_sink(c->w), "252 %s\n", quoteutf8(v));
   else
     sink_writes(ev_writer_sink(c->w), "555 not found\n");
   return 1;
@@ -623,7 +623,7 @@ static int c_length(struct conn *c,
     return 1;
   }
   if((v = trackdb_get(track, "_length")))
-    sink_printf(ev_writer_sink(c->w), "252 %s\n", v);
+    sink_printf(ev_writer_sink(c->w), "252 %s\n", quoteutf8(v));
   else
     sink_writes(ev_writer_sink(c->w), "550 not found\n");
   return 1;
@@ -925,7 +925,7 @@ static int c_part(struct conn *c,
                  char **vec,
                  int attribute((unused)) nvec) {
   sink_printf(ev_writer_sink(c->w), "252 %s\n",
-             trackdb_getpart(vec[0], vec[1], vec[2]));
+             quoteutf8(trackdb_getpart(vec[0], vec[1], vec[2])));
   return 1;
 }
 
@@ -938,7 +938,7 @@ static int c_resolve(struct conn *c,
     sink_writes(ev_writer_sink(c->w), "550 cannot resolve track\n");
     return 1;
   }
-  sink_printf(ev_writer_sink(c->w), "252 %s\n", track);
+  sink_printf(ev_writer_sink(c->w), "252 %s\n", quoteutf8(track));
   return 1;
 }
 
@@ -975,7 +975,7 @@ static int c_get_global(struct conn *c,
   const char *s = trackdb_get_global(vec[0]);
 
   if(s)
-    sink_printf(ev_writer_sink(c->w), "252 %s\n", s);
+    sink_printf(ev_writer_sink(c->w), "252 %s\n", quoteutf8(s));
   else
     sink_writes(ev_writer_sink(c->w), "555 not found\n");
   return 1;
@@ -1352,7 +1352,8 @@ static int listen_callback(ev_source *ev,
   c->l = l;
   c->rights = 0;
   gcry_randomize(c->nonce, sizeof c->nonce, GCRY_STRONG_RANDOM);
-  sink_printf(ev_writer_sink(c->w), "231 %s %s\n",
+  sink_printf(ev_writer_sink(c->w), "231 %d %s %s\n",
+             2,
              config->authorization_algorithm,
              hex(c->nonce, sizeof c->nonce));
   return 0;

diff --git a/tests/dtest.py b/tests/dtest.py
index edf793b7fe1133d2a1b72aad31e359783ece082a..d4e3472b54693613cfa505e13efb1286591c100f 100644 (file)
--- a/tests/dtest.py
+++ b/tests/dtest.py
@@ -307,7 +307,12 @@ def run(module=None, report=True):
         name = module.__name__
     # Open the error log
     global errs
-    errs = open("%s.log" % name, "w")
+    logfile = "%s.log" % name
+    try:
+        os.remove(logfile)
+    except:
+        pass
+    errs = open(logfile, "a")
     # Ensure that disorder.py uses the test installation
     disorder._configfile = "%s/config" % testroot
     disorder._userconf = False

diff --git a/tests/dump.py b/tests/dump.py
index f3122c237e4399f4897ac8380aa9ddeaa43b658f..84018404802b493a6a672ad3aab40d13125c08d4 100755 (executable)
--- a/tests/dump.py
+++ b/tests/dump.py
@@ -54,15 +54,16 @@ def test():
     print dtest.command(["disorder-dump", "--config", disorder._configfile,
                          "--dump", dump])
     print " changing track pref"
-    c.set(track, "foo", "after");
-    assert c.get(track, "foo") == "after", "checking track foo=before"
+    c.set(track, "foo", "after dump");
+    print c.get(track, "foo")
+    assert c.get(track, "foo") == "after dump", "checking track foo=after dump"
     print " changing global pref"
-    c.setglobal("foo", "after");
-    assert c.getglobal("foo") == "after", "checking global foo=before"
+    c.setglobal("foo", "after dump");
+    assert c.getglobal("foo") == "after dump", "checking global foo=after dump"
     print " adding fresh track pref"
-    c.set(track, "bar", "after")
+    c.set(track, "bar", "after dump")
     print " adding fresh global pref"
-    c.setglobal("bar", "after")
+    c.setglobal("bar", "after dump")
     dtest.stop_daemon();
     print "restoring database"
     print dtest.command(["disorder-dump", "--config", disorder._configfile,