andyetitmoves / libmpdee

An Emacs lisp client library for mpd
12 stars 7 forks source link

Quote strings containing a single quote #5

Closed dme closed 8 years ago

dme commented 8 years ago

In `mpd-safe-string', any strings that contain single quotes need to be wrapped in double quotes.

andyetitmoves commented 8 years ago

Thanks for your contribution, David! Could you point me to the protocol reference or code which indicates this? I admit I haven't been in close touch with the recent changes, but the request syntax still mentions that only space or tab would be used to delimit args, and if it shouldn't be, the argument should be in double quotes.

(Separately I see that we only escape when the string has a space, probably tab should be included)

dme commented 8 years ago

On Sat, Jan 16 2016, Ramkumar Aiyengar wrote:

Thanks for your contribution, David!

Thanks for the code!

Could you point me to the protocol reference or code which indicates this?

Well, that took a bit longer than I expected...

In mpd/src/util/Tokenizer.cxx the test for a valid unquoted character is defined as:

static inline bool valid_unquoted_char(char ch) { return (unsigned char)ch > 0x20 && ch != '"' && ch != '\''; }

I'm actually using mingus, which uses libmpdee to access the mpd server. mingus sends "lsinfo" commands to the server, and I can replicate the resulting problem by hand:

heart-of-gold ~/f % telnet localhost 6600 Trying ::1... Connected to localhost. Escape character is '^]'. OK MPD 0.19.0 lsinfo Sky/'cadmium...' ACK [2@0] {lsinfo} Invalid unquoted character lsinfo "Sky/'cadmium...'" file: Sky/'cadmium...'/01 Troika.mp3 Last-Modified: 2016-01-06T21:06:04Z Time: 179 Artist: Sky Album: 'cadmium...' Title: Troika Track: 01 Genre: Other Date: 1983 ...

I admit I haven't been in close touch with the recent changes, but the request syntax still mentions that only space or tab would be used to delimit args, and if it shouldn't be, the argument should be in double quotes.

I agree the specification reads that way, and I can't immediately see where mpc/libmpdclient perform the relevant quoting.

(Separately I see that we only escape when the string has a space, probably tab should be included)

dme commented 8 years ago

Ah, libmpdclient quotes all argument strings that it sends to the server.

andyetitmoves commented 8 years ago

Perhaps we should do the same thing? Going by the code above, looks like we should quote more things, including control characters. Also, at least from emacs, you could have other UTF-8 characters match this validation, and the server code appears to ignore this complication (unless the caller takes care of it, haven't looked).