dashdemux: corrected parsing of segment templates

Corrected the parsing of a segment template string.
Added unit tests to test the segment template parsing.
All reported problems are now correctly handled.

https://bugzilla.gnome.org/show_bug.cgi?id=751735

ext/dash/gstmpdparser.c

@@ -2961,12 +2961,11 @@ gst_mpdparser_get_initializationURL (GstActiveStream * stream,
 static gboolean
 validate_format (const gchar * format)
 {
-  gchar *p;
+  const gchar *p = format;
 
-  /* Check if there is a % at all */
+  /* Check if it starts with % */
-  p = strchr (format, '%');
+  if (!p || p[0] != '%')
-  if (!p)
+    return FALSE;
-    return TRUE;
   p++;
 
   /* Following the % must be a 0, or any of d, x or u.
@@ -2999,16 +2998,13 @@ validate_format (const gchar * format)
 static gchar *
 promote_format_to_uint64 (const gchar * format)
 {
-  gchar *p;
+  const gchar *p = format;
   gchar *promoted_format;
 
   /* Must be called with a validated format! */
   g_return_val_if_fail (validate_format (format), NULL);
 
-  /* Check if there is a % at all */
+  /* it starts with % */
-  p = strchr (format, '%');
-  if (!p)
-    return g_strdup (format);
   p++;
 
   /* Following the % must be a 0, or any of d, x or u.
@@ -3061,7 +3057,6 @@ gst_mpdparser_build_URL_from_template (const gchar * url_template,
   gchar **tokens, *token, *ret;
   const gchar *format;
   gint i, num_tokens;
-  gboolean last_token_par = TRUE;       /* last token was a parameter */
 
   g_return_val_if_fail (url_template != NULL, NULL);
   tokens = g_strsplit_set (url_template, "$", -1);
@@ -3071,10 +3066,30 @@ gst_mpdparser_build_URL_from_template (const gchar * url_template,
   }
   num_tokens = g_strv_length (tokens);
 
+  /*
+   * each identifier is guarded by 2 $, which means that we must have an odd number of tokens
+   * An even number of tokens means the string is not valid.
+   */
+  if ((num_tokens & 1) == 0) {
+    GST_ERROR ("Invalid number of tokens (%d). url_template is '%s'",
+        num_tokens, url_template);
+    g_strfreev (tokens);
+    return NULL;
+  }
+
   for (i = 0; i < num_tokens; i++) {
     token = tokens[i];
     format = default_format;
 
+    /* the tokens to replace must be provided between $ characters, eg $token$
+     * For a string like token0$token1$token2$token3$token4, only the odd number
+     * tokens (1,3,...) must be parsed.
+     *
+     * Skip even tokens
+     */
+    if ((i & 1) == 0)
+      continue;
+
     if (!g_strcmp0 (token, "RepresentationID")) {
       if (!gst_mpdparser_validate_rfc1738_url (id)) {
         GST_WARNING ("String '%s' has characters invalid in an RFC 1738 URL",
@@ -3083,7 +3098,6 @@ gst_mpdparser_build_URL_from_template (const gchar * url_template,
       }
       tokens[i] = g_strdup_printf ("%s", id);
       g_free (token);
-      last_token_par = TRUE;
     } else if (!strncmp (token, "Number", 6)) {
       if (strlen (token) > 6) {
         format = token + 6;     /* format tag */
@@ -3093,7 +3107,6 @@ gst_mpdparser_build_URL_from_template (const gchar * url_template,
 
       tokens[i] = g_strdup_printf (format, number);
       g_free (token);
-      last_token_par = TRUE;
     } else if (!strncmp (token, "Bandwidth", 9)) {
       if (strlen (token) > 9) {
         format = token + 9;     /* format tag */
@@ -3103,7 +3116,6 @@ gst_mpdparser_build_URL_from_template (const gchar * url_template,
 
       tokens[i] = g_strdup_printf (format, bandwidth);
       g_free (token);
-      last_token_par = TRUE;
     } else if (!strncmp (token, "Time", 4)) {
       gchar *promoted_format;
 
@@ -3117,17 +3129,16 @@ gst_mpdparser_build_URL_from_template (const gchar * url_template,
       tokens[i] = g_strdup_printf (promoted_format, time);
       g_free (promoted_format);
       g_free (token);
-      last_token_par = TRUE;
     } else if (!g_strcmp0 (token, "")) {
-      if (!last_token_par) {
       tokens[i] = g_strdup_printf ("%s", "$");
       g_free (token);
-        last_token_par = TRUE;
     } else {
-        last_token_par = FALSE;
+      /* unexpected identifier found between $ signs
-      }
+       *
-    } else {
+       * "If the URL contains unescaped $ symbols which do not enclose a valid
-      last_token_par = FALSE;
+       * identifier then the result of URL formation is undefined"
+       */
+      goto invalid_format;
     }
   }
 

tests/check/elements/dash_mpd.c

@@ -2464,20 +2464,57 @@ GST_END_TEST;
  */
 GST_START_TEST (dash_mpdparser_template_parsing)
 {
-  const gchar *url_template;
   const gchar *id = "TestId";
   guint number = 7;
   guint bandwidth = 2500;
   guint64 time = 100;
   gchar *result;
 
-  url_template = "TestMedia$Bandwidth$$$test";
+  struct TestUrl
-  result =
+  {
-      gst_mpdparser_build_URL_from_template (url_template, id, number,
+    const gchar *urlTemplate;
-      bandwidth, time);
+    const gchar *expectedResponse;
-  assert_equals_string (result, "TestMedia2500$test");
+  };
-  g_free (result);
 
+  /* various test scenarios to attempt */
+  struct TestUrl testUrl[] = {
+    {"", NULL},                 /* empty string for template */
+    {"$$", "$"},                /* escaped $ */
+    {"Number", "Number"},       /* string similar with an identifier, but without $ */
+    {"Number$Number$", "Number7"},      /* Number identifier */
+    {"Number$Number$$$", "Number7$"},   /* Number identifier followed by $$ */
+    {"Number$Number$Number$Number$", "Number7Number7"}, /* series of "Number" string and Number identifier */
+    {"Representation$RepresentationID$", "RepresentationTestId"},       /* RepresentationID identifier */
+    {"TestMedia$Bandwidth$$$test", "TestMedia2500$test"},       /* Bandwidth identifier */
+    {"TestMedia$Time$", "TestMedia100"},        /* Time identifier */
+    {"TestMedia$Time", NULL},   /* Identifier not finished with $ */
+    {"Time$Time%0d$", "Time100"},       /* usage of format smaller than number of digits */
+    {"Time$Time%01d$", "Time100"},      /* usage of format smaller than number of digits */
+    {"Time$Time%05d$", "Time00100"},    /* usage of format bigger than number of digits */
+    {"Time$Time%05dtest$", "Time00100test"},    /* usage extra text in format */
+    {"Time$Time%0$", NULL},     /* incorrect format: no d, x or u */
+    {"Time$Time1%01d$", NULL},  /* incorrect format: does not start with % after identifier */
+    {"$Bandwidth%/init.mp4v", NULL},    /* incorrect identifier: not finished with $ */
+    {"$Number%/$Time$.mp4v", NULL},     /* incorrect number of $ separators */
+    {"$RepresentationID1$", NULL},      /* incorrect identifier */
+    {"$Bandwidth1$", NULL},     /* incorrect identifier */
+    {"$Number1$", NULL},        /* incorrect identifier */
+    {"$RepresentationID%01d$", NULL},   /* incorrect format: RepresentationID does not support formatting */
+    {"Time$Time%05u$", "Time00100"},    /* %u format */
+    {"Time$Time%05x$", "Time00064"},    /* %x format */
+    {"Time$Time%05utest$", "Time00100test"},    /* %u format followed by text */
+    {"Time$Time%05xtest$", "Time00064test"},    /* %x format followed by text */
+    {"Time$Time%05xtest%$", NULL},      /* second % character in format */
+  };
+
+  guint count = sizeof (testUrl) / sizeof (testUrl[0]);
+  for (int i = 0; i < count; i++) {
+    result =
+        gst_mpdparser_build_URL_from_template (testUrl[i].urlTemplate, id,
+        number, bandwidth, time);
+    assert_equals_string (result, testUrl[i].expectedResponse);
+    g_free (result);
+  }
 }
 
 GST_END_TEST;