fix(html): enable huge_tree on HTMLParser so deeply nested HTML partitions#4340
fix(html): enable huge_tree on HTMLParser so deeply nested HTML partitions#4340CrepuscularIRIS wants to merge 2 commits into
Conversation
There is a safety reason why this should not be the default. I think the default should remain |
|
Thanks @cragwolfe — fully agree on the safety concern. I've pushed fe6216d which:
Both tests pass locally; full |
…tions Fixes Unstructured-IO#4289 `partition_html` returned an empty element list for HTML documents whose DOM depth exceeded lxml's default depth limit (~256) because the module-level `etree.HTMLParser` used the default `huge_tree=False`, which silently drops subtrees past the limit. Enabling `huge_tree=True` on the shared parser makes deep documents round-trip correctly. A regression test builds a 260-level-deep `<div>` chain wrapping a `<p>`; the test fails without the fix (0 elements) and passes with it (1 element). Signed-off-by: CrepuscularIRIS <serenitygp@qq.com>
Per @cragwolfe review: huge_tree=True disables libxml2's safety guards against malicious inputs (https://lxml.de/FAQ.html), so it must remain opt-in. Default stays huge_tree=False; set UNSTRUCTURED_HTML_HUGE_TREE to 1/true/yes to enable for trusted inputs. - Add test confirming default behavior drops nodes silently (matches prior behavior — no regression for existing users). - Test for the opt-in path patches the parser since it's built at module import time. - Updated changelog to document the env var and the security tradeoff. Signed-off-by: CrepuscularIRIS <serenitygp@qq.com>
fe6216d to
a851154
Compare
|
Rebased onto main to clear the merge conflict — head is now |
|
code change looks good, just the CHANGELOG.md got botched with deletions against main. also need to bump version. thanks! e.g.: CrepuscularIRIS/unstructured@fix/html-huge-tree...Unstructured-IO:unstructured:crag/pr-4340-do-not-merge |
|
Will fix the CHANGELOG merge artifact and bump the version, then push shortly. |

Summary
Fixes #4289.
partition_htmlsilently returned zero elements for HTML documents whose DOM depth exceeded lxml's defaulthuge_tree=Falsedepth limit (~256 levels). The module-leveletree.HTMLParserinunstructured/partition/html/parser.pywas constructed withouthuge_tree, soetree.fromstringdropped the subtree beyond the limit and the downstream flow saw an empty document.This PR flips the kwarg to
huge_tree=Trueon that single shared parser and adds a regression test.Root Cause
html_parser = etree.HTMLParser(remove_comments=True)inherits lxml's defaulthuge_tree=False, which caps tree depth / node text size. Whenetree.fromstring(html_text, html_parser)inpartition.py:258hits that cap, the subtree past the cap is dropped, producing an emptyFlowroot — sopartition_htmlyields[]for large/deeply-nested HTML.Changes
unstructured/partition/html/parser.py: passhuge_tree=Trueto the sharedetree.HTMLParser(1 line).test_unstructured/partition/html/test_partition.py: addtest_partition_html_parses_deeply_nested_html, which builds a 260-level<div>wrapper around a<p>and asserts the inner text is recovered.CHANGELOG.md: new0.22.22section under### Fixes.unstructured/__version__.py: bump0.22.21→0.22.22.Testing
test_unstructured/partition/html/— 312/312 passing.test_unstructured/partition/html/+partition/test_xml.py+partition/test_text.py— 390/390 passing.Notes
huge_tree=TrueonHTMLParserrelaxes lxml's tree-size / depth safeguards. It does not enable entity expansion (that flag isresolve_entities, which remains off by default), so no new XXE surface is introduced.huge_treeopt-in if preferred, but the shared-parser approach matches what the issue and the prior self-closed PR fix: enable huge_tree for HTMLParser to handle large documents #4306 proposed.