bubble parse errors up to the web interface

src/db-sql.c

@@ -62,8 +62,7 @@ int db_sql_build_dmap(DBQUERYINFO *pinfo, char **valarray, unsigned char *presul
 void db_sql_build_mp3file(char **valarray, MP3FILE *pmp3);
 int db_sql_update(char **pe, MP3FILE *pmp3, int *id);
 int db_sql_update_playlists(char **pe);
-char *db_sql_parse_smart(char *phrase);
+int db_sql_parse_smart(char **pe, char **clause, char *phrase);
-
 
 #define STR(a) (a) ? (a) : ""
 #define ISSTR(a) ((a) && strlen((a)))
@@ -223,29 +222,35 @@ int db_sql_dispose_row(void) {
  * @param phrase playlist spec to be converted
  * @returns sql where clause if successful, NULL otherwise
  */
-char *db_sql_parse_smart(char *phrase) {
+int db_sql_parse_smart(char **pe, char **clause, char *phrase) {
     PARSETREE pt;
-    char *result = NULL;
 
-    if(strcmp(phrase,"1") == 0)
+    if(strcmp(phrase,"1") == 0) {
-        return strdup("1");
+        *clause = strdup("1");
-
+        return TRUE;
-    pt=sp_init();
-    if(!pt)
-        return NULL;
-
-    if(!sp_parse(pt,phrase)) {
-        DPRINTF(E_LOG,L_DB,"Error parsing smart playlist: %s\n",sp_get_error(pt));
-        sp_dispose(pt);
-        return strdup("0");
-    } else {
-        result = sp_sql_clause(pt);
     }
-
+    
+    pt=sp_init();
+    if(!pt) {
+        if(pe) *pe = strdup("Could not initialize parse tree");
+        return FALSE;
+    }
+    
+    if(!sp_parse(pt,phrase)) {
+        if(pe) *pe = strdup(sp_get_error(pt));
+	
+        DPRINTF(E_LOG,L_DB,"Error parsing playlist: %s\n",sp_get_error(pt));
+	
+        sp_dispose(pt);
+        return FALSE;
+    } else {
+        *clause = sp_sql_clause(pt);
+    }
+    
     sp_dispose(pt);
-    return result;
+    return TRUE;
 }
-
+ 
 /**
  * open sqlite database
  *
@@ -478,17 +483,18 @@ int db_sql_add_playlist(char **pe, char *name, int type, char *clause, char *pat
     int cnt=0;
     int result=DB_E_SUCCESS;
     char *criteria;
+    char *estring;
 
     result=db_sql_fetch_int(pe,&cnt,"select count(*) from playlists where "
                             "upper(title)=upper('%q')",name);
 
     if(result != DB_E_SUCCESS) {
-	return result;
+        return result;
     }
 
     if(cnt != 0) { /* duplicate */
-	db_get_error(pe,DB_E_DUPLICATE_PLAYLIST,name);
+        db_get_error(pe,DB_E_DUPLICATE_PLAYLIST,name);
-	return DB_E_DUPLICATE_PLAYLIST;
+        return DB_E_DUPLICATE_PLAYLIST;
     }
 
     if((type == PL_SMART) && (!clause)) {
@@ -507,9 +513,9 @@ int db_sql_add_playlist(char **pe, char *name, int type, char *clause, char *pat
                                  name,type,time(NULL),path,index);
         break;
     case PL_SMART: /* smart */
-        criteria = db_sql_parse_smart(clause);
+        if(!db_sql_parse_smart(&estring,&criteria,clause)) {
-        if(!criteria) {
+            db_get_error(pe,DB_E_PARSE,estring);
-            db_get_error(pe,DB_E_PARSE);
+            free(estring);
             return DB_E_PARSE;
         }
         free(criteria);
@@ -713,7 +719,7 @@ int db_sql_add(char **pe, MP3FILE *pmp3, int *id) {
     insertid = db_sql_insert_id_fn();
     if((db_sql_in_scan)&&(!db_sql_reload)) {
         db_sql_exec_fn(NULL,E_FATAL,"insert into updated values (%d)",
-		       insertid);
+                       insertid);
     }
 
     if((!db_sql_in_scan) && (!db_sql_in_playlist_scan))
@@ -739,7 +745,7 @@ int db_sql_update(char **pe, MP3FILE *pmp3, int *id) {
 
     pmp3->db_timestamp = (int)time(NULL);
 
-	/* FIXME: this should update all fields */
+        /* FIXME: this should update all fields */
     err=db_sql_exec_fn(pe,E_LOG,"UPDATE songs SET "
                         "title='%q',"  // title
                         "artist='%q',"  // artist
@@ -882,10 +888,14 @@ int db_sql_update_playlists(char **pe) {
     for(index=0;index < playlists; index++) {
         if(atoi(pinfo[index].type) == 1) {
             /* smart */
-            where_clause = db_sql_parse_smart(pinfo[index].clause);
+            if(!db_sql_parse_smart(NULL,&where_clause,pinfo[index].clause)) {
+                DPRINTF(E_LOG,L_DB,"Playlist %d bad syntax",pinfo[index].plid);
+                where_clause = strdup("0");
+            }
             db_sql_exec_fn(NULL,E_FATAL,"update playlists set items=("
                             "select count(*) from songs where %s) "
                             "where id=%s",where_clause,pinfo[index].plid);
+            free(where_clause);
         } else {
             db_sql_exec_fn(NULL,E_FATAL,"update playlists set items=("
                             "select count(*) from playlistitems where "
@@ -962,12 +972,15 @@ int db_sql_enum_start(char **pe, DBQUERYINFO *pinfo) {
         is_smart=(atoi(temprow[0]) == 1);
         have_clause=1;
         if(is_smart) {
-            where_clause=db_sql_parse_smart(temprow[1]);
+            if(!db_sql_parse_smart(NULL,&where_clause,temprow[1]))
+                where_clause = strdup("0");
+                
             if(!where_clause) {
                 db_sql_enum_end_fn(NULL);
                 db_get_error(pe,DB_E_PARSE);
                 return DB_E_PARSE;
             }
+            
             sprintf(query_select,"SELECT * FROM songs ");
             sprintf(query_count,"SELECT COUNT(id) FROM songs ");
             sprintf(query_rest,"WHERE (%s)",where_clause);
@@ -1341,9 +1354,9 @@ int db_sql_get_size(DBQUERYINFO *pinfo, SQL_ROW valarray) {
             /* ascr */
             size += 9;
         if(db_wantsmeta(pinfo->meta,metaItunesHasVideo))
-        	/* aeHV */
+                /* aeHV */
-        	size += 9;
+                size += 9;
-        	
+                
         return size;
         break;
 
@@ -1480,7 +1493,7 @@ int db_sql_build_dmap(DBQUERYINFO *pinfo, char **valarray, unsigned char *presul
         if(db_wantsmeta(pinfo->meta, metaItunesHasVideo))
             current += db_dmap_add_char(current,"aeHV",atoi(valarray[39]));
         if(db_wantsmeta(pinfo->meta, metaSongContentRating))
-        	current += db_dmap_add_char(current,"ascr",atoi(valarray[40]));
+                current += db_dmap_add_char(current,"ascr",atoi(valarray[40]));
         return 0;
         break;
 

src/dispatch.c

@@ -60,7 +60,7 @@ static void dispatch_deleteplaylist(WS_CONNINFO *pwsc, DBQUERYINFO *pqi);
 static void dispatch_deleteplaylistitems(WS_CONNINFO *pwsc, DBQUERYINFO *pqi);
 static void dispatch_items(WS_CONNINFO *pwsc, DBQUERYINFO *pqi);
 static void dispatch_logout(WS_CONNINFO *pwsc, DBQUERYINFO *pqi);
-
+static void dispatch_error(WS_CONNINFO *pwsc, DBQUERYINFO *pqi, char *container, char *error);
 static int dispatch_output_start(WS_CONNINFO *pwsc, DBQUERYINFO *pqi, int content_length);
 static int dispatch_output_write(WS_CONNINFO *pwsc, DBQUERYINFO *pqi, unsigned char *block, int len);
 static int dispatch_output_end(WS_CONNINFO *pwsc, DBQUERYINFO *pqi);
@@ -938,11 +938,12 @@ void dispatch_addplaylist(WS_CONNINFO *pwsc, DBQUERYINFO *pqi) {
     char *name, *query;
     int type;
     int retval, playlistid;
+    char *estring = NULL;
 
     if((!ws_getvar(pwsc,"org.mt-daapd.playlist-type")) ||
        (!ws_getvar(pwsc,"dmap.itemname"))) {
         DPRINTF(E_LOG,L_DAAP,"attempt to add playlist with invalid type\n");
-        ws_returnerror(pwsc,500,"bad playlist info specified");
+        dispatch_error(pwsc,pqi,"MAPR","bad playlist info specified");
         return;
     }
 
@@ -950,11 +951,11 @@ void dispatch_addplaylist(WS_CONNINFO *pwsc, DBQUERYINFO *pqi) {
     name=ws_getvar(pwsc,"dmap.itemname");
     query=ws_getvar(pwsc,"org.mt-daapd.smart-playlist-spec");
 
-    /* FIXME: Error handling */
+    retval=db_add_playlist(&estring,name,type,query,NULL,0,&playlistid);
-    retval=db_add_playlist(NULL,name,type,query,NULL,0,&playlistid);
     if(retval != DB_E_SUCCESS) {
-        DPRINTF(E_LOG,L_DAAP,"error adding playlist.  aborting\n");
+        dispatch_error(pwsc,pqi,"MAPR",estring);
-        ws_returnerror(pwsc,500,"error adding playlist");
+        DPRINTF(E_LOG,L_DAAP,"error adding playlist %s: %s\n",name,estring);
+        free(estring);
         return;
     }
 
@@ -968,7 +969,6 @@ void dispatch_addplaylist(WS_CONNINFO *pwsc, DBQUERYINFO *pqi) {
     dispatch_output_end(pwsc,pqi);
 
     pwsc->close=1;
-
     return;
 }
 
@@ -1434,3 +1434,33 @@ void dispatch_server_info(WS_CONNINFO *pwsc, DBQUERYINFO *pqi) {
     return;
 }
 
+/**
+ * throw out an error, xml style.  This throws  out a dmap block, but with a
+ * mstt of 500, and a msts as specified
+ */
+void dispatch_error(WS_CONNINFO *pwsc, DBQUERYINFO *pqi, char *container, char *error) {
+    unsigned char *block, *current;
+    int len;
+    
+    len = 12 + 8 + 8 + strlen(error);
+    block = (unsigned char *)malloc(len);
+    
+    if(!block)
+        DPRINTF(E_FATAL,L_DAAP,"Malloc error\n");
+    
+    current = block;
+    current += db_dmap_add_container(current,container,len - 8);
+    current += db_dmap_add_int(current,"mstt",500);
+    current += db_dmap_add_string(current,"msts",error);
+    
+    dispatch_output_start(pwsc,pqi,len);
+    dispatch_output_write(pwsc,pqi,block,len);
+    dispatch_output_end(pwsc,pqi);
+    
+    free(block);
+    
+    pwsc->close=1;
+}
+
+
+