Bug 104680 – [libxml2] String parser contexts are not using necessary options

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: XML

[Zan Dobersek]

Compared to fragment parser contexts, they're not given any options, causing specific tests to fail.

[Zan Dobersek]

Created attachment 178820 [details]
Provisional patch

[Zan Dobersek]

Created attachment 180880 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 180880 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=180880&action=review

> Source/WebCore/ChangeLog:13
> +        forces the parsed entities to be loaded. This is done only if Libxml2 version used is at

This is a bit misleading - the check is based on version of headers, and is not a runtime one.

That's probably OK, but does not match ChangeLog.

> Source/WebCore/ChangeLog:14
> +        least 2.9.0, otherwise the previous option setting behavior is retained.

What is the reason to not do this unconditionally?

[Zan Dobersek]

Comment on attachment 180880 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=180880&action=review

>> Source/WebCore/ChangeLog:14
>> +        least 2.9.0, otherwise the previous option setting behavior is retained.
> 
> What is the reason to not do this unconditionally?

The intention here is to preserve current behavior when using pre-2.9.0 libxml2 version. For instance, I don't know what version of libxml2 the Mac port uses and I'd hate to possibly break the behavior.

Though, I can tell that in the current code `parser->replaceEntities = true` is also set when xmlCtxtUseOptions is called with the XML_PARSE_NOENT option so the check could perhaps just be removed and xmlCtxtUseOptions call would do that work instead.

[Alexey Proskuryakov]

Has <http://trac.webkit.org/changeset/148144> taken care of this? That patch is somewhat different, so I'm not quite sure.

[Alexey Proskuryakov]

Comment on attachment 180880 [details]
Patch

r- since this patch doesn't apply cleanly any more. Please post an updated version if XML_PARSE_DTDVALID is needed with libxml2 2.9.0. If XML_PARSE_NODICT is also needed, it's probably better to add it in a separate patch with a regression test.

[Zan Dobersek]

(In reply to comment #5)
> Has <http://trac.webkit.org/changeset/148144> taken care of this? That patch is somewhat different, so I'm not quite sure.

Possibly. Don't have any definitive data, but the http/tests/security/xss-DENIED-xml-external-entity.xhtml layout test is passing at the moment and has been passing for some time now.

That failure was the reason behind this bug report and patch, so I think this is safe to mark as a duplicate.

*** This bug has been marked as a duplicate of bug 114377 ***