Skip to content

Commit

Permalink
MediaFragmentURIParser rejects valid NPT strings if 'hours' is define…
Browse files Browse the repository at this point in the history
…d using 1 digit

https://bugs.webkit.org/show_bug.cgi?id=160199

Reviewed by Eric Carlson.

This patch aligns WebKit with Blink / Chromium and web specification [1]:

Merge: https://chromium.googlesource.com/chromium/src/+/139d93b86c305ec60e9f41890cefcb4972dd8e7f

[1] https://www.w3.org/TR/media-frags/#naming-time

"Minutes and seconds must be specified as exactly two digits, hours and fractional seconds can be any number of digits."

As above, and also per RFC2326 [2], 'hours' can be single digit.

[2] https://www.ietf.org/rfc/rfc2326.txt

Refer:

> npt-hh       =   1*DIGIT     ; any positive number

* Source/WebCore/html/MediaFragmentURIParser.cpp:
(MediaFragmentURIParser::parseNPTTime):
* LayoutTests/media/media-fragments/TC0095.html: Add Test Case
* LayoutTests/media/media-fragments/TC0095-expected.txt: Add Test Case Expectation
* LayoutTests/media/media-fragments/media-fragments.js: Update Test script

Canonical link: https://commits.webkit.org/278951@main
  • Loading branch information
Ahmad Saleem committed May 18, 2024
1 parent d41105b commit 1b08c7e
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 5 deletions.
12 changes: 12 additions & 0 deletions LayoutTests/media/media-fragments/TC0095-expected.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@


Title: TC0095
Fragment: 't=0:00:03,0:00:07'
Comment: The media is requested from a to b.
EVENT(canplaythrough)
EXPECTED (video.currentTime == '3') OK
RUN(video.play())
EVENT(pause)
EXPECTED (video.currentTime - fragmentEndTime <= '0.75') OK
END OF TEST

9 changes: 9 additions & 0 deletions LayoutTests/media/media-fragments/TC0095.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<html>
<head>
<script src=../media-file.js></script>
<script src=../video-test.js></script>
<script src=media-fragments.js></script>
</head>
<body onload="start()">
</html>
3 changes: 2 additions & 1 deletion LayoutTests/media/media-fragments/media-fragments.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 14 additions & 4 deletions Source/WebCore/html/MediaFragmentURIParser.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2011, 2012 Apple Inc. All rights reserved.
* Copyright (C) 2011-2024 Apple Inc. All rights reserved.
* Copyright (C) 2023 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -275,10 +276,8 @@ bool MediaFragmentURIParser::parseNPTTime(std::span<const LChar> timeString, uns
return true;
}

if (digits1.length() < 2)
if (digits1.length() < 1)
return false;
if (digits1.length() > 2)
mode = hours;

// Collect the next sequence of 0-9 after ':'
if (offset >= timeString.size() || timeString[offset++] != ':')
Expand All @@ -291,6 +290,15 @@ bool MediaFragmentURIParser::parseNPTTime(std::span<const LChar> timeString, uns
int value2 = parseInteger<int>(digits2).value();

// Detect whether this timestamp includes hours.
if (offset < timeString.size() && timeString[offset] == ':')
mode = hours;
if (mode == minutes) {
if (digits1.length() != 2)
return false;
if (value1 > 59 || value2 > 59)
return false;
}

int value3;
if (mode == hours || (offset < timeString.size() && timeString[offset] == ':')) {
if (offset >= timeString.size() || timeString[offset++] != ':')
Expand All @@ -301,6 +309,8 @@ bool MediaFragmentURIParser::parseNPTTime(std::span<const LChar> timeString, uns
if (digits3.length() != 2)
return false;
value3 = parseInteger<int>(digits3).value();
if (value2 > 59 || value3 > 59)
return false;
} else {
value3 = value2;
value2 = value1;
Expand Down

0 comments on commit 1b08c7e

Please sign in to comment.