hlsdemux: unquote all the quoted-string attributes

The URI attribute from the EXT-X-KEY tag and the URI attribute from the
EXT-X-I-FRAMES-ONLY tag are both quoted-string attibutes that have their
quotation marks removed during parsing. The CODECS attribute of the
EXT-X-STREAM-INF is also a quoted-string attribute, but this attribute
was not being un-quoted.

This commit changes the parser to always unquote all quoted-string
attributes and adjusts the unit tests to this new bevahiour for the
CODECS attribute.

An additional test is added to check that parsing of all of the fields
in the EXT-X-STREAM tag is correct, including those that contain comma
characters.

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

ext/hls/m3u8.c

@@ -200,7 +200,7 @@ double_from_string (gchar * ptr, gchar ** endptr, gdouble * val)
 static gboolean
 parse_attributes (gchar ** ptr, gchar ** a, gchar ** v)
 {
-  gchar *end = NULL, *p;
+  gchar *end = NULL, *p, *ve;
 
   g_return_val_if_fail (ptr != NULL, FALSE);
   g_return_val_if_fail (*ptr != NULL, FALSE);
@@ -233,8 +233,20 @@ parse_attributes (gchar ** ptr, gchar ** a, gchar ** v)
 
   *v = p = g_utf8_strchr (*ptr, -1, '=');
   if (*v) {
-    *v = g_utf8_next_char (*v);
     *p = '\0';
+    *v = g_utf8_next_char (*v);
+    if (**v == '"') {
+      ve = g_utf8_next_char (*v);
+      if (ve) {
+        ve = g_utf8_strchr (ve, -1, '"');
+      }
+      if (ve) {
+        *v = g_utf8_next_char (*v);
+        *ve = '\0';
+      } else {
+        GST_WARNING ("Cannot remove quotation marks from %s", *a);
+      }
+    }
   } else {
     GST_WARNING ("missing = after attribute");
     return FALSE;
@@ -244,28 +256,6 @@ parse_attributes (gchar ** ptr, gchar ** a, gchar ** v)
   return TRUE;
 }
 
-static gchar *
-unquote_string (gchar * string)
-{
-  gchar *string_ret;
-
-  string_ret = strchr (string, '"');
-  if (string_ret != NULL) {
-    /* found initialization quotation mark of string */
-    string = string_ret + 1;
-    string_ret = strchr (string, '"');
-    if (string_ret != NULL) {
-      /* found finalizing quotation mark of string */
-      string_ret[0] = '\0';
-    } else {
-      GST_WARNING
-          ("wrong string unqouting - cannot find finalizing quotation mark");
-      return NULL;
-    }
-  }
-  return string;
-}
-
 static gint
 _m3u8_compare_uri (GstM3U8 * a, gchar * uri)
 {
@@ -476,24 +466,13 @@ gst_m3u8_update (GstM3U8Client * client, GstM3U8 * self, gchar * data,
             }
           } else if (iframe && g_str_equal (a, "URI")) {
             gchar *name;
-            gchar *uri = g_strdup (v);
+            gchar *uri;
-            gchar *urip = uri;
 
-            uri = unquote_string (uri);
+            uri = uri_join (self->base_uri ? self->base_uri : self->uri, v);
             if (uri) {
-              uri = uri_join (self->base_uri ? self->base_uri : self->uri, uri);
-              if (uri == NULL) {
-                g_free (urip);
-                continue;
-              }
               name = g_strdup (uri);
-
               gst_m3u8_set_uri (new_list, uri, NULL, name);
-            } else {
-              GST_WARNING
-                  ("Cannot remove quotation marks from i-frame-stream URI");
             }
-            g_free (urip);
           }
         }
 
@@ -536,18 +515,8 @@ gst_m3u8_update (GstM3U8Client * client, GstM3U8 * self, gchar * data,
         current_key = NULL;
         while (data && parse_attributes (&data, &a, &v)) {
           if (g_str_equal (a, "URI")) {
-            gchar *key = g_strdup (v);
-            gchar *keyp = key;
-
-            key = unquote_string (key);
-            if (key) {
             current_key =
-                  uri_join (self->base_uri ? self->base_uri : self->uri, key);
+                uri_join (self->base_uri ? self->base_uri : self->uri, v);
-            } else {
-              GST_WARNING
-                  ("Cannot remove quotation marks from decryption key URI");
-            }
-            g_free (keyp);
           } else if (g_str_equal (a, "IV")) {
             gchar *ivp = v;
             gint i;

tests/check/elements/hlsdemux_m3u8.c

@@ -381,7 +381,7 @@ do_test_load_main_playlist_variant (const gchar * playlist)
   assert_equals_int (stream->bandwidth, 65000);
   assert_equals_int (stream->program_id, 1);
   assert_equals_string (stream->uri, "http://example.com/audio-only.m3u8");
-  assert_equals_string (stream->codecs, "\"mp4a.40.5\"");
+  assert_equals_string (stream->codecs, "mp4a.40.5");
 
   /* Low */
   tmp = g_list_next (tmp);
@@ -1333,6 +1333,29 @@ GST_START_TEST (test_simulation)
 GST_END_TEST;
 #endif
 
+GST_START_TEST (test_stream_inf_tag)
+{
+  static const gchar *MASTER_PLAYLIST = "#EXTM3U \n"
+      "#EXT-X-VERSION:4\n"
+      "#EXT-X-STREAM-INF:PROGRAM-ID=1, BANDWIDTH=1251135, CODECS=\"avc1.42001f, mp4a.40.2\", RESOLUTION=640x352\n"
+      "media.m3u8\n";
+  GstM3U8Client *client;
+  GstM3U8 *media;
+
+  client = load_playlist (MASTER_PLAYLIST);
+
+  assert_equals_int (g_list_length (client->main->lists), 1);
+  media = g_list_nth_data (client->main->lists, 0);
+  assert_equals_int64 (media->program_id, 1);
+  assert_equals_int64 (media->width, 640);
+  assert_equals_int64 (media->height, 352);
+  assert_equals_int64 (media->bandwidth, 1251135);
+  assert_equals_string (media->codecs, "avc1.42001f, mp4a.40.2");
+  gst_m3u8_client_free (client);
+}
+
+GST_END_TEST;
+
 static Suite *
 hlsdemux_suite (void)
 {
@@ -1373,7 +1396,7 @@ hlsdemux_suite (void)
 #endif
   tcase_add_test (tc_m3u8, test_playlist_with_doubles_duration);
   tcase_add_test (tc_m3u8, test_playlist_with_encryption);
-
+  tcase_add_test (tc_m3u8, test_stream_inf_tag);
   return s;
 }