Bug 46241 – HTML5 video element Useragent string is QuickTime

• Component: Media
• Status: RESOLVED FIXED

[Shiv Kumar]

When Safari makes a request for a video stream, the useragent string the server side receives is Quicktime. This causes problems because we don't typically allow Quicktime direct access to videos.

The useragent string should be the same as the Safari useragent string.

[Eric Carlson]

<rdar://problem/8463568>

[Eric Carlson]

Created attachment 130426 [details]
Proposed patch

[Daniel Bates]

Comment on attachment 130426 [details]
Proposed patch

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

This patch looks straightforward to me.

Notice that the code in LayoutTests/http/tests/media/resources/serve-video.php doesn't adhere to the WebKit style guide. Although we don't seem to strictly enforce these rules for test files and test support files, you may want to consider fixing the style so as to be consistent with the style guide either in this patch or in a follow up patch.

> Source/WebCore/ChangeLog:3
> +        Html 5 video element Useragent string is Quicktime

Nit: "Html 5" => "HTML5"

> LayoutTests/http/tests/media/resources/serve-video.php:12
> +        $range = explode("-", substr($contentRange, 6)); 

I know that you extracted this code from file LayoutTests/http/tests/media/resources/video-referer-check-referer.php.

The number 6 for the length parameter of substr() seems mysterious, especially for people who may not be familiar with the syntax for Byte Ranges (maybe we don't have to worry about this audience?) . For your consideration, I suggest either substituting the expression strlen("bytes=") or adding an inline comment that explains that 6 is the length of the bytes-unit string "bytes=".

[Daniel Bates]

(In reply to comment #3)
>
>[...] length of the bytes-unit string "bytes=".

This should read:

[...] length of the bytes-unit string plus "=".

[Eric Carlson]

(In reply to comment #3)
> (From update of attachment 130426 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130426&action=review
> 
> Notice that the code in LayoutTests/http/tests/media/resources/serve-video.php doesn't adhere to the WebKit style guide. Although we don't seem to strictly enforce these rules for test files and test support files, you may want to consider fixing the style so as to be consistent with the style guide either in this patch or in a follow up patch.
> 
Done.

> > Source/WebCore/ChangeLog:3
> > +        Html 5 video element Useragent string is Quicktime
> 
> Nit: "Html 5" => "HTML5"
> 
> > LayoutTests/http/tests/media/resources/serve-video.php:12
> > +        $range = explode("-", substr($contentRange, 6)); 
> 
> I know that you extracted this code from file LayoutTests/http/tests/media/resources/video-referer-check-referer.php.
> 
Well, I "extracted" code that I wrote in the first place so the fault is mine ;-)

> The number 6 for the length parameter of substr() seems mysterious, especially for people who may not be familiar with the syntax for Byte Ranges (maybe we don't have to worry about this audience?) . For your consideration, I suggest either substituting the expression strlen("bytes=") or adding an inline comment that explains that 6 is the length of the bytes-unit string "bytes=".

Done.

Thanks for the review!

[Eric Carlson]

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