Closed Bug 1532722 Opened 5 years ago Closed 4 years ago

<video crossorigin="use-credentials"> doesn't send cookies during seek

Categories

(Core :: Audio/Video: Playback, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: a.s.datsyuk.s, Assigned: naktinis)

References

Details

Attachments

(2 files)

Attached image Image1.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.119 Safari/537.36

Steps to reproduce:

I have HTML file with element <video crossorigin="use-credentials" ...
Also I have .NET Core RESTfull service, which takes authorization token from the cookie. As soon as GET request contains cookie with authorization token, service may authorize this request.

Actual results:

2 requests to the RESTfull service are made: first one contains cookies, but second one is without cookies. This breaks authorization and video couldn't be displayed.
Actually I downloaded Firefox sources, built them and debugged an issue. I think I have found the reason of the bug, and how it could be fixed.

Here are details of my investigation:

In C++ code (HTMLMediaElement::ChannelLoader()) I see that when <video> element is orginally processed, "crossorigin" attribute is respected, and flag to include cookies is added to securityFlags. So, channel is created with this flag, and thus cookies are successfully passed during first HTTP request to the RESTfull service.

But after this, ChannelMediaResource::Seek function is called, which closes channel and creates new one by calling ChannelMediaResource::RecreateChannel(). RecreateChannel() function has code which is very similar to HTMLMediaElement::ChannelLoader(), but it misses part of the code which checks value of "crossorigin" attribute, and adds flag for including cookies to securityFlag. This leads to the creation of the channel which doesn't send the cookies.

As far as I could see, bug could be fixed by copying flag-setting code from HTMLMediaElement::ChannelLoader() to ChannelMediaResource::RecreateChannel() (with some adaptation of the identifiers).

In fact I did this change locally, and after that problem disappeared. With this fix cookies are sent with each request which is done for <video> element.

If this fix doesn't cause any side effects, which I may not know, I propose to apply it to the official Firefox sources.

Expected results:

I expect <video> element to send cookies during each request which is performed (in case if crossorigin="use-credentials").

Component: Untriaged → Security
OS: Unspecified → All
Hardware: Unspecified → All

Christoph, let me know if the Content Security team can help triage this bug, thanks.

Component: Security → DOM: Security
Flags: needinfo?(ckerschb)
Product: Firefox → Core

(In reply to Ethan Tseng [:ethan] from comment #1)

Christoph, let me know if the Content Security team can help triage this bug, thanks.

Thanks for moving it over into the dom:security component. It shows up in our queue for the weekly triage. FWIW, next weekly triage is scheduled for tuesday.

Flags: needinfo?(ckerschb)
Component: DOM: Security → Audio/Video: Playback

The priority flag is not set for this bug.
:bryce, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(bvandyk)

Thanks for looking into the issue and finding a fix! Do you have the patch that you mention in comment 0? Would you be willing to create a phabricator account and opening a review for the patch?

Flags: needinfo?(bvandyk) → needinfo?(a.s.datsyuk.s)

Glad you could find a fix. By the way, this seems to be a duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=1264990

(In reply to Bryce Seager van Dyk (:bryce) from comment #4)

Thanks for looking into the issue and finding a fix! Do you have the patch that you mention in comment 0? Would you be willing to create a phabricator account and opening a review for the patch?

Actually I don't have it as a patch. Right now I even don't have Firefox sources locally in order to prepare a patch. But I think it shouldn't be a problem to make a fix based on the screenshot.
All the changes I did you may see on the attached screenshot. This is the whole fix. I just copied these 3 lines of code from HTMLMediaElement:ChannelLoader::LoadInternal to the RecreateChannel() method in the file ChannelMediaResource.cpp. And then in the copied code I replaced "aElement" with "element".
About phabricator account I'm not really sure. I never contributed to open source products, so don't know how this should work. But if it is really important, you may send me instructions what should I do - and I will do it.

Flags: needinfo?(a.s.datsyuk.s)
Assignee: nobody → naktinis
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc2fdf33488c
Include cookies in ChannelMediaResource requests when CORS mode is use-credentials. r=bryce
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: