svn-1171 BUG w/ patch – Bad smart playlist causes segv

[kit]

If you create a smart playlist with invalid rules such as the following text :

title = “foo”
title = “foo”

The server seg faults. sp_get_error() is called, but in some cases no specific error value has been set. I don’t see a way to attach a patch directly to my message, so here it is in the body.

--- ../../orig/mt-daapd-svn-1171/src/smart-parser.c     2006-05-27 01:02:51.000000000 -0700

+++ smart-parser.c      2006-06-06 15:44:28.000000000 -0700

@@ -359,6 +359,7 @@

#define SP_E_TIMEINTERVAL  0x0b

#define SP_E_DATE          0x0c

#define SP_E_EXPRQUOTE     0x0d

+#define SP_E_UNKNOWN       0x0e



char *sp_errorstrings[] = {

"Success",

@@ -374,7 +375,8 @@

"Expecting interval comparison (before, after)",

"Expecting time interval (days, weeks, months, years)",

"Expecting date",

-    "Expecting ' (single quote)n"

+    "Expecting ' (single quote)n",

+    "Fatal parse errorn"

};



/* Forwards */

@@ -1478,7 +1480,15 @@

* @returns text of the last error

*/

char *sp_get_error(PARSETREE tree) {

-    return tree->error;

+    /* If no error explicitly, return unknown error instead of NULL.

+     * All of the calling functions assume != NULL return value.

+     */

+    if (tree->error == NULL) {

+        sp_set_error(tree, SP_E_UNKNOWN);

+        return(tree->error);

+    } else {

+        return tree->error;

+    }

}

Without this patch the segv results in the following stack trace :

(gdb) info stack

#0  0x009f7d23 in strdup () from /lib/libc.so.6

#1  0x0807bd57 in db_sql_parse_smart (pe=0x78a2830, clause=0x78a2834,

phrase=0x9cb2780 "title = "foo"ntitle = "foo"") at db-sql.c:279

#2  0x0807dfad in db_sql_add_playlist (pe=0x78a2b7c, name=0x9cb27f0 "Foo",

type=1, clause=0x9cb2780 "title = "foo"ntitle = "foo"", path=0x0,

index=0, playlistid=0x78a2b80) at db-sql.c:589

#3  0x08054f34 in db_add_playlist (pe=0x78a2b7c, name=0x9cb27f0 "Foo", type=1,

clause=0x9cb2780 "title = "foo"ntitle = "foo"", path=0x0, index=0,

playlistid=0x78a2b80) at db-generic.c:589

#4  0x08058c98 in daap_handler (pwsc=0x9cb3090) at dispatch.c:1030

#5  0x0804f162 in ws_dispatcher (arg=0x9cb3090) at webserver.c:986

#6  0x00b0a40b in start_thread () from /lib/libpthread.so.0

#7  0x00a56b7e in clone () from /lib/libc.so.6

[rpedde]

@kit wrote:

If you create a smart playlist with invalid rules such as the following text :

title = “foo”
title = “foo”

The server seg faults. sp_get_error() is called, but in some cases no specific error value has been set. I don’t see a way to attach a patch directly to my message, so here it is in the body.

Nice catch. It’s actually in sp_parse_phrase, when it token_id != T_EOF, and the error returned should be “Expecting end of statement” or something.

I’ve patched mine there, rather than at the get_error, but I’ve also added some defensive code similar to yours in case I forget another one. 🙂

Thanks. Bug reports with patches kick ass. 😛

[Author]

Posts