Skip to content

Commit

Permalink
structure: don't unescape values before deserializing
Browse files Browse the repository at this point in the history 
No longer call _priv_gst_value_parse_string with unescape set to TRUE
before passing a value to gst_value_deserialize in
_priv_gst_value_parse_value. This latter function is called by
gst_structure_from_string and gst_caps_from_string.

When gst_structure_to_string and gst_caps_to_string are called, no
escaping is performed after calling gst_value_serialize. Therefore, by
unescaping the value string, we were introducing an additional operation
that was not performed by the original *_to_string functions. In
particular, this has meant that the derialization functions for many
non-basic types are incomplete reverses of the corresponding
serialization function (i.e., if you pipe the output of the
serialization function into the deserialization function it could fail)
because they have to compensate for this additional escaping operation,
when really this should be the domain of the deserialization functions
instead.

Correspondingly changed a few deserialization functions.

Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/452

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/303>
  • Loading branch information
@hwilkes-igalia
hwilkes-igalia authored and GStreamer Merge Bot committed Jan 19, 2021
1 parent f0d0032 commit 7f26739
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 33 deletions.
61 changes: 54 additions & 7 deletions gst/gstvalue.c
View file
Expand Up @@ -2243,6 +2243,8 @@ gst_value_deserialize_caps (GValue * dest, const gchar * s)
GstCaps *caps;

if (*s != '"') {
/* this can happen if caps are ANY, EMPTY, or only contains a single
* empty structure */
caps = gst_caps_from_string (s);
} else {
gchar *str = gst_string_unwrap (s);
Expand Down Expand Up @@ -2802,7 +2804,7 @@ _priv_gst_value_parse_value (gchar * str,
};
int i;

if (G_UNLIKELY (!_priv_gst_value_parse_string (s, &value_end, &s, TRUE)))
if (G_UNLIKELY (!_priv_gst_value_parse_string (s, &value_end, &s, FALSE)))
return FALSE;
/* Set NULL terminator for deserialization */
value_s = g_strndup (value_s, value_end - value_s);
Expand All @@ -2817,8 +2819,7 @@ _priv_gst_value_parse_value (gchar * str,
} else {
g_value_init (value, type);

if (G_UNLIKELY (!_priv_gst_value_parse_string (s, &value_end, &s,
(type != G_TYPE_STRING))))
if (G_UNLIKELY (!_priv_gst_value_parse_string (s, &value_end, &s, FALSE)))
return FALSE;
/* Set NULL terminator for deserialization */
value_s = g_strndup (value_s, value_end - value_s);
Expand Down Expand Up @@ -2878,14 +2879,32 @@ gst_value_serialize_segment (const GValue * value)
}

static gboolean
gst_value_deserialize_segment (GValue * dest, const gchar * s)
gst_value_deserialize_segment_internal (GValue * dest, const gchar * s,
gboolean unescape)
{
GstStructure *str;
GstSegment seg;
gboolean res;
gsize len;
gchar *t;

str = gst_structure_from_string (s, NULL);
if (str == NULL)
if (unescape) {
len = strlen (s);
if (G_UNLIKELY (*s != '"' || len < 2 || s[len - 1] != '"')) {
/* "\"" is not an accepted string, so len must be at least 2 */
GST_ERROR ("Failed deserializing segement: expected string to start and "
"end with '\"'");
return FALSE;
}
t = g_strdup (s + 1);
t[len - 2] = '\0';
/* removed trailing '"' */
str = gst_structure_from_string (t, NULL);
g_free (t);
} else {
str = gst_structure_from_string (s, NULL);
}
if (G_UNLIKELY (str == NULL))
return FALSE;

res = gst_structure_id_get (str,
Expand All @@ -2908,6 +2927,12 @@ gst_value_deserialize_segment (GValue * dest, const gchar * s)
return res;
}

static gboolean
gst_value_deserialize_segment (GValue * dest, const gchar * s)
{
return gst_value_deserialize_segment_internal (dest, s, TRUE);
}

/****************
* GstStructure *
****************/
Expand Down Expand Up @@ -2952,6 +2977,8 @@ gst_value_serialize_structure (const GValue * value)
GstStructure *structure = g_value_get_boxed (value);

return priv_gst_string_take_and_wrap (gst_structure_to_string (structure));
/* string should always end up being wrapped, since a structure string
* ends in a ';' character */
}

static gboolean
Expand All @@ -2960,6 +2987,14 @@ gst_value_deserialize_structure (GValue * dest, const gchar * s)
GstStructure *structure;

if (*s != '"') {
/* the output of gst_value_serialize_structure would never produce
* such a string, but a user may pass to gst_structure_from_string
* the string:
* name, sub=(GstStructure)sub-name, val=(int)5;
* and expect sub to be read as an *empty* structure with the name
* sub-name. Similar to
* name, caps=(GstCaps)video/x-raw, val=(int)5;
* which gst_structure_to_string can produce. */
structure = gst_structure_from_string (s, NULL);
} else {
gchar *str = gst_string_unwrap (s);
Expand Down Expand Up @@ -3048,6 +3083,9 @@ gst_value_deserialize_caps_features (GValue * dest, const gchar * s)
GstCapsFeatures *features;

if (*s != '"') {
/* This can happen if gst_caps_features_to_string only returns
* ALL, NONE, or a single features name, which means it is not
* actually wrapped by priv_gst_string_take_and_wrap */
features = gst_caps_features_from_string (s);
} else {
gchar *str = gst_string_unwrap (s);
Expand Down Expand Up @@ -3086,6 +3124,13 @@ gst_value_deserialize_tag_list (GValue * dest, const gchar * s)
GstTagList *taglist;

if (*s != '"') {
/* the output of gst_value_serialize_tag_list would never produce
* such a string, but a user may pass to gst_structure_from_string
* the string:
* name, list=(GstTagList)taglist, val=(int)5;
* and expect list to be read as an *empty* tag list. Similar to
* name, caps=(GstCaps)video/x-raw, val=(int)5;
* which gst_structure_to_string can produce. */
taglist = gst_tag_list_new_from_string (s);
} else {
gchar *str = gst_string_unwrap (s);
Expand All @@ -3110,6 +3155,8 @@ gst_value_serialize_tag_list (const GValue * value)
GstTagList *taglist = g_value_get_boxed (value);

return priv_gst_string_take_and_wrap (gst_tag_list_to_string (taglist));
/* string should always end up being wrapped, since a taglist (structure)
* string ends in a ';' character */
}


Expand Down Expand Up @@ -3367,7 +3414,7 @@ gst_value_deserialize_sample (GValue * dest, const gchar * s)
g_strdelimit (fields[2], "_", '=');
g_base64_decode_inplace (fields[2], &outlen);
GST_TRACE ("segment : %s", fields[2]);
if (!gst_value_deserialize_segment (&sval, fields[2]))
if (!gst_value_deserialize_segment_internal (&sval, fields[2], FALSE))
goto fail;
}

Expand Down
49 changes: 23 additions & 26 deletions tests/check/gst/gstvalue.c
View file
Expand Up @@ -3294,31 +3294,30 @@ GST_START_TEST (test_structure_ops)
const gchar *op;
gint ret;
GType str_type;
const gchar *str_result;
} comparisons[] = {
/* *INDENT-OFF* */
{"foo,bar=(int)1", "foo,bar=(int)1", "compare", GST_VALUE_EQUAL, 0, NULL},
{"foo,bar=(int)1", "foo,bar=(int)1", "is_subset", TRUE, 0, NULL},
{"foo,bar=(int)1", "foo,bar=(int)1", "intersect", TRUE, GST_TYPE_STRUCTURE, "foo,bar=(int)1"},
{"foo,bar=(int)1", "foo,bar=(int)1", "union", TRUE, GST_TYPE_STRUCTURE, "foo,bar=(int)1"},
{"foo,bar=(int)[1,2]", "foo,bar=(int)1", "compare", GST_VALUE_UNORDERED, 0, NULL},
{"foo,bar=(int)[1,2]", "foo,bar=(int)1", "is_subset", FALSE, 0, NULL},
{"foo,bar=(int)[1,2]", "foo,bar=(int)1", "intersect", TRUE, GST_TYPE_STRUCTURE, "foo,bar=(int)1"},
{"foo,bar=(int)[1,2]", "foo,bar=(int)1", "union", TRUE, GST_TYPE_STRUCTURE, "foo,bar=(int)[1,2]"},
{"foo,bar=(int)1", "foo,bar=(int)[1,2]", "compare", GST_VALUE_UNORDERED, 0, NULL},
{"foo,bar=(int)1", "foo,bar=(int)[1,2]", "is_subset", TRUE, 0, NULL},
{"foo,bar=(int)1", "foo,bar=(int)[1,2]", "intersect", TRUE, GST_TYPE_STRUCTURE, "foo,bar=(int)1"},
{"foo,bar=(int)1", "foo,bar=(int)[1,2]", "union", TRUE, GST_TYPE_STRUCTURE, "foo,bar=(int)[1,2]"},
{"foo,bar=(int)1", "foo,bar=(int)2", "compare", GST_VALUE_UNORDERED, 0, NULL},
{"foo,bar=(int)1", "foo,bar=(int)2", "is_subset", FALSE, 0, NULL},
{"foo,bar=(int)1", "foo,bar=(int)2", "intersect", FALSE, 0, NULL},
{"foo,bar=(int)1", "foo,bar=(int)2", "union", TRUE, GST_TYPE_STRUCTURE, "foo,bar=(int)[1,2]"},
{"foo,bar=(int)1", "baz,bar=(int)1", "compare", GST_VALUE_UNORDERED, 0, NULL},
{"foo,bar=(int)1", "baz,bar=(int)1", "is_subset", FALSE, 0, NULL},
{"foo,bar=(int)1", "baz,bar=(int)1", "intersect", FALSE, 0, NULL},
{"foo,bar=(int)1", "foo,bar=(int)1", "compare", GST_VALUE_EQUAL, 0},
{"foo,bar=(int)1", "foo,bar=(int)1", "is_subset", TRUE, 0},
{"foo,bar=(int)1", "foo,bar=(int)1", "intersect", TRUE, GST_TYPE_STRUCTURE},
{"foo,bar=(int)1", "foo,bar=(int)1", "union", TRUE, GST_TYPE_STRUCTURE},
{"foo,bar=(int)[1,2]", "foo,bar=(int)1", "compare", GST_VALUE_UNORDERED, 0},
{"foo,bar=(int)[1,2]", "foo,bar=(int)1", "is_subset", FALSE, 0},
{"foo,bar=(int)[1,2]", "foo,bar=(int)1", "intersect", TRUE, GST_TYPE_STRUCTURE},
{"foo,bar=(int)[1,2]", "foo,bar=(int)1", "union", TRUE, GST_TYPE_STRUCTURE},
{"foo,bar=(int)1", "foo,bar=(int)[1,2]", "compare", GST_VALUE_UNORDERED, 0},
{"foo,bar=(int)1", "foo,bar=(int)[1,2]", "is_subset", TRUE, 0},
{"foo,bar=(int)1", "foo,bar=(int)[1,2]", "intersect", TRUE, GST_TYPE_STRUCTURE},
{"foo,bar=(int)1", "foo,bar=(int)[1,2]", "union", TRUE, GST_TYPE_STRUCTURE},
{"foo,bar=(int)1", "foo,bar=(int)2", "compare", GST_VALUE_UNORDERED, 0},
{"foo,bar=(int)1", "foo,bar=(int)2", "is_subset", FALSE, 0},
{"foo,bar=(int)1", "foo,bar=(int)2", "intersect", FALSE, 0},
{"foo,bar=(int)1", "foo,bar=(int)2", "union", TRUE, GST_TYPE_STRUCTURE},
{"foo,bar=(int)1", "baz,bar=(int)1", "compare", GST_VALUE_UNORDERED, 0},
{"foo,bar=(int)1", "baz,bar=(int)1", "is_subset", FALSE, 0},
{"foo,bar=(int)1", "baz,bar=(int)1", "intersect", FALSE, 0},
#if 0
/* deserializing lists is not implemented (but this should still work!) */
{"foo,bar=(int)1", "baz,bar=(int)1", "union", TRUE, G_TYPE_LIST, "{foo,bar=(int)1;, baz,bar=(int)1;}"},
{"foo,bar=(int)1", "baz,bar=(int)1", "union", TRUE, G_TYPE_LIST},
#endif
/* *INDENT-ON* */
};
Expand All @@ -3333,8 +3332,7 @@ GST_START_TEST (test_structure_ops)
fail_unless (s2 != NULL);

GST_DEBUG ("checking %s with structure1 %" GST_PTR_FORMAT " structure2 %"
GST_PTR_FORMAT " is %d, %s", comparisons[i].op, s1, s2,
comparisons[i].ret, comparisons[i].str_result);
GST_PTR_FORMAT " is %d", comparisons[i].op, s1, s2, comparisons[i].ret);

g_value_init (&v1, GST_TYPE_STRUCTURE);
gst_value_set_structure (&v1, s1);
Expand All @@ -3357,11 +3355,10 @@ GST_START_TEST (test_structure_ops)

str = gst_value_serialize (&v3);
GST_LOG ("result %s", str);
g_free (str);

g_value_init (&result, comparisons[i].str_type);
fail_unless (gst_value_deserialize (&result,
comparisons[i].str_result));
fail_unless (gst_value_deserialize (&result, str));
g_free (str);
fail_unless (gst_value_compare (&result, &v3) == GST_VALUE_EQUAL);
g_value_unset (&v3);
g_value_unset (&result);
Expand Down

0 comments on commit 7f26739

Please sign in to comment.