Introduce a new WPS parsing error case: limits exceeded. It includes the cases when there are too many tokens, lines, sublines, viewports, strings, characters or conditional levels. This prevents the parser from failing silently or going on, as it used to do in those cases. Thanks to fml (Alexander Levin) for mentioning this issue.

I also changed the error types from #defines to an enum, for cleanliness.


git-svn-id: svn://svn.rockbox.org/rockbox/trunk@18015 a1c6a512-1295-4272-9138-f99709370657

3 files changed, 53 insertions, 41 deletions

diff --git a/apps/gui/gwps.h b/apps/gui/gwps.h
index 69a84f915b..99bf701a7d 100644
--- a/apps/gui/gwps.h
+++ b/apps/gui/gwps.h
@@ -126,6 +126,15 @@ struct align_pos {
                                 (1/HZ sec, or 100ths of sec) */
 #define SUBLINE_RESET -1
 
+enum wps_parse_error {
+    PARSE_OK,
+    PARSE_FAIL_UNCLOSED_COND,
+    PARSE_FAIL_INVALID_CHAR,
+    PARSE_FAIL_COND_SYNTAX_ERROR,
+    PARSE_FAIL_COND_INVALID_PARAM,
+    PARSE_FAIL_LIMITS_EXCEEDED,
+};
+
 enum wps_token_type {
     WPS_NO_TOKEN,   /* for WPS tags we don't want to save as tokens */
     WPS_TOKEN_UNKNOWN,
diff --git a/apps/gui/wps_debug.c b/apps/gui/wps_debug.c
index 2295b4db6e..4e6af388ac 100644
--- a/apps/gui/wps_debug.c
+++ b/apps/gui/wps_debug.c
@@ -30,11 +30,6 @@
 #include "debug.h"
 #endif
 
-#define PARSE_FAIL_UNCLOSED_COND        1
-#define PARSE_FAIL_INVALID_CHAR         2
-#define PARSE_FAIL_COND_SYNTAX_ERROR    3
-#define PARSE_FAIL_COND_INVALID_PARAM   4
-
 #if defined(SIMULATOR) || defined(__PCTOOL__)
 extern bool debug_wps;
 extern int wps_verbose_level;
@@ -574,7 +569,7 @@ static void print_wps_strings(struct wps_data *data)
 }
 #endif
 
-void print_debug_info(struct wps_data *data, int fail, int line)
+void print_debug_info(struct wps_data *data, enum wps_parse_error fail, int line)
 {
 #if defined(SIMULATOR) || defined(__PCTOOL__)
     if (debug_wps && wps_verbose_level)
@@ -590,13 +585,16 @@ void print_debug_info(struct wps_data *data, int fail, int line)
                WPS_MAX_TOKENS - 1);
     }
 
-    if (fail)
+    if (fail != PARSE_OK)
     {
         char buf[64];
 
         DEBUGF("Failed parsing on line %d : ", line);
         switch (fail)
         {
+            case PARSE_OK:
+                break;
+                
             case PARSE_FAIL_UNCLOSED_COND:
                 DEBUGF("Unclosed conditional");
                 break;
@@ -624,6 +622,10 @@ void print_debug_info(struct wps_data *data, int fail, int line)
                                       buf, sizeof(buf))
                       );
                 break;
+                
+            case PARSE_FAIL_LIMITS_EXCEEDED:
+                DEBUGF("Limits exceeded");
+                break;
         }
         DEBUGF("\n");
     }
diff --git a/apps/gui/wps_parser.c b/apps/gui/wps_parser.c
index 163bf84917..88a237cb0f 100644
--- a/apps/gui/wps_parser.c
+++ b/apps/gui/wps_parser.c
@@ -59,11 +59,6 @@
 
 #define WPS_ERROR_INVALID_PARAM         -1
 
-#define PARSE_FAIL_UNCLOSED_COND        1
-#define PARSE_FAIL_INVALID_CHAR         2
-#define PARSE_FAIL_COND_SYNTAX_ERROR    3
-#define PARSE_FAIL_COND_INVALID_PARAM   4
-
 /* level of current conditional.
    -1 means we're not in a conditional. */
 static int level = -1;
@@ -1186,7 +1181,7 @@ static bool wps_parse(struct wps_data *data, const char *wps_bufptr)
 
     char *stringbuf = data->string_buffer;
     int stringbuf_used = 0;
-    int fail = 0;
+    enum wps_parse_error fail = PARSE_OK;
     int ret;
     line = 1;
     level = -1;
@@ -1205,6 +1200,11 @@ static bool wps_parse(struct wps_data *data, const char *wps_bufptr)
                     fail = PARSE_FAIL_COND_INVALID_PARAM;
                     break;
                 }
+                else if (level >= WPS_MAX_COND_LEVEL - 1)
+                {
+                    fail = PARSE_FAIL_LIMITS_EXCEEDED;
+                    break;
+                }
                 wps_bufptr += ret;
                 break;
 
@@ -1219,7 +1219,7 @@ static bool wps_parse(struct wps_data *data, const char *wps_bufptr)
                 if (data->num_sublines+1 < WPS_MAX_SUBLINES)
                     wps_start_new_subline(data);
                 else
-                    wps_bufptr += skip_end_of_line(wps_bufptr);
+                    fail = PARSE_FAIL_LIMITS_EXCEEDED;
 
                 break;
 
@@ -1343,38 +1343,35 @@ static bool wps_parse(struct wps_data *data, const char *wps_bufptr)
                     /* If a matching string is found, found is true and i is
                        the index of the string. If not, found is false */
 
-                    /* If it's NOT a duplicate, do nothing if we already have
-                       too many unique strings */
-                    if (found ||
-                        (stringbuf_used < STRING_BUFFER_SIZE - 1 &&
-                         data->num_strings < WPS_MAX_STRINGS))
+                    if (!found)
                     {
-                        if (!found)
-                        {
-                            /* new string */
-
-                            /* truncate? */
-                            if (stringbuf_used + len > STRING_BUFFER_SIZE - 1)
-                                len = STRING_BUFFER_SIZE - stringbuf_used - 1;
-
-                            strncpy(stringbuf, string_start, len);
-                            *(stringbuf + len) = '\0';
+                        /* new string */
 
-                            data->strings[data->num_strings] = stringbuf;
-                            stringbuf += len + 1;
-                            stringbuf_used += len + 1;
-                            data->tokens[data->num_tokens].value.i =
-                                data->num_strings;
-                            data->num_strings++;
-                        }
-                        else
+                        if (stringbuf_used + len > STRING_BUFFER_SIZE - 1
+                            || data->num_strings >= WPS_MAX_STRINGS)
                         {
-                            /* another ocurrence of an existing string */
-                            data->tokens[data->num_tokens].value.i = i;
+                            /* too many strings or characters */
+                            fail = PARSE_FAIL_LIMITS_EXCEEDED;
+                            break;
                         }
-                        data->tokens[data->num_tokens].type = WPS_TOKEN_STRING;
-                        data->num_tokens++;
+
+                        strncpy(stringbuf, string_start, len);
+                        *(stringbuf + len) = '\0';
+
+                        data->strings[data->num_strings] = stringbuf;
+                        stringbuf += len + 1;
+                        stringbuf_used += len + 1;
+                        data->tokens[data->num_tokens].value.i =
+                            data->num_strings;
+                        data->num_strings++;
                     }
+                    else
+                    {
+                        /* another occurrence of an existing string */
+                        data->tokens[data->num_tokens].value.i = i;
+                    }
+                    data->tokens[data->num_tokens].type = WPS_TOKEN_STRING;
+                    data->num_tokens++;
                 }
                 break;
         }
@@ -1382,6 +1379,10 @@ static bool wps_parse(struct wps_data *data, const char *wps_bufptr)
 
     if (!fail && level >= 0) /* there are unclosed conditionals */
         fail = PARSE_FAIL_UNCLOSED_COND;
+    
+    if (*wps_bufptr && !fail)
+        /* one of the limits of the while loop was exceeded */
+        fail = PARSE_FAIL_LIMITS_EXCEEDED;
 
     data->viewports[data->num_viewports].last_line = data->num_lines - 1;