142165 - Don't serialize 'http://host/file.xul' and 'file:///c:/file.xul' [@ js_XDRScript]

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[John Morrison]

With the xul-fastload serialization changes that landed last night, we
are now serializing 'file:///c:/somefile.xul' and 'http://host/somefile.xul'.

A subsequent attempt to load the same resource crashes, even for the minimal
xul file.

<?xml version="1.0"?>
<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
<window
  xmlns:html="http://www.w3.org/1999/xhtml"
  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
    <text value="Some value"/>
</window>

The crash is in DOM code, checking property access restrictions, but the fix
is that resources other than 'chrome://' (and resource:// ??) should not be 
serialized into the XUL cache.

This needs fixing ASAP, since any remote xul will now crash a trunk build.

JSDOM! `string' + 47 bytes
nsWindowSH::AddProperty()
XPC_WN_Helper_AddProperty()
js_DefineNativeProperty()
js_DefineProperty()
js_DefineFunction()
JS_DefineFunction()
nsWindowSH::GlobalResolve()
nsWindowSH::NewResolve()
XPC_WN_Helper_NewResolve()
js_LookupProperty()
js_GetProperty()
JS_GetProperty()
nsDOMClassInfo::PostCreate()
XPCWrappedNative::GetNewOrUsed()
XPCConvert::NativeInterface2JSObject()
nsXPConnect::WrapNative()
nsDOMClassInfo::WrapNative()
nsNodeSH::PreCreate()
XPCWrappedNative::GetNewOrUsed()
XPCConvert::NativeInterface2JSObject()
nsXPConnect::WrapNative()
nsDOMClassInfo::WrapNative()
nsNodeSH::PreCreate()
XPCWrappedNative::GetNewOrUsed()
XPCConvert::NativeInterface2JSObject()
nsXPConnect::WrapNative()
nsXBLProtoImpl::InitTargetObjects()
nsXBLProtoImpl::InstallImplementation()
nsXBLPrototypeBinding::InstallImplementation()
nsXBLBinding::InstallImplementation()
nsXBLBinding::InstallImplementation()
nsXBLService::LoadBindings()
nsCSSFrameConstructor::ConstructFrameInternal()
nsCSSFrameConstructor::ConstructFrame()
nsCSSFrameConstructor::CreateAnonymousFrames()
nsCSSFrameConstructor::CreateAnonymousFrames()
nsCSSFrameConstructor::ConstructDocElementFrame()
nsCSSFrameConstructor::ContentInserted()
StyleSetImpl::ContentInserted()
PresShell::InitialReflow()
nsXULDocument::StartLayout()
nsXULDocument::ResumeWalk()
nsXULDocument::CachedChromeStreamListener::OnStopRequest()
nsDocumentOpenInfo::OnStopRequest()
nsStreamListenerTee::OnStopRequest()
nsHttpChannel::OnStopRequest()
nsOnStopRequestEvent::HandleEvent()
PL_HandleEvent()
PL_ProcessPendingEvents()
_md_EventReceiverProc()
nsAppShellService::Run()
main1()
main()
WinMain()
MOZILLA! WinMainCRTStartup + 308 bytes
KERNEL32!

Comment 1

[John Morrison]

*** Bug 142026 has been marked as a duplicate of this bug. ***

Comment 2

[Brendan Eich [:brendan]]

Darnit, where'd we lose the IsChromeURI predicate safeguard that JS-in-XUL
fastload had?  Ben, see the old fastload code in nsXULDocument.cpp, search for
IsChromeURI.

/be

Comment 3

[Ben Goodger (use ben at mozilla dot org for email)]

Oops. Patch in a sec. 

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch

I'm building now on my PIII-500, so that'll take about an hr. I'm attaching
what I think is the patch anyway, so someone with a current build may wish to
test.

Comment 5

[John Morrison]

okay, on my win2k trunk build from 2am last night, with ben's patch applied,
I no longer crash when loading http://, file://, or ftp:// url's for XUL 
files. Inspection of the XUL.mfl on win2k shows that these resources are not
being deserialized into the fastload file anymore.

So, let's move forward on getting this landed tonight to fix the crash.

However, there's still something wrong:
1) delete XUL.mfl
2) start mozilla; quit; XUL.mfl is about 934973 Bytes
3) start mozilla; quit; XUL.mfl is still the same
4) start mozilla; do View->Page Info'; quit; XUL.mfl expands to 1037729 Bytes
   and updates timestamp
5) start mozilla; quit; XUL.mfl is not modified
6) Here's the problem ... start mozilla; load file:///c:/crash.xul which is 
   any XUL disk file. quit. XUL.mfl does not changes size, but the timestamp
   changes
7) start mozilla; XUL.mfl is invalidated (truncated to 0 and then reinitialized 
   with navigator.xul and friends to 934973 Bytes).

Seems that loaded non-chrome XUL is setting up the fastload file for failure 
on next startup.

Comment 6

[Brendan Eich [:brendan]]

Sounds like nsIFastLoadService::AddDependency is being called for crash.xul even
though it is not loaded via a chrome: URL.  Ben, can you debug and verify that?
 if so, we need an IsChromeURI test in nsChromeProtocolHandler.cpp, which seems
like it should not be necessary -- but jrgm's stack trace here shows a chrome
cache hit for non-chrome: XUL.  What's up with that?

/be

Comment 7

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: better patch

Fix the invalidation problem jrgm mentions.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 82320 [details] [diff] [review]
better patch

sr=[e-mail].

I noticed that nsXULPrototypeCache::GetPrototype calls PutPrototype even if
protoDoc->Read failed.	That's wrong, the cache should not be filled with a
badly-read protoDoc.  Ben, can you fix that here too.

/be

Comment 9

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch to prevent putting bad prototype into cache

Comment 10

[Blake Ross]

Comment on attachment 82576 [details] [diff] [review]
patch to prevent putting bad prototype into cache 

r=blake

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

Fixed. 

Comment 12

[Justin Turner Arthur]

thanks ben!
you dabomb.

Comment 13

[Axel Hecht]

*** Bug 142804 has been marked as a duplicate of this bug. ***

Comment 14

[John Morrison]

*** Bug 142916 has been marked as a duplicate of this bug. ***

Comment 15

[Scott MacGregor]

updating the stack trace in the summary to make it easier for folks to find dups
of this bug.