[Bug 774463] mpdparser: Allows invalid URI within xlink attributes

[GStreamer (GNOME Bugzilla)]

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

--- Comment #6 from Seungha Yang <sh.yang at lge.com> ---
(In reply to Sebastian Dröge (slomo) from comment #5)
> Review of attachment 339918 [details] [review]:
> 
> ::: ext/dash/gstmpdparser.c
> @@ -4246,3 @@
>          period_node->xlink_href, err->message);
>      g_clear_error (&err);
> -    *error = TRUE;
> 
> This means that error is never ever set to anything. Should just be removed
> from the parameters then

The error is still used for checking "downloader"
https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/ext/dash/gstmpdparser.c#n4217
Could you give me suggestion about that, whether "removing the condition on
function and checking downloader whenever before calling the function" or
"leave it"?

> 
> @@ +4309,3 @@
> +  if (new_periods) {
> +    g_list_free_full (new_periods,
> +        (GDestroyNotify) gst_mpdparser_free_period_node);
> 
> Shouldn't we keep all periods that were successfully parsed? Or is the spec
> saying to discard all if one can't be parsed?

Actually, there are a little uncertainty on that. In my interpretation of spec,
however, all period on external xml should be removed.

spec is saying that "if there is any error on remote xml, corresponding element
on parent MPD should be removed". That is, although one of period could be
correctly parsed, another error element will cause removal the external xml
element on parent MPD. So, the correctly parsed period on external xml is no
more validated.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.