Closed Bug 478957 Opened 11 years ago Closed 11 years ago
Audit events when loading local files from another domain
When audio/video elements try to load a resource, but access to that resource is restricted by load policy, we should not send any events or make state changes which tell the page that the restricted resource exists.
This patch adds a mochitest that attempts video/audio loads of local sources from another domain while listening on all specified events and polling every 1ms to detect state changes. This test checks that we have the correct events in the correct order, that we don't leak state, and also checks that pages served from another domain can't load local media. This patch is based on top of the patch for bug 465458, which updates the load algorithm to match the current spec. The mochitest passes on Windows and Linux (provided the patch for bug 465458 applied). It's possible that there are weird edge cases that this mochitest misses. I will still go through and audit the code manually, and maybe others want to as well, but this test gives us some confidence and will allows us to detect regressions.
This is cool. typo: "durationchane" You should probably add a test for cross-origin HTTP loads of non-media content. As a separate issue, you could think about what, if anything we might want to hide for cross-origin media loads.
As Patch v1, with spelling error fixed and HTTP cross-domain non-media load test case added.
Attachment #363187 - Flags: superreview?(roc) → superreview+
When reviewing the code, I realized I'd not checked the currentSrc attribute in my mochitest, so I've updated the test to check that. I also expanded a comment about a setTimeout() which Chris Double asked me about.
Result of code audit: * All state changes to networkState > LOADING or readyState > HAVE_NOTHING are initiated by the decoder. * Decoders are only constructed if the content policy does not veto the load, or if the channel has an appropriate mime type. * The only other events sent in nsHTMLMediaElement are loadstart, error, emptied and play. These can be sent regardless of networkState or readyState, and you can't tell the difference between a 404 and a vetoed load with these events. So I think we're ok.
Whiteboard: [sg:investigate] → [sg:investigate][needs landing]
The patch for bug 465458 must land before this one can.
this is ready to land now I guess
Whiteboard: [sg:investigate][needs landing 465458 first!] → [sg:investigate][needs landing]
As patch v3, but unbitrotten. This patch is based on top of bug 479711 and bug 479859. As such, the test is changed slightly, we now assume that networkState can be either <= NETWORK_LOADING, or NETWORK_NO_SOURCE (previously <= NETWORK_LOADING, NETWORK_NO_SOURCE is new). We also now never expect to receive an 'emptied' event, since now this only happens when we get an error during load, or we abort a load because a new one has started.
I left some "dump" debug calls commented in by mistake, turning them off again.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [sg:investigate][needs landing] → [sg:investigate][baking for 1.9.1]
You need to log in before you can comment on or make changes to this bug.