Bug 46241 - HTML5 video element Useragent string is QuickTime
Summary: HTML5 video element Useragent string is QuickTime
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Critical
Assignee: Eric Carlson
URL: http://exposureroom.com
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-09-21 20:11 PDT by Shiv Kumar
Modified: 2012-03-07 13:28 PST (History)
2 users (show)

See Also:


Attachments
Proposed patch (12.18 KB, patch)
2012-03-06 12:13 PST, Eric Carlson
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shiv Kumar 2010-09-21 20:11:27 PDT
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.
Comment 1 Eric Carlson 2010-09-22 07:24:13 PDT
<rdar://problem/8463568>
Comment 2 Eric Carlson 2012-03-06 12:13:11 PST
Created attachment 130426 [details]
Proposed patch
Comment 3 Daniel Bates 2012-03-07 11:50:40 PST
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=".
Comment 4 Daniel Bates 2012-03-07 11:53:05 PST
(In reply to comment #3)
>
>[...] length of the bytes-unit string "bytes=".

This should read:

[...] length of the bytes-unit string plus "=".
Comment 5 Eric Carlson 2012-03-07 13:27:47 PST
(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!
Comment 6 Eric Carlson 2012-03-07 13:28:06 PST
http://trac.webkit.org/changeset/110095