Rework quoting around metadata

* Do not shell-quote strings in the metadata token expansion function
  * This fixes https://github.com/xiph/ezstream/issues/6
* Expand all tokens at the same time in a way that ensures metadata
  is not additional input to expansion
* Simplify several functions and remove unused functionality

src/ezstream.c

@@ -53,7 +53,7 @@ volatile sig_atomic_t	 queryMetadata;
 volatile sig_atomic_t	 quit;
 
 void		sig_handler(int);
-char *		buildReencodeCommand(const char *, const char *, metadata_t);
+char *		_build_reencode_cmd(const char *, const char *, metadata_t);
 metadata_t	getMetadata(const char *);
 FILE *		openResource(stream_t, const char *, int *, metadata_t *,
 			     int *, long *);
@@ -89,15 +89,19 @@ sig_handler(int sig)
 }
 
 char *
-buildReencodeCommand(const char *extension, const char *fileName,
+_build_reencode_cmd(const char *extension, const char *fileName,
     metadata_t mdata)
 {
-	cfg_decoder_t	 decoder;
+	cfg_decoder_t		 decoder;
-	cfg_encoder_t	 encoder;
+	cfg_encoder_t		 encoder;
-	char		*dec_str, *enc_str;
+	char			*artist, *album, *title, *songinfo, *tmp;
-	char		*commandString;
+	char			*filename_quoted;
-	size_t		 commandStringLen;
+	char			*custom_songinfo;
-	char		*localTitle, *localArtist, *localMetaString, *localAlbum;
+	struct util_dict	 dicts[6];
+	char			*dec_str;
+	char			*enc_str;
+	char			*cmd_str;
+	size_t			 cmd_str_size;
 
 	decoder = cfg_decoder_find(extension);
 	if (!decoder) {
@@ -112,124 +116,89 @@ buildReencodeCommand(const char *extension, const char *fileName,
 		return (NULL);
 	}
 
-	localTitle = util_utf82char(metadata_get_title(mdata), ICONV_REPLACE);
+	tmp = util_utf82char(metadata_get_artist(mdata));
-	localArtist = util_utf82char(metadata_get_artist(mdata), ICONV_REPLACE);
+	artist = util_shellquote(tmp);
-	localAlbum = util_utf82char(metadata_get_album(mdata), ICONV_REPLACE);
+	xfree(tmp);
-	localMetaString = util_utf82char(metadata_get_string(mdata),
+
-	    ICONV_REPLACE);
+	tmp = util_utf82char(metadata_get_album(mdata));
+	album = util_shellquote(tmp);
+	xfree(tmp);
+
+	tmp = util_utf82char(metadata_get_title(mdata));
+	title = util_shellquote(tmp);
+	xfree(tmp);
+
+	tmp = util_utf82char(metadata_get_string(mdata));
+	songinfo = util_shellquote(tmp);
+	xfree(tmp);
+
+	filename_quoted = util_shellquote(fileName);
 
-	dec_str = util_replacestring(cfg_decoder_get_program(decoder),
-	    PLACEHOLDER_TRACK, fileName);
-	if (strstr(dec_str, PLACEHOLDER_ARTIST) != NULL) {
-		char *tmpStr = util_replacestring(dec_str, PLACEHOLDER_ARTIST,
-		    localArtist);
-		xfree(dec_str);
-		dec_str = tmpStr;
-	}
-	if (strstr(dec_str, PLACEHOLDER_TITLE) != NULL) {
-		char *tmpStr = util_replacestring(dec_str, PLACEHOLDER_TITLE,
-		    localTitle);
-		xfree(dec_str);
-		dec_str = tmpStr;
-	}
-	if (strstr(dec_str, PLACEHOLDER_ALBUM) != NULL) {
-		char *tmpStr = util_replacestring(dec_str, PLACEHOLDER_ALBUM,
-		    localAlbum);
-		xfree(dec_str);
-		dec_str = tmpStr;
-	}
 	/*
-	 * if meta
+	 * if (prog && format)
-	 *   if (prog && format)
+	 *    metatoformat
-	 *      metatoformat
+	 * else
+	 *   if (!prog && title)
+	 *     emptymeta
 	 *   else
-	 *     if (!prog && title)
+	 *     replacemeta
-	 *       emptymeta
-	 *     else
-	 *       replacemeta
 	 */
-	if (strstr(dec_str, PLACEHOLDER_METADATA) != NULL) {
+	if (cfg_get_metadata_program() &&
-		if (cfg_get_metadata_program() &&
+	    cfg_get_metadata_format_str()) {
-		    cfg_get_metadata_format_str()) {
+		char	*utf8, *unquoted;
-			char *mdataString = metadata_format_string(mdata,
+
-			    cfg_get_metadata_format_str());
+		utf8 = metadata_format_string(mdata,
-			char *tmpStr = util_replacestring(dec_str,
+		    cfg_get_metadata_format_str());
-			    PLACEHOLDER_METADATA, mdataString);
+		unquoted = util_utf82char(utf8);
-			xfree(dec_str);
+		xfree(utf8);
-			xfree(mdataString);
+		custom_songinfo = util_shellquote(unquoted);
-			dec_str = tmpStr;
+		xfree(unquoted);
+	} else {
+		if (!cfg_get_metadata_program() &&
+		    strstr(cfg_decoder_get_program(decoder),
+			PLACEHOLDER_TITLE) != NULL) {
+			custom_songinfo = xstrdup("");
 		} else {
-			if (!cfg_get_metadata_program() &&
+			custom_songinfo = xstrdup(songinfo);
-			    strstr(dec_str, PLACEHOLDER_TITLE) != NULL) {
-				char *tmpStr = util_replacestring(dec_str,
-				    PLACEHOLDER_METADATA, "");
-				xfree(dec_str);
-				dec_str = tmpStr;
-			} else {
-				char *tmpStr = util_replacestring(dec_str,
-				    PLACEHOLDER_METADATA, localMetaString);
-				xfree(dec_str);
-				dec_str = tmpStr;
-			}
 		}
 	}
 
-	if (!cfg_encoder_get_program(encoder))
+	memset(dicts, 0, sizeof(dicts));
-		return (dec_str);
+	dicts[0].from = PLACEHOLDER_ARTIST;
+	dicts[0].to = artist;
+	dicts[1].from = PLACEHOLDER_ALBUM;
+	dicts[1].to = album;
+	dicts[2].from = PLACEHOLDER_TITLE;
+	dicts[2].to = title;
+	dicts[3].from = PLACEHOLDER_TRACK;
+	dicts[3].to = filename_quoted;
+	dicts[4].from = PLACEHOLDER_METADATA;
+	dicts[4].to = custom_songinfo;
 
-	enc_str = util_replacestring(cfg_encoder_get_program(encoder),
+	dec_str = util_expand_words(cfg_decoder_get_program(decoder), dicts);
-	    PLACEHOLDER_ARTIST, localArtist);
+
-	if (strstr(enc_str, PLACEHOLDER_TITLE) != NULL) {
+	if (!cfg_get_metadata_program() &&
-		char *tmpStr = util_replacestring(enc_str, PLACEHOLDER_TITLE,
+	    strstr(cfg_encoder_get_program(encoder),
-		    localTitle);
+		PLACEHOLDER_TITLE) != NULL) {
-		xfree(enc_str);
+		xfree(custom_songinfo);
-		enc_str = tmpStr;
+		dicts[4].to = custom_songinfo = xstrdup("");
-	}
-	if (strstr(enc_str, PLACEHOLDER_ALBUM) != NULL) {
-		char *tmpStr = util_replacestring(enc_str, PLACEHOLDER_ALBUM,
-		    localAlbum);
-		xfree(enc_str);
-		enc_str = tmpStr;
-	}
-	if (strstr(enc_str, PLACEHOLDER_METADATA) != NULL) {
-		if (cfg_get_metadata_program() &&
-		    cfg_get_metadata_format_str()) {
-			char *mdataString = metadata_format_string(mdata,
-			    cfg_get_metadata_format_str());
-			char *tmpStr = util_replacestring(enc_str,
-			    PLACEHOLDER_METADATA, mdataString);
-			xfree(enc_str);
-			xfree(mdataString);
-			enc_str = tmpStr;
-		} else {
-			if (!cfg_get_metadata_program() &&
-			    strstr(enc_str, PLACEHOLDER_TITLE) != NULL) {
-				char *tmpStr = util_replacestring(enc_str,
-				    PLACEHOLDER_METADATA, "");
-				xfree(enc_str);
-				enc_str = tmpStr;
-			} else {
-				char *tmpStr = util_replacestring(enc_str,
-				    PLACEHOLDER_METADATA, localMetaString);
-				xfree(enc_str);
-				enc_str = tmpStr;
-			}
-		}
 	}
 
-	commandStringLen = strlen(dec_str) + strlen(" | ") +
+	enc_str = util_expand_words(cfg_encoder_get_program(encoder), dicts);
-	    strlen(enc_str) + 1;
-	commandString = xcalloc(commandStringLen, sizeof(char));
-	snprintf(commandString, commandStringLen, "%s | %s", dec_str,
-	    enc_str);
 
-	xfree(localTitle);
+	cmd_str_size = strlen(dec_str) + strlen(" | ") + strlen(enc_str) + 1;
-	xfree(localArtist);
+	cmd_str = xcalloc(cmd_str_size, sizeof(char));
-	xfree(localMetaString);
+	snprintf(cmd_str, cmd_str_size, "%s | %s", dec_str, enc_str);
 	xfree(dec_str);
 	xfree(enc_str);
 
-	return (commandString);
+	xfree(artist);
+	xfree(album);
+	xfree(title);
+	xfree(filename_quoted);
+	xfree(custom_songinfo);
+
+	return (cmd_str);
 }
 
 metadata_t
@@ -325,7 +294,7 @@ openResource(stream_t stream, const char *fileName, int *popenFlag,
 	if (cfg_get_stream_encoder()) {
 		int	stderr_fd = -1;
 
-		pCommandString = buildReencodeCommand(extension, fileName,
+		pCommandString = _build_reencode_cmd(extension, fileName,
 		    mdata);
 		if (mdata_p != NULL)
 			*mdata_p = mdata;
@@ -588,7 +557,7 @@ streamFile(stream_t stream, const char *fileName)
 		char	*tmp, *metaData;
 
 		tmp = metadata_assemble_string(mdata);
-		if ((metaData = util_utf82char(tmp, ICONV_REPLACE)) == NULL)
+		if ((metaData = util_utf82char(tmp)) == NULL)
 			metaData = xstrdup("(unknown title)");
 		xfree(tmp);
 		log_notice("streaming: %s (%s)", metaData,

src/metadata.c

@@ -176,7 +176,7 @@ metadata_get_name(const char *file)
 	if (strlen(p1) == 0)
 		name = xstrdup("[unknown]");
 	else
-		name = util_char2utf8(p1, ICONV_REPLACE);
+		name = util_char2utf8(p1);
 
 	xfree(filename);
 	return (name);
@@ -371,16 +371,14 @@ metadata_program_update(struct metadata *md, enum metadata_request md_req)
 		return (0);
 	}
 
-	if (fgets(buf, (int)sizeof(buf), filep) == NULL) {
+	memset(buf, 0, sizeof(buf));
-		if (ferror(filep))
+	if (fgets(buf, (int)sizeof(buf), filep) == NULL &&
-			log_error("%s: output read error: %s", md->filename,
+	    ferror(filep)) {
-			    strerror(errno));
+		log_alert("%s: output read error: %s", md->filename,
+		    strerror(errno));
 		pclose(filep);
-		log_alert("program not (or no longer) usable: %s",
-		    md->filename);
 		exit(1);
 	}
-
 	pclose(filep);
 
 	if (strlen(buf) == sizeof(buf) - 1)
@@ -520,43 +518,22 @@ metadata_assemble_string(struct metadata *md)
 char *
 metadata_format_string(struct metadata *md, const char *format)
 {
-	char	*tmp, *str;
+	struct util_dict	 dicts[6];
 
 	if (format == NULL)
 		return (NULL);
 
-	str = xstrdup(format);
+	memset(dicts, 0, sizeof(dicts));
+	dicts[0].from = PLACEHOLDER_ARTIST;
+	dicts[0].to = metadata_get_artist(md);
+	dicts[1].from = PLACEHOLDER_ALBUM;
+	dicts[1].to = metadata_get_album(md);
+	dicts[2].from = PLACEHOLDER_TITLE;
+	dicts[2].to = metadata_get_title(md);
+	dicts[3].from = PLACEHOLDER_TRACK;
+	dicts[3].to = metadata_get_filename(md);
+	dicts[4].from = PLACEHOLDER_STRING;
+	dicts[4].to = metadata_get_string(md);
 
-	if (strstr(format, PLACEHOLDER_ARTIST) != NULL) {
+	return (util_expand_words(format, dicts));
-		tmp = util_replacestring(str, PLACEHOLDER_ARTIST,
-		    metadata_get_artist(md));
-		xfree(str);
-		str = tmp;
-	}
-	if (strstr(format, PLACEHOLDER_TITLE) != NULL) {
-		tmp = util_replacestring(str, PLACEHOLDER_TITLE,
-		    metadata_get_title(md));
-		xfree(str);
-		str = tmp;
-	}
-	if (strstr(format, PLACEHOLDER_STRING) != NULL) {
-		tmp = util_replacestring(str, PLACEHOLDER_STRING,
-		    metadata_get_string(md));
-		xfree(str);
-		str = tmp;
-	}
-	if (strstr(format, PLACEHOLDER_TRACK) != NULL) {
-		tmp = util_replacestring(str, PLACEHOLDER_TRACK,
-		    metadata_get_filename(md));
-		xfree(str);
-		str = tmp;
-	}
-	if (strstr(format, PLACEHOLDER_ALBUM) != NULL) {
-		tmp = util_replacestring(str, PLACEHOLDER_ALBUM,
-		    metadata_get_album(md));
-		xfree(str);
-		str = tmp;
-	}
-
-	return (str);
 }

src/util.c

@@ -53,11 +53,11 @@ static FILE		*pidfile_file;
 static pid_t		 pidfile_pid;
 static unsigned int	 pidfile_numlocks;
 
-static char *	_iconvert(const char *, const char *, const char *, int);
+static char *	_iconvert(const char *, const char *, const char *);
 static void	_cleanup_pidfile(void);
 
 static char *
-_iconvert(const char *in_str, const char *from, const char *to, int mode)
+_iconvert(const char *in_str, const char *from, const char *to)
 {
 #ifdef HAVE_ICONV
 	iconv_t 		 cd;
@@ -73,26 +73,7 @@ _iconvert(const char *in_str, const char *from, const char *to, int mode)
 	if (NULL == in_str)
 		return (xstrdup(""));
 
-	switch (mode) {
+	tocode = xstrdup(to);
-		size_t	siz;
-
-	case ICONV_TRANSLIT:
-		siz = strlen(to) + strlen("//TRANSLIT") + 1;
-		tocode = xcalloc(siz, sizeof(char));
-		snprintf(tocode, siz, "%s//TRANSLIT", to);
-		break;
-	case ICONV_IGNORE:
-		siz = strlen(to) + strlen("//IGNORE") + 1;
-		tocode = xcalloc(siz, sizeof(char));
-		snprintf(tocode, siz, "%s//IGNORE", to);
-		break;
-	case ICONV_REPLACE:
-		/* FALLTHROUGH */
-	default:
-		tocode = xstrdup(to);
-		break;
-	}
-
 	if ((cd = iconv_open(tocode, from)) == (iconv_t)-1 &&
 	    (cd = iconv_open("", from)) == (iconv_t)-1 &&
 	    (cd = iconv_open(tocode, "")) == (iconv_t)-1) {
@@ -147,7 +128,6 @@ _iconvert(const char *in_str, const char *from, const char *to, int mode)
 #else
 	(void)from;
 	(void)to;
-	(void)mode;
 
 	if (NULL == in_str)
 		return (xstrdup(""));
@@ -249,7 +229,7 @@ util_strrcasecmp(const char *s, const char *sub)
 }
 
 char *
-util_char2utf8(const char *in_str, int mode)
+util_char2utf8(const char *in_str)
 {
 	char	*codeset;
 
@@ -257,11 +237,11 @@ util_char2utf8(const char *in_str, int mode)
 	codeset = nl_langinfo((nl_item)CODESET);
 	setlocale(LC_CTYPE, "C");
 
-	return (_iconvert(in_str, codeset, "UTF-8", mode));
+	return (_iconvert(in_str, codeset, "UTF-8"));
 }
 
 char *
-util_utf82char(const char *in_str, int mode)
+util_utf82char(const char *in_str)
 {
 	char	*codeset;
 
@@ -269,32 +249,51 @@ util_utf82char(const char *in_str, int mode)
 	codeset = nl_langinfo((nl_item)CODESET);
 	setlocale(LC_CTYPE, "C");
 
-	return (_iconvert(in_str, "UTF-8", codeset, mode));
+	return (_iconvert(in_str, "UTF-8", codeset));
 }
 
 char *
-util_replacestring(const char *source, const char *from, const char *to)
+util_expand_words(const char *in, struct util_dict dicts[])
 {
-	char		*to_quoted, *dest;
+	size_t	 i;
-	size_t		 dest_size;
+	char	*out;
-	const char	*p1, *p2;
+	size_t	 out_size = strlen(in) + 1;
 
-	to_quoted = util_shellquote(to);
+	/* empty input string? */
-	dest_size = strlen(source) + strlen(to_quoted) + 1;
+	if (1 == out_size)
-	dest = xcalloc(dest_size, sizeof(char));
+		return (NULL);
 
-	p1 = source;
+	out = xstrdup(in);
-	p2 = strstr(p1, from);
+	i = out_size - 1;
-	if (p2 != NULL) {
+	while (i--) {
-		strncat(dest, p1, (size_t)(p2 - p1));
+		struct util_dict *d = dicts;
-		strlcat(dest, to_quoted, dest_size);
+
-		p1 = p2 + strlen(from);
+		while (d && d->from) {
+			if (0 == strncmp(&out[i], d->from, strlen(d->from))) {
+				char	*buf, *tmp;
+				size_t	 buf_len;
+
+				buf_len = strlen(&out[i]) + strlen(d->to)
+				    - strlen(d->from);
+				buf = xcalloc(buf_len + 1, sizeof(*buf));
+				snprintf(buf, buf_len + 1, "%s%s",
+				    d->to, &out[i + strlen(d->from)]);
+
+				out_size += buf_len;
+				tmp = xcalloc(out_size, sizeof(*tmp));
+				snprintf(tmp, i + 1, "%s", out);
+				snprintf(tmp + i, out_size - i, "%s", buf);
+				free(buf);
+				free(out);
+				out = tmp;
+
+				break;
+			}
+			d++;
+		}
 	}
-	strlcat(dest, p1, dest_size);
 
-	xfree(to_quoted);
+	return (out);
-
-	return (dest);
 }
 
 #define SHELLQUOTE_INLEN_MAX	8191UL

src/util.h

@@ -16,16 +16,17 @@
 #ifndef __UTIL_H__
 #define __UTIL_H__
 
-#define ICONV_REPLACE		0
+struct util_dict {
-#define ICONV_TRANSLIT		1
+	const char	*from;
-#define ICONV_IGNORE		2
+	const char	*to;
+};
 
 int	util_write_pid_file(const char *);
 int	util_strrcmp(const char *, const char *);
 int	util_strrcasecmp(const char *, const char *);
-char *	util_char2utf8(const char *, int);
+char *	util_char2utf8(const char *);
-char *	util_utf82char(const char *, int);
+char *	util_utf82char(const char *);
-char *	util_replacestring(const char *, const char *, const char *);
+char *	util_expand_words(const char *, struct util_dict[]);
 char *	util_shellquote(const char *);
 int	util_urlparse(const char *, char **, unsigned short *, char **);