feat: add HAR backend and Office macro detection#3707
Conversation
|
✅ DCO Check Passed Thanks @AyaanB-DotCom, all your commits are properly signed off. 🎉 |
Merge Protections🔴 1 of 2 protections blocking · waiting on 👀 reviews
🔴 Require two reviewer for test updatesWaiting for
This rule is failing.When test data is updated, we require two reviewers
Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Thanks @AyaanB-DotCom for this PR and your first contribution! I guess you have edited some files on a Windows system that changed all the line endings. As a result, your PR shows a massive diff in some files (e.g., 2796 additions and 2794 deletions in For instance, on Windows: or on MacOS: Btw, I have just opened another PR #3709 to prevent this issue. Another recommendation for your next contribution: please open separate PRs if you are addressing two different and disconnected topics. |
dolfim-ibm
left a comment
There was a problem hiding this comment.
Security Review
Scope: the Office backend diffs (
msword_backend.py,msexcel_backend.py,mspowerpoint_backend.py) are pure line-ending (CRLF→LF) churn with no logic changes and are excluded from this review. The analysis covers the two genuinely new files:docling/backend/har_backend.pyanddocling/utils/office_utils.py.
Overall verdict: needs changes before merge — three medium-severity gaps.
The contributor shows clear security awareness (per-body cap, binary MIME exclusion, stdlib JSON, no URL fetching, BytesIO seek-back), but the following gaps should be addressed:
🟠 MEDIUM — F-1: No file-size guard before reading HAR into memory (har_backend.py · __init__)
The constructor reads the entire document into a Python string with no size check:
raw = Path(self.path_or_stream).read_text("utf-8") # path branch
raw = self.path_or_stream.getvalue().decode("utf-8") # stream branchA crafted (or simply large real-world) HAR can be several GB and will load entirely into RAM. Suggested fix:
_MAX_HAR_BYTES = 100 * 1024 * 1024 # 100 MB
# path branch
path = Path(self.path_or_stream)
if path.stat().st_size > _MAX_HAR_BYTES:
raise DocumentLoadError("HAR file exceeds size limit")
raw = path.read_text("utf-8")
# stream branch
buf = self.path_or_stream.getvalue()
if len(buf) > _MAX_HAR_BYTES:
raise DocumentLoadError("HAR stream exceeds size limit")
raw = buf.decode("utf-8")🟠 MEDIUM — F-2: Unbounded entry iteration (har_backend.py · convert())
_MAX_BODY_CHARS = 2000 correctly caps individual bodies, but the for entry in entries loop has no bound on the number of entries. A crafted HAR with 1 million 1-byte entries still produces 1 million document nodes. Suggested fix:
_MAX_ENTRIES = 10_000
if len(entries) > _MAX_ENTRIES:
_log.warning("HAR contains %d entries; truncating to %d.", len(entries), _MAX_ENTRIES)
entries = entries[:_MAX_ENTRIES]
for entry in entries:
...🟠 MEDIUM — F-3: Zip-slip not checked in warn_if_macros (office_utils.py)
msexcel_backend._parse_threaded_comments already validates ZIP entry names before accessing members:
if any(m.startswith("/") or ".." in m for m in zip_file.namelist()):
_log.warning("Skipping file with unsafe ZIP paths")
return threaded_commentswarn_if_macros performs the same ZIP inspection but skips this check. The inconsistency should be resolved. Also worth narrowing the catch clause — except (zipfile.BadZipFile, Exception) makes the first term redundant and silences MemoryError; prefer except (zipfile.BadZipFile, OSError, ValueError).
with zipfile.ZipFile(path_or_stream) as zf:
names = zf.namelist()
if any(n.startswith("/") or ".." in n for n in names):
_log.warning("Skipping archive with unsafe ZIP entry paths")
return
has_macros = any(_VBA_MARKER in name for name in names)✅ What's done well
_MAX_BODY_CHARS = 2000— per-body cap on both request and response bodies, covered by tests._is_text_mime()— binary/base64 blobs (images, fonts, etc.) are correctly excluded.- No URL fetching —
urlandmethodfields are written as plain text; SSRF not possible. json.loads()— standard-library parser, no eval,JSONDecodeErrorcorrectly re-raised asDocumentLoadError.- BytesIO seek-back —
finallyblock inwarn_if_macrosrestores stream position; covered bytest_bytesio_rewound_after_check.
I, AyaanB-DotCom <a.bhatti3005@gmail.com>, hereby add my Signed-off-by to this commit: b65f18f I, AyaanB-DotCom <a.bhatti3005@gmail.com>, hereby add my Signed-off-by to this commit: 2fa042a I, AyaanB-DotCom <a.bhatti3005@gmail.com>, hereby add my Signed-off-by to this commit: 52a0034 I, Ayaan B <a.bhatti3005@gmail.com>, hereby add my Signed-off-by to this commit: 2a40838 Signed-off-by: AyaanB-DotCom <a.bhatti3005@gmail.com>
99f543b to
616e16d
Compare
I, AyaanB-DotCom <a.bhatti3005@gmail.com>, hereby add my Signed-off-by to this commit: b65f18f I, AyaanB-DotCom <a.bhatti3005@gmail.com>, hereby add my Signed-off-by to this commit: 2fa042a I, AyaanB-DotCom <a.bhatti3005@gmail.com>, hereby add my Signed-off-by to this commit: 52a0034 I, Ayaan B <a.bhatti3005@gmail.com>, hereby add my Signed-off-by to this commit: 2a40838 I, AyaanB-DotCom <a.bhatti3005@gmail.com>, hereby add my Signed-off-by to this commit: 616e16d Signed-off-by: AyaanB-DotCom <a.bhatti3005@gmail.com>
|
@AyaanB-DotCom for moving on the review of this PR, can you please update the branch with the latest changes from |
Add support for converting HAR (HTTP Archive) files to
DoclingDocument, andwarn when Office files (docx/xlsx/pptx) contain VBA macros before conversion.
Changes
docling/backend/har_backend.py: newHarDocumentBackend— converts HARlog.entriesinto headings, status text, and code blocks; caps inline bodypreviews at 2000 chars; skips binary MIME types
docling/utils/office_utils.py:warn_if_macros()utility — inspects OOXMLZIPs for
vbaProject.bin, logs a warning, never blocks conversiondocling/datamodel/base_models.py: addedInputFormat.HARwith extensionsand MIME type
docling/datamodel/document.py: added HAR entry to_mime_from_extensiondocling/document_converter.py: addedHarFormatOptiondocling/backend/msword_backend.py,msexcel_backend.py,mspowerpoint_backend.py: callwarn_if_macros()before loadingtests/test_backend_har.py: 13 tests covering format registration, path/streamloading, content output, binary exclusion, converter integration
tests/test_office_macros.py: 7 tests covering warning, silence on clean files,BytesIO rewind behaviour
Checklist: