283691 - XPConnect or xpt* or something needs to defend against warring .xpt files

Status: RESOLVED INCOMPLETE

(Core :: XPConnect, defect)

Description

[Brendan Eich [:brendan]]

in particular, two pieces of interface info for one interface name.

See bug 280084.  This can and does happen due to installing the zip or tarball
build and then running the installer targeting the same install dir.  We should
fix the build process so the zip build picks up the merged .xpt files that go in
the installer, but we also want to bulletproof the platform.  That's what this
bug is all about.

dbaron suggests noticing the conflict and throwing out both interface infos with
a nasty big warning, followed by coverage-dependent JS errors due to no such
interface.

bryner suggests making XPConnect cross-check the UUID from the .xpt file against
what the implementation being accessed supports via QI.  This would have saved
us in bug 280084 as dveditz did rev the UUID when he added an attribute in the
middle of nsIAutoCompleteInput.

Other ideas welcome.

/be

Comment 1

[Mike Shaver (:shaver emeritus)]

When would we cross-check the XPT's UUID?  I hope not on every method return, as
the XPConnect-thunk penalty is already pretty high.  (C++ code would have the
same problem, of course, since people typically do not QI return values to the
declared static type.)

http://lxr.mozilla.org/mozilla/source/xpcom/reflect/xptinfo/src/xptiInterfaceInfoManager.cpp#1297
has a duplicate-name check that looks quite suitable for beefing up and wiring
to an error report of some kind.

Adding some "verify installation" option to the installer which invoked a full
XPT scan as well as other sanity checks would not offend me either, though
clearly that's another bug.

Comment 2

[Jay Patel [:jay]]

Although bug 280084 greatly reduced the number of
"nsAutoCompleteController::HandleEnter" crashes in Firefox 1.0.1, there are
still a huge number of crashes being reported.  Looks like the patch in that bug
only fixed a few of the cases...so this is a critical bug to fix for the next
release.

Nominating for 1.1. (can't set the flag for 1.0.3, so if someone can do that for
me that would be great. this would be nice to get into the next security release
if there is one)

Comment 3

[Asa Dotzler [:asa]]

Shaver, any chance you'll be able to get something here for 1.1? If not, I think
this probably falls over the fence.

Comment 4

[Mike Shaver (:shaver emeritus)]

There's a chance, sure.  What do we want to do in the case of a duplicate name
(with differing IID, or other differences)?  We don't have very many exciting
logging or notification choices available that early in startup, and the APIs
xptiInterfaceInfoManager has make it hard to propagate a specific error back up
to a caller that might do something more interesting.

Can someone put together a test case for this, in the form of some xpt files to
drop in that trigger this case?

Comment 5

[Worcester12345]

Should this bug just be closed or marked "WONTFIX"?