Closed
Bug 842561
Opened 11 years ago
Closed 11 years ago
Assert/ensure that the inheritance chain for DOM objects matches the interface inheritance chain
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla22
Tracking | Status | |
---|---|---|
firefox18 | --- | unaffected |
firefox19 | --- | unaffected |
firefox20 | + | fixed |
firefox21 | + | fixed |
firefox22 | --- | fixed |
firefox-esr17 | --- | unaffected |
b2g18 | --- | unaffected |
b2g18-v1.0.0 | --- | unaffected |
b2g18-v1.0.1 | --- | unaffected |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
(Keywords: sec-high, Whiteboard: [adv-main20-])
Attachments
(1 file, 1 obsolete file)
11.63 KB,
patch
|
peterv
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bit us in bug 842089. I'm adding some checks which make the code in that bug not compile. They _also_ make CDATASection not compile, hence the security-sensitive bit. I _think_ in practice the code in CDATASection is maybe safe. Maybe. I still think we should take this on 20 and 21, which is where bug 826703 was fixed...
![]() |
Assignee | |
Comment 1•11 years ago
|
||
to return an already_AddRefed<Text>, but then we'd need more casting in nsDocument.cpp for the XPCOM CreateTextNode. Not sure which way is better, really.
Attachment #715516 -
Flags: review?(peterv)
![]() |
Assignee | |
Comment 2•11 years ago
|
||
Try run at https://tbpl.mozilla.org/?tree=Try&rev=724f14bb5a81
tracking-firefox20:
--- → ?
tracking-firefox21:
--- → ?
Comment 3•11 years ago
|
||
Could this be a static assert?
![]() |
Assignee | |
Comment 4•11 years ago
|
||
No, because you can't use static_cast in static asserts. Yes, that's moronic; that's C++ for you. The assert code here will fail to compile at all in cases like bug 842089, so will catch those at compile time. It'll only be able to catch incorrect object layout at runtime, though.
![]() |
Assignee | |
Comment 5•11 years ago
|
||
That fails the asserts because nsContentList does not in fact reinterpret_cast to nsIHTMLCollection. But I guess that requirement is too strict in any case. Fix coming up that doesn't impose it.
Comment 6•11 years ago
|
||
Pessimistically marking this sec-high, based on comment 0.
status-b2g18:
--- → unaffected
status-b2g18-v1.0.0:
--- → unaffected
status-b2g18-v1.0.1:
--- → unaffected
status-firefox18:
--- → unaffected
status-firefox19:
--- → unaffected
status-firefox20:
--- → affected
status-firefox21:
--- → affected
status-firefox-esr17:
--- → unaffected
Keywords: sec-high
![]() |
Assignee | |
Comment 7•11 years ago
|
||
Attachment #715573 -
Flags: review?(peterv)
![]() |
Assignee | |
Updated•11 years ago
|
Attachment #715516 -
Attachment is obsolete: true
Attachment #715516 -
Flags: review?(peterv)
![]() |
Assignee | |
Comment 8•11 years ago
|
||
New try run: https://tbpl.mozilla.org/?tree=Try&rev=362dd168fb25
Updated•11 years ago
|
Updated•11 years ago
|
Attachment #715573 -
Flags: review?(peterv) → review+
![]() |
Assignee | |
Comment 9•11 years ago
|
||
Comment on attachment 715573 [details] [diff] [review] With that fixed [Approval Request Comment] Bug caused by (feature/regressing bug #): 826703 User impact if declined: Risk of exploitable crashes, though I'm not entirely certain this can actually happen. Testing completed (on m-c, etc.): Passes our regression tests. Risk to taking this patch (and alternatives if risky): I believe the risk here is fairly low. String or UUID changes made by this patch: None
Attachment #715573 -
Flags: approval-mozilla-beta?
Attachment #715573 -
Flags: approval-mozilla-aurora?
![]() |
Assignee | |
Comment 10•11 years ago
|
||
Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/b9b189c8951b
![]() |
Assignee | |
Updated•11 years ago
|
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla22
Comment 11•11 years ago
|
||
Unfortunately, this was backed out due to one of the changesets in the push making browser_dbg_bug723069_editor-breakpoints.js fail frequently on Windows and OSX opt builds. https://hg.mozilla.org/integration/mozilla-inbound/rev/ca34c11bf55d https://tbpl.mozilla.org/php/getParsedLog.php?id=19995636&tree=Mozilla-Inbound 11:12:16 INFO - TEST-PASS | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | editor.getBreakpoints().length is correct 11:12:16 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | remove the second breakpoint using the mouse 11:12:16 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | Console message: [JavaScript Warning: "Use of attributes' specified attribute is deprecated. It always returns true." {file: "chrome://browser/content/orion.js" line: 6341}] 11:12:16 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | Console message: [JavaScript Warning: "Use of removeAttributeNode() is deprecated. Use removeAttribute() instead." {file: "chrome://browser/content/orion.js" line: 6342}] 11:12:17 INFO - DBG-FRONTEND: Updating the DebuggerView editor: http://example.com/browser/browser/devtools/debugger/test/test-script-switching-02.js @ 6, flags: ({noSwitch:true}) 11:12:17 INFO - DBG-FRONTEND: Updating the DebuggerView editor: http://example.com/browser/browser/devtools/debugger/test/test-script-switching-02.js @ 6, flags: ({noSwitch:true}) 11:12:17 INFO - DBG-FRONTEND: Updating the DebuggerView editor: http://example.com/browser/browser/devtools/debugger/test/test-script-switching-02.js @ 6, flags: ({noSwitch:true}) 11:12:17 INFO - DBG-FRONTEND: Updating the DebuggerView editor: http://example.com/browser/browser/devtools/debugger/test/test-script-switching-02.js @ 6, flags: ({noSwitch:true}) 11:12:46 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | Test timed out 11:12:46 WARNING - This is a harness error. 11:12:46 INFO - args: ['/usr/sbin/screencapture', '-C', '-x', '-t', 'png', '/var/folders/qd/srwd5f710sj0fcl9z464lkj00000gn/T/mozilla-test-fail_GzeI8j'] 11:12:48 INFO - SCREENSHOT: <see log> 11:12:48 INFO - DBG-FRONTEND: Destroying the DebuggerView 11:12:48 INFO - DBG-FRONTEND: Destroying the ToolbarView 11:12:48 INFO - DBG-FRONTEND: Destroying the OptionsView 11:12:48 INFO - DBG-FRONTEND: Destroying the ChromeGlobalsView 11:12:48 INFO - DBG-FRONTEND: Destroying the SourcesView 11:12:48 INFO - DBG-FRONTEND: Destroying the FilterView 11:12:48 INFO - DBG-FRONTEND: Destroying the FilteredSourcesView 11:12:48 INFO - DBG-FRONTEND: Destroying the StackFramesView 11:12:48 INFO - DBG-FRONTEND: Destroying the BreakpointsView 11:12:48 INFO - DBG-FRONTEND: Destroying the WatchExpressionsView 11:12:48 INFO - DBG-FRONTEND: Destroying the GlobalSearchView 11:12:48 INFO - DBG-FRONTEND: Destroying the DebuggerView window 11:12:48 INFO - DBG-FRONTEND: Destroying the DebuggerView panes 11:12:48 INFO - DBG-FRONTEND: Destroying the DebuggerView editor 11:12:48 INFO - DBG-FRONTEND: SourceScripts is disconnecting... 11:12:48 INFO - DBG-FRONTEND: StackFrames is disconnecting... 11:12:48 INFO - DBG-FRONTEND: ThreadState is disconnecting... 11:12:48 INFO - TEST-PASS | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | correct number of breakpoints have been added 11:12:48 INFO - TEST-PASS | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | correct number of breakpoints have been removed 11:12:48 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | correct number of editor breakpoint changes - Got 3, expected 4 11:12:48 WARNING - This is a harness error. 11:12:48 INFO - Stack trace: 11:12:48 INFO - JS frame :: chrome://mochikit/content/browser-test.js :: test_is :: line 486 11:12:48 INFO - JS frame :: chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js :: <TOP_LEVEL> :: line 295 11:12:48 INFO - JS frame :: chrome://mochikit/content/browser-test.js :: Tester_nextTest :: line 250 11:12:48 INFO - JS frame :: chrome://mochikit/content/browser-test.js :: <TOP_LEVEL> :: line 419 11:12:48 INFO - native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0 11:12:48 INFO - INFO TEST-END | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_bug723069_editor-breakpoints.js | finished in 30099ms
![]() |
Assignee | |
Comment 12•11 years ago
|
||
Confirmed via try that this bug is not responsible for that orange (as far as I can tell) and pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/62f3d4a4421a
https://hg.mozilla.org/mozilla-central/rev/62f3d4a4421a
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 14•11 years ago
|
||
Comment on attachment 715573 [details] [diff] [review] With that fixed passes tests, low risk, early enough in the cycle to take this for uplift.
Attachment #715573 -
Flags: approval-mozilla-beta?
Attachment #715573 -
Flags: approval-mozilla-beta+
Attachment #715573 -
Flags: approval-mozilla-aurora?
Attachment #715573 -
Flags: approval-mozilla-aurora+
Comment 15•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/e485a12e27ae https://hg.mozilla.org/releases/mozilla-beta/rev/72293b033da5
Comment 16•11 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #15) > https://hg.mozilla.org/releases/mozilla-aurora/rev/e485a12e27ae > https://hg.mozilla.org/releases/mozilla-beta/rev/72293b033da5 Backed out for debug bustage. The beta backout might have been a bit premature, but I decided better to keep the two together. https://hg.mozilla.org/releases/mozilla-aurora/rev/31d3b7ffa704 https://hg.mozilla.org/releases/mozilla-beta/rev/771f5f0cfce6 https://tbpl.mozilla.org/php/getParsedLog.php?id=20069736&tree=Mozilla-Aurora MutationObserverBinding.cpp LocalMediaStreamBinding.cpp: In function 'JSObject* mozilla::dom::LocalMediaStreamBinding::Wrap(JSContext*, JSObject*, mozilla::DOMLocalMediaStream*, nsWrapperCache*, bool*)': LocalMediaStreamBinding.cpp:242:61: error: invalid static_cast from type 'mozilla::DOMLocalMediaStream*' to type 'mozilla::dom::EventTarget*' make[6]: *** [LocalMediaStreamBinding.o] Error 1 make[6]: *** Waiting for unfinished jobs.... MediaStreamBinding.cpp: In function 'JSObject* mozilla::dom::MediaStreamBinding::Wrap(JSContext*, JSObject*, mozilla::DOMMediaStream*, nsWrapperCache*, bool*)': MediaStreamBinding.cpp:241:61: error: invalid static_cast from type 'mozilla::DOMMediaStream*' to type 'mozilla::dom::EventTarget*' make[6]: *** [MediaStreamBinding.o] Error 1 make[6]: Leaving directory `/builds/slave/m-aurora-lx-d-0000000000000000/build/obj-firefox/dom/bindings' make[5]: *** [bindings_libs] Error 2
![]() |
Assignee | |
Comment 17•11 years ago
|
||
Ryan, this bug depends on bug 842089; see comment 0 and dependencies. Did you push it before pushing that bug?
Comment 18•11 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #17) > Ryan, this bug depends on bug 842089; see comment 0 and dependencies. Did > you push it before pushing that bug? Argh, fail. https://hg.mozilla.org/releases/mozilla-aurora/rev/6b56e7dcd0c6 https://hg.mozilla.org/releases/mozilla-beta/rev/134b35ff1447
Updated•11 years ago
|
Whiteboard: [adv-main20-]
Updated•10 years ago
|
Group: core-security
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•