WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
61403
Video fails to play on Vimeo
https://bugs.webkit.org/show_bug.cgi?id=61403
Summary
Video fails to play on Vimeo
Jer Noble
Reported
2011-05-24 16:22:18 PDT
Video fails to play on Vimeo
Attachments
Patch
(9.63 KB, patch)
2011-05-24 16:31 PDT
,
Jer Noble
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
Patch
(13.50 KB, patch)
2011-05-24 17:21 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(15.70 KB, patch)
2011-05-25 02:22 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(14.76 KB, patch)
2011-05-25 09:40 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(14.52 KB, patch)
2011-05-25 10:08 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(360.58 KB, patch)
2011-05-25 11:54 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(361.25 KB, patch)
2011-05-25 14:05 PDT
,
Jer Noble
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Jer Noble
Comment 1
2011-05-24 16:23:06 PDT
<
rdar://problem/9488551
>
Jer Noble
Comment 2
2011-05-24 16:31:23 PDT
Created
attachment 94710
[details]
Patch
Jer Noble
Comment 3
2011-05-24 17:21:35 PDT
Created
attachment 94720
[details]
Patch
WebKit Review Bot
Comment 4
2011-05-24 17:47:23 PDT
Comment on
attachment 94710
[details]
Patch
Attachment 94710
[details]
did not pass cr-mac-ews (chromium): Output:
http://queues.webkit.org/results/8732554
WebKit Review Bot
Comment 5
2011-05-24 20:08:33 PDT
Comment on
attachment 94720
[details]
Patch
Attachment 94720
[details]
did not pass cr-mac-ews (chromium): Output:
http://queues.webkit.org/results/8733424
WebKit Commit Bot
Comment 6
2011-05-24 20:43:43 PDT
Comment on
attachment 94720
[details]
Patch
Attachment 94720
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/8726937
Maciej Stachowiak
Comment 7
2011-05-24 21:23:39 PDT
Comment on
attachment 94720
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=94720&action=review
r- because this appears to break the build. I am not entirely sure why, and otherwise it seems ok though see other comment.
> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundationObjC.mm:259 > - [NSNumber numberWithInt:AVAssetReferenceRestrictionForbidCrossSiteReference], AVURLAssetReferenceRestrictionsKey, > + [NSNumber numberWithInt:AVAssetReferenceRestrictionForbidRemoteReferenceToLocal | AVAssetReferenceRestrictionForbidLocalReferenceToRemote], AVURLAssetReferenceRestrictionsKey,
Is there any way we can allow cross-origin redirects (which seem safe enough) while still preventing cross-origin auxiliary resource references (e.g. reference movies)? I think that should be the goal, but if that's not achievable, this approach seems ok.
Jer Noble
Comment 8
2011-05-25 01:52:22 PDT
(In reply to
comment #7
)
> (From update of
attachment 94720
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=94720&action=review
> > r- because this appears to break the build. I am not entirely sure why, and otherwise it seems ok though see other comment.
Looks like it's because of a missing newline from WebCoreSystemInterface.mm. I'll get that fixed.
> Is there any way we can allow cross-origin redirects (which seem safe enough) while still preventing cross-origin auxiliary resource references (e.g. reference movies)? I think that should be the goal, but if that's not achievable, this approach seems ok.
Unfortunately, QTKit doesn't make any distinction between reference movies and redirects. We could, in theory, take over loading somewhat by checking for HTTP redirects before passing the request on to QTKit, but this would be a major disruptive change.
Jer Noble
Comment 9
2011-05-25 02:22:48 PDT
Created
attachment 94758
[details]
Patch Fixed mac build, and made the changes to MediaPlayerPrivateQTKit >SnowLeopard only.
WebKit Review Bot
Comment 10
2011-05-25 02:40:23 PDT
Comment on
attachment 94758
[details]
Patch
Attachment 94758
[details]
did not pass cr-mac-ews (chromium): Output:
http://queues.webkit.org/results/8731660
WebKit Commit Bot
Comment 11
2011-05-25 02:41:25 PDT
Comment on
attachment 94758
[details]
Patch
Attachment 94758
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/8734385
Jer Noble
Comment 12
2011-05-25 09:40:51 PDT
Created
attachment 94794
[details]
Patch
WebKit Commit Bot
Comment 13
2011-05-25 09:51:00 PDT
Comment on
attachment 94794
[details]
Patch
Attachment 94794
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/8735106
Jer Noble
Comment 14
2011-05-25 10:08:54 PDT
Created
attachment 94800
[details]
Patch One last piece wrapped with >SL conditionals.
Jer Noble
Comment 15
2011-05-25 11:06:38 PDT
Comment on
attachment 94800
[details]
Patch After talking with Eric Carlson, we've decided that the QTKit fix should be platform independent. So I'm taking down this patch for review until I whip up a new one.
Jer Noble
Comment 16
2011-05-25 11:54:30 PDT
Created
attachment 94822
[details]
Patch
WebKit Review Bot
Comment 17
2011-05-25 11:57:22 PDT
Attachment 94822
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 WebKitLibraries/WebKitSystemInterface.h:210: The parameter name "movie" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Darin Adler
Comment 18
2011-05-25 13:24:24 PDT
Comment on
attachment 94822
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=94822&action=review
> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundationObjC.mm:785 > + return requestedURL.host() == resolvedURL.host() && requestedURL.port() == resolvedURL.port();
Normally, security origin checks need to cover protocol as well as host and port. It’s probably safest to do security origin checks by constructing SecurityOrigin objects and using the member functions of the SecurityOrigin class rather than writing custom logic here. But if we have to have custom logic, it should be sure to check protocol.
> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:1621 > + return requestedURL.host() == resolvedURL.host() && requestedURL.port() == resolvedURL.port();
Same comment.
Jer Noble
Comment 19
2011-05-25 14:05:21 PDT
Created
attachment 94853
[details]
Patch
Darin Adler
Comment 20
2011-05-25 14:08:04 PDT
Comment on
attachment 94853
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=94853&action=review
> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundationObjC.mm:786 > + return resolvedOrigin->isSameSchemeHostPort(requestedOrigin.get());
This check is probably too strict. The correct check would probably be to check if the original security origin SecurityOrigin::canRequest the resolved asset URL. The original assertURL doesn’t matter at all. We can live with this for now.
Adam Barth
Comment 21
2011-05-25 14:41:45 PDT
Comment on
attachment 94853
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=94853&action=review
>> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundationObjC.mm:786 >> + return resolvedOrigin->isSameSchemeHostPort(requestedOrigin.get()); > > This check is probably too strict. > > The correct check would probably be to check if the original security origin SecurityOrigin::canRequest the resolved asset URL. The original assertURL doesn’t matter at all. > > We can live with this for now.
Yep.
Jer Noble
Comment 22
2011-05-25 15:53:37 PDT
Committed
r87328
: <
http://trac.webkit.org/changeset/87328
>
Darin Adler
Comment 23
2011-05-26 10:36:23 PDT
When we want to improve the security check further, there are three considerations I can think of: 1) We will have to double check whether canAccess, canRequest, or canDisplay is the correct check to make for media. My initial thought is that it should be canDisplay, despite the fact that my earlier comment mentioned canRequest. Someone needs to think it through and presumably check what we already do in the other media code paths. 2) The canDisplay check establishes that it’s OK for the page to display the media, but I’m not sure if it sufficiently checks that the media can be trusted to affect the page. Sorry this is worded so vaguely; I’m not sure what language to use to talk about this type of issue or how we already deal with it. 3) There are resources other than the mail asset URL and we would like WebKit to perform the security checks for all resources. Even though QuickTime has an ability to do its own security checks, we would instead like to plumb the checks back through to WebKit so it can establish the policy.
Adam Barth
Comment 24
2011-05-26 10:46:47 PDT
I'm not sure I fully understand the issues here.
> 1) We will have to double check whether canAccess, canRequest, or canDisplay is the correct check to make for media. My initial thought is that it should be canDisplay, despite the fact that my earlier comment mentioned canRequest. Someone needs to think it through and presumably check what we already do in the other media code paths.
Certainly we should check whether the web page can display the media (which involves checking each hop in the redirect chain).
> 2) The canDisplay check establishes that it’s OK for the page to display the media, but I’m not sure if it sufficiently checks that the media can be trusted to affect the page. Sorry this is worded so vaguely; I’m not sure what language to use to talk about this type of issue or how we already deal with it.
How can the media affect the page? My understanding is the media cannot script the embedding page, so the affect that it can have on the embedding page is minimal. If the media tries to script the page (i.e., if that's possible), we need to check whether the media can access the page, like we do for iframes.
> 3) There are resources other than the mail asset URL and we would like WebKit to perform the security checks for all resources. Even though QuickTime has an ability to do its own security checks, we would instead like to plumb the checks back through to WebKit so it can establish the policy.
If the media's security context cannot be described by a single URL, it's going to be complicated to do the access checks correctly. For example, images have a notion of whether they have a single security origin. In cases where they have more than one security origin, we just cause all access checks to fail (e.g., tainting canvases). I guess the main thing I don't understand is how media can affect the embedding page. I was under the impression that embedding a video was much like embedding an image. Is there more that media can do to the enclosing page?
Darin Adler
Comment 25
2011-05-26 16:11:49 PDT
(In reply to
comment #24
)
> Certainly we should check whether the web page can display the media (which involves checking each hop in the redirect chain).
Makes sense. We should call canDisplay on each.
> > 2) The canDisplay check establishes that it’s OK for the page to display the media, but I’m not sure if it sufficiently checks that the media can be trusted to affect the page. Sorry this is worded so vaguely; I’m not sure what language to use to talk about this type of issue or how we already deal with it. > > How can the media affect the page? My understanding is the media cannot script the embedding page, so the affect that it can have on the embedding page is minimal. > > If the media tries to script the page (i.e., if that's possible), we need to check whether the media can access the page, like we do for frames.
OK, makes total sense. I think the media can’t. I’m thinking back to QuickTime and the QuickTime plug-in, which at one point could host more-active media with scripts in it. But I don’t think this is true for the media element.
> > 3) There are resources other than the mail asset URL and we would like WebKit to perform the security checks for all resources. Even though QuickTime has an ability to do its own security checks, we would instead like to plumb the checks back through to WebKit so it can establish the policy. > > If the media's security context cannot be described by a single URL, it's going to be complicated to do the access checks correctly. For example, images have a notion of whether they have a single security origin. In cases where they have more than one security origin, we just cause all access checks to fail (e.g., tainting canvases).
I see. This might apply to some media, but again I may be thinking more of the more-complex QuickTime past here.
> I guess the main thing I don't understand is how media can affect the embedding page. I was under the impression that embedding a video was much like embedding an image. Is there more that media can do to the enclosing page?
I think that was my mistake, and the answer is no.
Ademar Reis
Comment 26
2011-06-03 14:02:31 PDT
Revision
r87328
cherry-picked into qtwebkit-2.2 with commit b7169cc <
http://gitorious.org/webkit/qtwebkit/commit/b7169cc
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug