Skip to content

fix: prevent partial ItemRef picture substitution - #494

Merged
entelecheia merged 4 commits into
mainfrom
fix/453-itemref-resolution
Nov 10, 2026
Merged

entelecheia merged 4 commits into
mainfrom
fix/453-itemref-resolution

Conversation

@entelecheia

Copy link
Copy Markdown
Member

Closes #453.

An unlisted ItemRef("image1") could select BinData/image10.png through the HWP5 writer's substring fallback and embed the wrong picture. Resolve unlisted ItemRefs using the same case-sensitive exact name, path-suffix and filename-stem rules as Document::bin_stream_index. Preserve authoritative manifest lookup and the native BinRef::Id compatibility branch. A missing match uses the existing content-free BinaryAssetRemoved ledger path.

Pre-change compatibility measurement:

  • Bounded ZIP/XML inspection of 7 available HWPX archives found 2 image references, both manifest-backed, with identical old/model resolution. No substring-fallback use was observed in those archives.
  • Inspection of 3 committed generated fixture definitions (5 references) found 2 old substring-fallback uses: an unlisted filename stem and a duplicate-byte stream fixture with distinct names. Both select the same stream through the model's exact filename-stem rule. The other 3 references use manifests.
  • This is archive-surface/source-definition evidence, not a native-parser or Hangul run. Broader private/diagnostic corpus roots were unavailable. The available image coverage is small, so this is not a broad corpus-compatibility claim. No private document or XML derivative is committed.

Five new regressions failed at their intended assertions on the test-only baseline b5950a5: body/cell/Generic-list partial references lacked the asset-loss event; a later exact stem lost to an earlier partial name; writer/model resolution disagreed. Four existing ItemRef tests passed on that baseline. The tests also check emitted payload bytes or the absence of a synthesized stream.

Focused verification at c6d7f6d: HWP5 library tests 128/128 passed and all-target HWP5 clippy passed with -D warnings, Rust 1.93.0, two build/test workers. Existing assertions and fixture guards remain intact. The separate comment-only commit translates touched-file Korean prose as required by AGENTS.md; quoted sample identifiers/marker text and all non-comment source were preserved.

Full verification at 03b0d6380c8a74e093db5215938bdbc3c7c0a7ae: exact-head remote Rust 1.93.0, CARGO_BUILD_JOBS=2 RUST_TEST_THREADS=2 scripts/check.sh </dev/null, with verifier stdin also closed, exit 0.

== check: OK (fmt/clippy/test/crate-edges/pdf-runner/structured-corpus/claims/doc-surface/release-block/readiness-selfcheck/skip-accounting/public-parity=skipped) skipped-for-missing-fixtures=49 (optional=4) ==

The 49 missing-fixture skips include 4 optional cases; those absent fixture paths remain unverified. Public PDF parity auto-skipped because this environment lacks the pinned Poppler version; hosted CI reports that separate gate.

Copilot AI balanced review requested due to automatic review settings November 10, 2026 13:43
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-10T13:46:39.526815Z 03b0d63 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@entelecheia
entelecheia merged commit 10b2e3e into main Nov 10, 2026
7 checks passed
@entelecheia
entelecheia deleted the fix/453-itemref-resolution branch November 10, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hwp5 writer: ItemRef substring fallback can bind a picture to the wrong BinData stream

2 participants