Skip to content

Commit

Permalink
WebCore::XMLDocumentParserScope does not restore xmlStructuredErrorCo…
Browse files Browse the repository at this point in the history
…ntext properly

<https://bugs.webkit.org/show_bug.cgi?id=273665>
<rdar://127468626>

Reviewed by Alex Christensen.

Prior to this change, XMLDocumentParserScope would restore
xmlStructuredErrorContext using the saved value of
xmlGenericErrorContext.

The fix is to store the original value of xmlStructuredErrorContext in
its own instance variable so it can be restored separately.

If the structuredErrorContext parameter is not set when calling the
XMLDocumentParserScope() constructor, the value of genericErrorContext
is used for structuredErrorContext (as before).

API Test:
    TestWebKitAPI.XMLParsing.WebCoreRestoresLibxml2GenericAndStructuredErrorState

* Source/WebCore/xml/parser/XMLDocumentParserScope.cpp:
(WebCore::XMLDocumentParserScope::XMLDocumentParserScope):
- Add structuredErrorContext argument to constructor so that the
  single-argument constructor can pass in the correct value.
- Store xmlStructuredErrorContext in m_oldStructuredErrorContext.
- Set the structuredErrorContext using that argument, or fall back to
  genericErrorContext if structuredErrorContext is nullptr.
(WebCore::XMLDocumentParserScope::~XMLDocumentParserScope):
- Properly restore saved structuredErrorContext in
  m_oldStructuredErrorContext in the destructor.
* Source/WebCore/xml/parser/XMLDocumentParserScope.h:
(WebCore::XMLDocumentParserScope::XMLDocumentParserScope):
- Declare optional structuredErrorContext argument.
- Rename m_oldErrorContext to m_oldGenericErrorContext.
- Add m_oldStructuredErrorContext.

* Tools/TestWebKitAPI/Tests/WebCore/cocoa/XMLParsing.mm:
(TestWebKitAPI::TEST_F(XMLParsing, WebCoreRestoresLibxml2GenericAndStructuredErrorState)): Add.
- Add test for this change.

Canonical link: https://commits.webkit.org/278436@main
  • Loading branch information
David Kilzer authored and ddkilzer committed May 7, 2024
1 parent 6dc831d commit c864765
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 9 deletions.
15 changes: 8 additions & 7 deletions Source/WebCore/xml/parser/XMLDocumentParserScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ WeakPtr<CachedResourceLoader>& XMLDocumentParserScope::currentCachedResourceLoad

#if ENABLE(XSLT)
XMLDocumentParserScope::XMLDocumentParserScope(CachedResourceLoader* cachedResourceLoader)
: XMLDocumentParserScope(cachedResourceLoader, xmlGenericError, xmlStructuredError, xmlGenericErrorContext)
: XMLDocumentParserScope(cachedResourceLoader, xmlGenericError, xmlStructuredError, xmlGenericErrorContext, xmlStructuredErrorContext)
{
}
#else
Expand All @@ -54,20 +54,21 @@ XMLDocumentParserScope::XMLDocumentParserScope(CachedResourceLoader* cachedResou
#endif // ENABLE(XSLT)

#if ENABLE(XSLT)
XMLDocumentParserScope::XMLDocumentParserScope(CachedResourceLoader* cachedResourceLoader, xmlGenericErrorFunc genericErrorFunc, xmlStructuredErrorFunc structuredErrorFunc, void* errorContext)
XMLDocumentParserScope::XMLDocumentParserScope(CachedResourceLoader* cachedResourceLoader, xmlGenericErrorFunc genericErrorFunc, xmlStructuredErrorFunc structuredErrorFunc, void* genericErrorContext, void* structuredErrorContext)
: m_oldCachedResourceLoader(currentCachedResourceLoader())
, m_oldGenericErrorFunc(xmlGenericError)
, m_oldStructuredErrorFunc(xmlStructuredError)
, m_oldErrorContext(xmlGenericErrorContext)
, m_oldGenericErrorContext(xmlGenericErrorContext)
, m_oldStructuredErrorContext(xmlStructuredErrorContext)
{
initializeXMLParser();
m_oldEntityLoader = xmlGetExternalEntityLoader();
currentCachedResourceLoader() = cachedResourceLoader;
xmlSetExternalEntityLoader(WebCore::externalEntityLoader);
if (genericErrorFunc)
xmlSetGenericErrorFunc(errorContext, genericErrorFunc);
xmlSetGenericErrorFunc(genericErrorContext, genericErrorFunc);
if (structuredErrorFunc)
xmlSetStructuredErrorFunc(errorContext, structuredErrorFunc);
xmlSetStructuredErrorFunc(structuredErrorContext ?: genericErrorContext, structuredErrorFunc);
}
#endif

Expand All @@ -76,8 +77,8 @@ XMLDocumentParserScope::~XMLDocumentParserScope()
currentCachedResourceLoader() = m_oldCachedResourceLoader;
xmlSetExternalEntityLoader(m_oldEntityLoader);
#if ENABLE(XSLT)
xmlSetGenericErrorFunc(m_oldErrorContext, m_oldGenericErrorFunc);
xmlSetStructuredErrorFunc(m_oldErrorContext, m_oldStructuredErrorFunc);
xmlSetGenericErrorFunc(m_oldGenericErrorContext, m_oldGenericErrorFunc);
xmlSetStructuredErrorFunc(m_oldStructuredErrorContext, m_oldStructuredErrorFunc);
#endif
}

Expand Down
5 changes: 3 additions & 2 deletions Source/WebCore/xml/parser/XMLDocumentParserScope.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class XMLDocumentParserScope {
static WeakPtr<CachedResourceLoader>& currentCachedResourceLoader();

#if ENABLE(XSLT)
XMLDocumentParserScope(CachedResourceLoader*, xmlGenericErrorFunc, xmlStructuredErrorFunc = nullptr, void* errorContext = nullptr);
XMLDocumentParserScope(CachedResourceLoader*, xmlGenericErrorFunc, xmlStructuredErrorFunc = nullptr, void* genericErrorContext = nullptr, void* structuredErrorContext = nullptr);
#endif

private:
Expand All @@ -57,7 +57,8 @@ class XMLDocumentParserScope {
#if ENABLE(XSLT)
xmlGenericErrorFunc m_oldGenericErrorFunc { nullptr };
xmlStructuredErrorFunc m_oldStructuredErrorFunc { nullptr };
void* m_oldErrorContext { nullptr };
void* m_oldGenericErrorContext { nullptr };
void* m_oldStructuredErrorContext { nullptr };
#endif
};

Expand Down
32 changes: 32 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebCore/cocoa/XMLParsing.mm
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,38 @@ void SetUp() final
}
};

TEST_F(XMLParsing, WebCoreRestoresLibxml2GenericAndStructuredErrorState)
{
void* origGenericErrorContext = xmlGenericErrorContext;
void* origStructuredErrorContext = xmlStructuredErrorContext;
xmlGenericErrorFunc origGenericErrorFunc = xmlGenericError;
xmlStructuredErrorFunc origStructuredErrorFunc = xmlStructuredError;

void* testGenericErrorContext = reinterpret_cast<void*>(0xaaaaaaaa);
void* testStructuredErrorContext = reinterpret_cast<void*>(0xbbbbbbbb);
xmlGenericErrorFunc testGenericErrorFunc = reinterpret_cast<xmlGenericErrorFunc>(0xcccccccc);
xmlStructuredErrorFunc testStructuredErrorFunc = reinterpret_cast<xmlStructuredErrorFunc>(0xdddddddd);

// Set test values.
xmlSetGenericErrorFunc(testGenericErrorContext, testGenericErrorFunc);
xmlSetStructuredErrorFunc(testStructuredErrorContext, testStructuredErrorFunc);

// Parse XHTML in WebKit.
String chunk = "<x/>"_s;
auto result = WebCoreTestSupport::testDocumentFragmentParseXML(chunk, WebCore::DefaultParserContentPolicy);
EXPECT_TRUE(result);

// Verify test values are restored.
EXPECT_EQ(testGenericErrorContext, xmlGenericErrorContext);
EXPECT_EQ(testStructuredErrorContext, xmlStructuredErrorContext);
EXPECT_EQ(testGenericErrorFunc, xmlGenericError);
EXPECT_EQ(testStructuredErrorFunc, xmlStructuredError);

// Restore original values.
xmlSetGenericErrorFunc(origGenericErrorContext, origGenericErrorFunc);
xmlSetStructuredErrorFunc(origStructuredErrorContext, origStructuredErrorFunc);
}

TEST_F(XMLParsing, WebCoreDoesNotBreakLibxml2)
{
xmlExternalEntityLoader origEntityLoader = xmlGetExternalEntityLoader();
Expand Down

0 comments on commit c864765

Please sign in to comment.