Closed
Bug 428288
Opened 16 years ago
Closed 16 years ago
[@ NS_GetInnermostURI - nsDocShell::ValidateOrigin]
Categories
(Core :: DOM: Navigation, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: timeless, Assigned: dveditz)
References
Details
(Keywords: crash, regression, topcrash)
Crash Data
Attachments
(2 files, 1 obsolete file)
1.17 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
2.04 KB,
patch
|
Details | Diff | Splinter Review |
180 NS_IMETHODIMP 181 nsSystemPrincipal::GetURI(nsIURI** aURI) 182 { 183 *aURI = nsnull; 184 return NS_OK; 185 } Assuming GetURI will result in a non null uri is clearly not a good idea. Signature NS_GetInnermostURI(nsIURI*) UUID 6fa70ccc-06e9-11dd-8a6c-001cc4e2bf68 Time 2008-04-10 02:30:32-07:00 Uptime 3424 Product Firefox Version 3.0pre Build ID 2008040907 OS Windows NT OS Version 5.1.2600 Service Pack 2 CPU x86 CPU Info GenuineIntel family 6 model 13 stepping 8 Crash Reason EXCEPTION_ACCESS_VIOLATION Crash Address 0x0 Comments Crashing Thread Frame Module Signature [Expand] Source ntdll!KiUserExceptionDispatcher+0xe xul!NS_GetInnermostURI(class nsIURI * uri = 0x00000000)+0x1b [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\obj-fx-trunk\dist\include\necko\nsnetutil.h @ 1410] xul!nsDocShell::ValidateOrigin(class nsIDocShellTreeItem * aOriginTreeItem = 0x0384ffd4, class nsIDocShellTreeItem * aTargetTreeItem = 0x00f972b4)+0x1b2 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\docshell\base\nsdocshell.cpp @ 1090] xul!nsDocShell::CanAccessItem(class nsIDocShellTreeItem * aTargetItem = 0x00f972b4, class nsIDocShellTreeItem * aAccessingItem = 0x0384ffd4, int aConsiderOpener = 1)+0x68 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\docshell\base\nsdocshell.cpp @ 1972] xul!nsDocShell::FindItemWithName(wchar_t * aName = 0x0012d204 "_content", class nsISupports * aRequestor = 0x00000000, class nsIDocShellTreeItem * aOriginalRequestor = 0x0384ffd4, class nsIDocShellTreeItem ** _retval = 0x0012d084)+0x160 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\docshell\base\nsdocshell.cpp @ 2105] xul!nsDocShell::InternalLoad+0x296a24 xul!nsCOMPtr_base::assign_from_qi(class nsQueryInterface qi = class nsQueryInterface, struct nsID * iid = 0x00000000)+0x40 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\obj-fx-trunk\xpcom\build\nscomptr.cpp @ 98] xul!NS_TableDrivenQI(void * aThis = 0x24448b10, struct QITableEntry * entries = 0x0012cef4, struct nsID * aIID = 0x00000000, void ** aInstancePtr = 0x0012cf1c)+0x55 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\obj-fx-trunk\xpcom\build\nsisupportsimpl.cpp @ 50] xul!nsHTMLAnchorElement::QueryInterface(struct nsID * aIID = 0xffffb2e8, void ** aInstancePtr = 0x04c25eff)+0x46 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\content\html\content\src\nshtmlanchorelement.cpp @ 163] I was using venkman at the time :)
Flags: blocking1.9?
dveditz wanted steps: 1. open venkman 2. click either of: <http://www.hacksrus.com/~ginda/venkman/faq/venkman-faq.html> <http://www.mozilla.org/projects/venkman/> expected results: no crash. actual results: crash.
Keywords: topcrash
the expected results is that the chrome frame w/ the help text is not a valid match for the target (return false), so the engine will search up until it finds no matches in the window, and then ask the engine if any other windows can handle browsing (true for firefox/seamonkey, false for most others). if it can't find a browsing handler, it'll use exthandler and eventually ask the os to handle the url. In the old code, since x-jsd!=http, the code returned false. The current code would return false if any of the pointers were null, so let's let them be null.
Assignee | ||
Comment 3•16 years ago
|
||
Attachment #314867 -
Flags: superreview?(jst)
Attachment #314867 -
Flags: review?(jst)
Assignee | ||
Comment 4•16 years ago
|
||
Comment on attachment 314867 [details] [diff] [review] Watch out for URI-less system principals Requesting approval before reviews to see if this is something that will be allowed in.
Attachment #314867 -
Flags: approval1.9?
Updated•16 years ago
|
Attachment #314867 -
Flags: superreview?(jst)
Attachment #314867 -
Flags: superreview+
Attachment #314867 -
Flags: review?(jst)
Attachment #314867 -
Flags: review+
Assignee | ||
Comment 5•16 years ago
|
||
I don't know how "top" this crash is--it's around #78--but it's a safe fix and a guaranteed crash for any extension that does this kind of thing. #78 crash is a pretty impressive showing if the problem is limited to Venkman help.
Assignee | ||
Updated•16 years ago
|
Assignee: timeless → dveditz
Status: ASSIGNED → NEW
Comment 6•16 years ago
|
||
Can we get a test?
Comment 7•16 years ago
|
||
Comment on attachment 314867 [details] [diff] [review] Watch out for URI-less system principals I think Damon meant: tests would be nice, but it's pretty easy to see the value of these null checks!
Attachment #314867 -
Flags: approval1.9? → approval1.9+
Attachment #314866 -
Attachment description: use null checks → use null checks (identical to the next patch)
Attachment #314866 -
Attachment is obsolete: true
Attachment #314866 -
Flags: review?(dveditz)
Assignee | ||
Comment 8•16 years ago
|
||
Assignee | ||
Comment 9•16 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Flags: in-testsuite+
Updated•13 years ago
|
Crash Signature: [@ NS_GetInnermostURI - nsDocShell::ValidateOrigin]
You need to log in
before you can comment on or make changes to this bug.
Description
•