From 7b32e917cba7c78d74297b2aefaf823ed262a921 Mon Sep 17 00:00:00 2001 Message-Id: <7b32e917cba7c78d74297b2aefaf823ed262a921.1715390346.git.mdw@distorted.org.uk> From: Mark Wooding Date: Fri, 21 Dec 2007 14:46:49 +0000 Subject: [PATCH] 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. Organization: Straylight/Edgeware From: Richard Kettlewell --- doc/disorder_protocol.5.in | 9 +++++-- lib/client.c | 48 +++++++++++++++++++++++++++----------- lib/eclient.c | 12 +++++++--- python/disorder.py.in | 15 +++++++----- server/server.c | 13 ++++++----- tests/dtest.py | 7 +++++- tests/dump.py | 13 ++++++----- 7 files changed, 80 insertions(+), 37 deletions(-) diff --git a/doc/disorder_protocol.5.in b/doc/disorder_protocol.5.in index 8cbbde7..fa98a81 100644 --- 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 e4792db..739fc20 100644 --- 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 2bf86e9..2d85fb9 100644 --- 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 7b888b5..650671d 100644 --- 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 b204513..7c0d3b3 100644 --- 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 edf793b..d4e3472 100644 --- 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 f3122c2..8401840 100755 --- 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, -- [mdw]