Bug 602115: Fix XSLT error handling bugs. r=peterv a=dveditz
authorJonas Sicking <jonas@sicking.cc>
Mon, 13 Dec 2010 14:24:55 -0800
changeset 27301 4693aab773ece6f3405867b1bfed6e0d96054524
parent 27300 4e5edc8a6382ba14a978ffd4f8701569b75e456c
child 27302 7f3448d6580873671655fe56039f889887471d5d
push id2641
push usersicking@mozilla.com
push dateWed, 19 Jan 2011 18:01:19 +0000
reviewerspeterv, dveditz
bugs602115
milestone1.9.1.17pre
Bug 602115: Fix XSLT error handling bugs. r=peterv a=dveditz
content/xslt/crashtests/602115.html
content/xslt/crashtests/crashtests.list
content/xslt/src/xslt/txExecutionState.cpp
content/xslt/src/xslt/txExecutionState.h
content/xslt/src/xslt/txMozillaXSLTProcessor.cpp
new file mode 100644
--- /dev/null
+++ b/content/xslt/crashtests/602115.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<script>
+
+try {
+  var docType = document.implementation.createDocumentType(undefined, '', '');
+  var doc = document.implementation.createDocument('', '', null);
+  var xp = new XSLTProcessor;
+  xp.importStylesheet(doc);
+  xp.transformToDocument(docType);
+}
+catch (ex) {}
+
+try {
+  docType = document.implementation.createDocumentType(undefined, '', '');
+  doc = document.implementation.createDocument('', '', null);
+  xp = new XSLTProcessor;
+  xp.importStylesheet(doc);
+  xp.transformToFragment(docType, document);
+}
+catch (ex) {}
+
+</script>
--- a/content/xslt/crashtests/crashtests.list
+++ b/content/xslt/crashtests/crashtests.list
@@ -3,8 +3,9 @@ load 111994.xml
 load 226425.xml
 load 406106-1.html
 load 483444.xml
 load 485217.xml
 load 485286.xml
 load 528300.xml
 load 528488.xml
 load 545927.html
+load 602115.html
--- a/content/xslt/src/xslt/txExecutionState.cpp
+++ b/content/xslt/src/xslt/txExecutionState.cpp
@@ -85,41 +85,32 @@ txLoadedDocumentsHash::~txLoadedDocument
 }
 
 txExecutionState::txExecutionState(txStylesheet* aStylesheet,
                                    PRBool aDisableLoads)
     : mStylesheet(aStylesheet),
       mNextInstruction(nsnull),
       mLocalVariables(nsnull),
       mRecursionDepth(0),
-      mTemplateRules(nsnull),
-      mTemplateRulesBufferSize(0),
-      mTemplateRuleCount(0),
       mEvalContext(nsnull),
       mInitialEvalContext(nsnull),
       mGlobalParams(nsnull),
       mKeyHash(aStylesheet->getKeyMap()),
       mDisableLoads(aDisableLoads)
 {
     MOZ_COUNT_CTOR(txExecutionState);
 }
 
 txExecutionState::~txExecutionState()
 {
     MOZ_COUNT_DTOR(txExecutionState);
 
     delete mResultHandler;
     delete mLocalVariables;
     delete mEvalContext;
-
-    PRInt32 i;
-    for (i = 0; i < mTemplateRuleCount; ++i) {
-        NS_IF_RELEASE(mTemplateRules[i].mModeLocalName);
-    }
-    delete [] mTemplateRules;
     
     txStackIterator varsIter(&mLocalVarsStack);
     while (varsIter.hasNext()) {
         delete (txVariableMap*)varsIter.next();
     }
 
     txStackIterator contextIter(&mEvalContextStack);
     while (contextIter.hasNext()) {
@@ -199,17 +190,21 @@ txExecutionState::init(const txXPathNode
     NS_ENSURE_SUCCESS(rv, rv);
 
     return runTemplate(templ);
 }
 
 nsresult
 txExecutionState::end(nsresult aResult)
 {
-    popTemplateRule();
+    NS_ASSERTION(NS_FAILED(aResult) || mTemplateRules.Length() == 1,
+                 "Didn't clean up template rules properly");
+    if (NS_SUCCEEDED(aResult)) {
+        popTemplateRule();
+    }
     return mOutputHandler->endDocument(aResult);
 }
 
 
 
 nsresult
 txExecutionState::getVariable(PRInt32 aNamespace, nsIAtom* aLName,
                               txAExprResult*& aResult)
@@ -403,45 +398,28 @@ txExecutionState::popResultHandler()
     return oldHandler;
 }
 
 nsresult
 txExecutionState::pushTemplateRule(txStylesheet::ImportFrame* aFrame,
                                    const txExpandedName& aMode,
                                    txVariableMap* aParams)
 {
-    if (mTemplateRuleCount == mTemplateRulesBufferSize) {
-        PRInt32 newSize =
-            mTemplateRulesBufferSize ? mTemplateRulesBufferSize * 2 : 10;
-        TemplateRule* newRules = new TemplateRule[newSize];
-        NS_ENSURE_TRUE(newRules, NS_ERROR_OUT_OF_MEMORY);
-        
-        memcpy(newRules, mTemplateRules,
-               mTemplateRuleCount * sizeof(TemplateRule));
-        delete [] mTemplateRules;
-        mTemplateRules = newRules;
-        mTemplateRulesBufferSize = newSize;
-    }
-
-    mTemplateRules[mTemplateRuleCount].mFrame = aFrame;
-    mTemplateRules[mTemplateRuleCount].mModeNsId = aMode.mNamespaceID;
-    mTemplateRules[mTemplateRuleCount].mModeLocalName = aMode.mLocalName;
-    mTemplateRules[mTemplateRuleCount].mParams = aParams;
-    NS_IF_ADDREF(mTemplateRules[mTemplateRuleCount].mModeLocalName);
-    ++mTemplateRuleCount;
-    
-    return NS_OK;
+    TemplateRule* rule = mTemplateRules.AppendElement();
+    rule->mFrame = aFrame;
+    rule->mModeNsId = aMode.mNamespaceID;
+    rule->mModeLocalName = aMode.mLocalName;
+    rule->mParams = aParams;
 }
 
 void
 txExecutionState::popTemplateRule()
 {
-    // decrement outside of RELEASE, that would decrement twice
-    --mTemplateRuleCount;
-    NS_IF_RELEASE(mTemplateRules[mTemplateRuleCount].mModeLocalName);
+    NS_PRECONDITION(!mTemplateRules.IsEmpty(), "No rules to pop");
+    mTemplateRules.RemoveElementAt(mTemplateRules.Length() - 1);
 }
 
 txIEvalContext*
 txExecutionState::getEvalContext()
 {
     return mEvalContext;
 }
 
@@ -495,17 +473,18 @@ txExecutionState::getKeyNodes(const txEx
 {
     return mKeyHash.getKeyNodes(aKeyName, aRoot, aKeyValue,
                                 aIndexIfNotFound, *this, aResult);
 }
 
 txExecutionState::TemplateRule*
 txExecutionState::getCurrentTemplateRule()
 {
-    return mTemplateRules + mTemplateRuleCount - 1;
+    NS_PRECONDITION(!mTemplateRules.IsEmpty(), "No current rule!");
+    return &mTemplateRules[mTemplateRules.Length() - 1];
 }
 
 txInstruction*
 txExecutionState::getNextInstruction()
 {
     txInstruction* instr = mNextInstruction;
     if (instr) {
         mNextInstruction = instr->mNext;
--- a/content/xslt/src/xslt/txExecutionState.h
+++ b/content/xslt/src/xslt/txExecutionState.h
@@ -99,20 +99,21 @@ public:
                   txOwningExpandedNameMap<txIGlobalParameter>* aGlobalParams);
     nsresult end(nsresult aResult);
 
     TX_DECL_MATCH_CONTEXT;
 
     /**
      * Struct holding information about a current template rule
      */
-    struct TemplateRule {
+    class TemplateRule {
+    public:
         txStylesheet::ImportFrame* mFrame;
         PRInt32 mModeNsId;
-        nsIAtom* mModeLocalName;
+        nsCOMPtr<nsIAtom> mModeLocalName;
         txVariableMap* mParams;
     };
 
     // Stack functions
     nsresult pushEvalContext(txIEvalContext* aContext);
     txIEvalContext* popEvalContext();
     nsresult pushBool(PRBool aBool);
     PRBool popBool();
@@ -168,19 +169,17 @@ private:
     txStack mResultHandlerStack;
     txStack mParamStack;
     txInstruction* mNextInstruction;
     txVariableMap* mLocalVariables;
     txVariableMap mGlobalVariableValues;
     nsRefPtr<txAExprResult> mGlobalVarPlaceholderValue;
     PRInt32 mRecursionDepth;
 
-    TemplateRule* mTemplateRules;
-    PRInt32 mTemplateRulesBufferSize;
-    PRInt32 mTemplateRuleCount;
+    nsAutoTArray<TemplateRule, 10> mTemplateRules;
 
     txIEvalContext* mEvalContext;
     txIEvalContext* mInitialEvalContext;
     //Document* mRTFDocument;
     txOwningExpandedNameMap<txIGlobalParameter>* mGlobalParams;
 
     txLoadedDocumentsHash mLoadedDocuments;
     txKeyHash mKeyHash;
--- a/content/xslt/src/xslt/txMozillaXSLTProcessor.cpp
+++ b/content/xslt/src/xslt/txMozillaXSLTProcessor.cpp
@@ -672,20 +672,22 @@ txMozillaXSLTProcessor::TransformToDoc(n
     txExecutionState es(mStylesheet, IsLoadDisabled());
 
     // XXX Need to add error observers
 
     txToDocHandlerFactory handlerFactory(&es, sourceDOMDocument, aOutputDoc,
                                          mObserver);
     es.mOutputHandlerFactory = &handlerFactory;
 
-    es.init(*sourceNode, &mVariables);
+    nsresult rv = es.init(*sourceNode, &mVariables);
 
     // Process root of XML source document
-    nsresult rv = txXSLTProcessor::execute(es);
+    if (NS_SUCCEEDED(rv)) {
+        rv = txXSLTProcessor::execute(es);
+    }
     
     nsresult endRv = es.end(rv);
     if (NS_SUCCEEDED(rv)) {
       rv = endRv;
     }
     
     if (NS_SUCCEEDED(rv)) {
         if (aResult) {
@@ -729,20 +731,22 @@ txMozillaXSLTProcessor::TransformToFragm
 
     // XXX Need to add error observers
 
     rv = aOutput->CreateDocumentFragment(aResult);
     NS_ENSURE_SUCCESS(rv, rv);
     txToFragmentHandlerFactory handlerFactory(*aResult);
     es.mOutputHandlerFactory = &handlerFactory;
 
-    es.init(*sourceNode, &mVariables);
+    rv = es.init(*sourceNode, &mVariables);
 
     // Process root of XML source document
-    rv = txXSLTProcessor::execute(es);
+    if (NS_SUCCEEDED(rv)) {
+        rv = txXSLTProcessor::execute(es);
+    }
     // XXX setup exception context, bug 204658
     nsresult endRv = es.end(rv);
     if (NS_SUCCEEDED(rv)) {
       rv = endRv;
     }
 
     return rv;
 }