GNOME Bugzilla – Bug 752429
dashdemux: negative numbers are successfully read into unsigned variables
Last modified: 2015-10-29 10:57:57 UTC
If the xml contains a negative number into an attribute that is supposed to be unsigned (eg durations, dates, ranges), the parser will successfully read it and store the negative value as unsigned. I think it should reject the value and consider the parsing of that attribute a failure.
Created attachment 307487 [details] [review] proposed fix and unit tests attached a proposed fix and unit test reproducing the problem
Review of attachment 307487 [details] [review]: ::: ext/dash/gstmpdparser.c @@ +261,3 @@ if (prop_string) { + if (sscanf ((gchar *) prop_string, "%u", property_value) && + strstr ((gchar *) prop_string, "-") == NULL) { minor, but could use strchr since it's a single char @@ +727,1 @@ goto error; Why not valid range ? Same for hours etc below. Assuming no leap seconds.
(In reply to Vincent Penquerc'h from comment #2) > Review of attachment 307487 [details] [review] [review]: > > ::: ext/dash/gstmpdparser.c > @@ +261,3 @@ > if (prop_string) { > + if (sscanf ((gchar *) prop_string, "%u", property_value) && > + strstr ((gchar *) prop_string, "-") == NULL) { > > minor, but could use strchr since it's a single char No, because the possibility of spaces at the beginning of the string. > > @@ +727,1 @@ > goto error; > > Why not valid range ? Same for hours etc below. Assuming no leap seconds. Because 0 is not a valid value for year, month or day, but it is a valid value for hour, minute or second. http://books.xmlschemata.org/relaxng/ch19-77049.html
Created attachment 314335 [details] [review] proposed fix and unit tests I merged this to latest master, if you want to double check. Some of the hunks were already applied. If no comment, I'll push this later.
commit 7c2746f741ae25dc3086677306dd9d87012e0a57 Author: Florin Apostol <florin.apostol@oregan.net> Date: Wed Oct 28 16:24:01 2015 +0000 dashdemux: corrected parsing of negative values into unsigned data https://bugzilla.gnome.org/show_bug.cgi?id=752429