Repository navigation
Compare part names independent of percent-encoding - #97
Merged
aaronzi merged 2 commits intoOct 13, 2026
Merged
Conversation
The parts map was keyed by the raw ZIP entry name while lookups used the percent-encoded URL pathname. A part whose entry name contains a literal space (or another character URL encodes) could therefore not be found via FindPart, not even with its own Part.URI, and DeletePart silently did nothing. Flush also renamed such entries to their encoded form while their relationship targets kept the literal name, which broke the package on a plain open-and-flush round trip. ISO/IEC 29500-2 part names are URIs compared case-insensitively, so map keys are now built from the decoded, lowercased and re-encoded path segments. Parts read from an archive remember their entry name, which is reused on Flush, and relationship parts are named after their source part instead of the lowercased map key. Fixes eclipse-aascw#83
aaronzi
force-pushed
the
fix/part-name-percent-encoding
branch
from
October 13, 2026 06:44
a6c7731 to
81e8ccd
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Encoding-equivalent ZIP entries can collide and silently lose package content.
1 open finding
What changed in this PR
Canonically compares percent-encoded part names while preserving original ZIP entry names.
Changes:
- Normalizes part, relationship, and content-type lookup keys.
- Preserves archive names during round trips.
- Adds coverage for encoded names and relationship casing.
| File | Description |
|---|---|
src/package.ts |
Implements canonical lookup and entry-name preservation. |
test/package.test.ts |
Tests lookup, deletion, round trips, and naming. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Two ZIP entries whose names differ only in percent-encoding or ASCII case, e.g. "a b.pdf" and "a%20b.pdf", now map to the same part. Opening such an archive used to keep only one of them and lose the other on Flush, so it is now rejected as an invalid format, as the Go library already does. ISO/IEC 29500-2 compares part names as case-insensitive ASCII strings, so non-ASCII characters are no longer case-folded: "%C3%84" (Ä) and "%C3%A4" (ä) are distinct part names.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #83
Problem
The parts map was keyed by the raw ZIP entry name, but
FindPart/MustPart/DeletePart/IsSpec/… key on the percent-encodedURL.pathname. For an entry stored with a literal space (aasx/suppl/a b.pdf):FindPartreturnednull, even for the part's ownPart.URI.DeletePartdid nothing and raised no error.OpenReadWrite…→Flushrenamed the entry toa%20b.pdf, while the.relstarget kept the literal space. Reading the result back threwsupplementary part … not found.On the issue's framing: in ISO/IEC 29500-2 (which IDTA-01005 makes normative for AASX), part names are URIs, and the ZIP entry name is the part name without its leading
/. Conforming producers therefore writea%20b.pdf, and this library already handled that case correctly. Literal-space entries come from non-conforming producers. They're still worth reading robustly, especially because the old behavior could corrupt such a package on round trip.Change
/a b.pdf,/a%20b.pdfand/A%20B.pdfnow map to the same key, in line with the OPC rules for comparing part names. This applies equally to parts, relationship sources and targets, and content-type overrides.Part.URIfor entries with stray%,?or#escapes those characters, so they aren't treated as URL syntax.Flushwrites them back under it, so a round trip no longer renames entries.aasx/Data.xmlnow getsaasx/_rels/Data.xml.relsinstead ofaasx/_rels/data.xml.rels.Tests
New tests cover:
DeletePart%and#Three of these fail on
main. The other two guard behavior that already worked.I also compared
mainagainst this branch on 66 real-world AASX files (vendor packages, IDTA templates, the IDTA-01005 example). Specs, supplementaries and lookups are identical on all of them. The only difference: 3 fewer packages get entries renamed on an open-and-flush round trip.The Go library has the same defect, mirrored (it fails on the conforming
%20form). A matching PR is opened there.