Bug 83422 – WebVTT parser unnecessarily limits the value of a timestamp

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Eric Carlson]

The WebVTT spec defines the hour portion of a timestamp as: Two or more characters in the range U+0030 DIGIT ZERO (0) to U+0039 DIGIT NINE (9), representing the hours as a base ten integer.

WebKit's WebVTT parser uses an integer to represent hours and converts to a float by multiplying with another integer, limiting the maximum number of hours to 596,523 ((2**31) - 1) / 3600. Because the spec allows more than 6 digits for an hour, this is a bug.

Thanks to Ralph Giles for reporting this.

[Eric Carlson]

Created attachment 136133 [details]
Proposed patch

[Radar WebKit Bug Importer]

<rdar://problem/11206564>

[Darin Adler]

Comment on attachment 136133 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=136133&action=review

> Source/WebCore/html/track/WebVTTParser.cpp:337
> +    return (static_cast<double>(value1) * secondsPerHour) + (value2 * secondsPerMinute) + value3 + (static_cast<double>(value4) / 1000);

Why not cast of value2?

Another way to fix this is to make secondsPerHour and secondsPerMinute both double constants instead of in, and also make a third constant called secondsPerMillisecond (a double with the value 0.001) and write this:

    return value1 * secondsPerHour + value2 * secondsPerMinute + value3 + value4 * secondsPerMillisecond;

[Eric Carlson]

(In reply to comment #3)
> (From update of attachment 136133 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=136133&action=review
> 
> > Source/WebCore/html/track/WebVTTParser.cpp:337
> > +    return (static_cast<double>(value1) * secondsPerHour) + (value2 * secondsPerMinute) + value3 + (static_cast<double>(value4) / 1000);
> 
> Why not cast of value2?
> 
value2 is always between 0 and 59 so it did not need a cast.

> Another way to fix this is to make secondsPerHour and secondsPerMinute both double constants instead of in, and also make a third constant called secondsPerMillisecond (a double with the value 0.001) and write this:
> 
>     return value1 * secondsPerHour + value2 * secondsPerMinute + value3 + value4 * secondsPerMillisecond;

Thanks for the great suggestion, I made this change.

[Eric Carlson]

http://trac.webkit.org/changeset/113583