Skip to content

Update XML reader for quick-xml 0.41 and clean code warnings#894

Open
Sachin-Bhat wants to merge 12 commits into
bokuweb:mainfrom
Sachin-Bhat:main
Open

Update XML reader for quick-xml 0.41 and clean code warnings#894
Sachin-Bhat wants to merge 12 commits into
bokuweb:mainfrom
Sachin-Bhat:main

Conversation

@Sachin-Bhat

@Sachin-Bhat Sachin-Bhat commented Jun 23, 2026

Copy link
Copy Markdown

What does this change?

Updates the workspace for newer dependency and clippy compatibility.

Primary motivation: quick-xml versions before 0.41 have known security advisories affecting downstream consumers, so this PR moves the workspace off the vulnerable range. As a side effect it picks up the API changes between the old version and 0.41.

  • Security / dependency: upgrades XML parsing to quick-xml 0.41 (from the previously vulnerable 0.40 and earlier) in docx-core, and regenerates Cargo.lock. This requires the 0.40 migration below as a stepping stone.
    • Handles GeneralRef events, normalized attribute decoding, and broader entity-decoding coverage.
    • Renames EventReader::next() to EventReader::next_event() to match the new API.
  • API: boxes large enum variants in CommentChild, DeleteChild, and TableCellContent to satisfy clippy::large_enum_variant, with matching docx-wasm updates. This is a breaking change for any consumer constructing these enums directly (use the builder APIs instead).
  • Hygiene: cleans up clippy warnings across document, XML builder, XML JSON, type, benchmark, and manifest code.
  • Bindings: refreshes generated TypeScript bindings for the updated ts-rs output format (nullable T | null fields, JSDoc on FrameProperty / Pic / TablePositionProperty, TablePositionProperty schema link).
  • CI: bumps actions/checkout to v7, actions/setup-node to v6, actions/cache to v6; switches the Node version to node; replaces manual wasm-pack / target installs with moonrepo/setup-rust targets/bins inputs plus corepack for pnpm.

References

  • quick-xml security advisories for < 0.41 (RUSTSEC) — see the quick-xml changelog / advisory page for affected CVE IDs and fixed-in version.

Screenshots

Not applicable. This is a Rust library/workspace change.

What can I check for bug fixes?

Run:

cargo +nightly clippy --workspace --all-targets --all-features --locked -- -D warnings -W clippy::perf
cargo +nightly test --workspace --all-features --locked --lib

Expected result: clippy completes with no warnings, and the workspace lib tests pass. The XML entity regression is covered by reader::xml_parser::tests::decodes_xml_entities_in_text_and_attributes.

Also verify the quick-xml version in docx-core/Cargo.toml resolves to 0.41.x via cargo tree -p docx-core | grep quick-xml.

Update the workspace dependency set and adapt the XML reader for the quick-xml 0.40 API. Handle GeneralRef events, use normalized attribute decoding, rename EventReader::next to next_event, refresh generated TS bindings for ts-rs 12, and update benches for criterion 0.8.

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
Box the large Paragraph, Table, and Run enum variants to satisfy clippy::large_enum_variant. Preserve builder-style APIs by boxing internally, and update the wasm table-cell wrapper for the new variant payloads.

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
Apply mechanical clippy fixes across document elements and document metadata code. This removes needless borrows, redundant formatting, needless lifetimes, manual ok conversions, single-character push_str calls, and collapsible matches.

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
Derive default enum implementations where possible, simplify XML builder and writer formatting, remove needless question-mark/update patterns, and replace checked unwraps in XML JSON name formatting with explicit Option matching.

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
- Bump actions/checkout v4 -> v7, actions/setup-node v4 -> v6,
  actions/cache v4 -> v6 across all jobs
- Switch node-version from pinned 20 to 'node' (latest)
- Replace manual wasm-pack/target add installs with moonrepo
  targets/bins inputs and GITHUB_TOKEN env, and use corepack
  to manage pnpm
- Split wasm build into discrete steps for clearer CI logs
- Strip trailing whitespace and normalize working-directory indent

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
Updates quick-xml in docx-core and regenerates the lockfile, pulling
in transitive upgrades (autocfg, bytemuck, cc, clap, etc.) and
removing the now-unused bitflags 1.x entry.

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
Tidy {:?} -> {res:?} at docx-wasm/src/doc.rs:216.

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
Refreshes docx-wasm/js/json/bindings/* from the latest ts-bindgen
output to match the updated Rust schemas: nullable fields are now
typed as 'T | null' (previously 'T?'), optional fields on the
generated interfaces are now explicitly nullable, and a few type
aliases (e.g. AlignmentType) were reformatted. No runtime change.

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
Expands the regenerated FrameProperty, Pic, and TablePositionProperty
TypeScript bindings with JSDoc copied from the OOXML/ECMA-376 schema
descriptions for each property, and converts the optional interfaces
to nullable 'T | null' types to align with the rest of the bindings.
Includes a reference URL on TablePositionProperty for the underlying
schema.

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
The generated _pageTypeNum payload now explicitly sets start and
chapStyle to null when callers pass undefined, matching the nullable
shape produced by the JSON bindings and avoiding 'undefined' entries
in the serialized object.

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
@Sachin-Bhat Sachin-Bhat changed the title Update XML reader for quick-xml 0.40 and clean code warnings Update XML reader for quick-xml 0.41 and clean code warnings Jul 6, 2026
@decebal

decebal commented Jul 14, 2026

Copy link
Copy Markdown

Thanks for tackling this @Sachin-Bhat — the quick-xml 0.41 migration itself looks solid: the test, build, and Clippy jobs are all green, so the reader/entity changes compile and pass.

The only red check is build-wasm, failing for a reason unrelated to quick-xml. The cargo update in this PR also bumped wasm-bindgen to 0.2.126, whose generated js/pkg/docx_wasm.d.ts now emits explicit-resource-management members (readonly [Symbol.dispose](): void;). But docx-wasm still pins typescript@4.9.3 with "lib": ["ES2017", "DOM"], and Symbol.dispose typing (ESNext.Disposable, where Symbol.dispose is a unique symbol) only exists in TypeScript ≥ 5.2. So tsc errors out with 38× TS1165 + 38× TS2339 on that generated file:

js/pkg/docx_wasm.d.ts: error TS2339: Property 'dispose' does not exist on type 'SymbolConstructor'.
js/pkg/docx_wasm.d.ts: error TS1165: A computed property name in an ambient context must refer to an expression whose type is a literal type or a 'unique symbol' type.

The fix is small — bump TypeScript and add the disposable lib:

  1. docx-wasm/package.json: "typescript": "4.9.3""typescript": "^5.5.0" (any 5.2+ works)
  2. In docx-wasm/tsconfig.json, tsconfig.node.json, and tsconfig.web.json: "lib": ["ES2017", "DOM"]"lib": ["ES2017", "ESNext.Disposable", "DOM"]
  3. Regenerate the lockfile so --frozen-lockfile stays happy: cd docx-wasm && pnpm install

I reproduced the failure on this branch and confirmed the change clears it: tsc -p tsconfig.node.json goes from 76 errors to 0 (tested with TS 5.9.3). ts-loader@9.5.7 and jest 28 both accept TS 5.x, so no peer conflicts. (Separately, pnpm test showed 2 image-snapshot byte-diffs that look arch-specific and pre-existing — unrelated to this TS change.) Happy to open a follow-up PR if that's easier. This one would be great to get in — quick-xml < 0.41 is flagged by RUSTSEC, so it clears the advisory for downstream consumers.

@Sachin-Bhat

Copy link
Copy Markdown
Author

Hey @decebal thanks for your feedback! Will make the relevant changes.

Bump TypeScript to ^5.5.0 and add ESNext.Disposable to the shared, Node, and web compiler configurations. This allows declarations generated by wasm-bindgen 0.2.126 to type-check when they expose Symbol.dispose.

Regenerate the pnpm lockfile with TypeScript 5.9.3. The frozen install, Node and web type checks, all 382 Rust tests, and the WASM builds pass. Jest still reports the two existing architecture-specific image snapshot byte differences.

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
@Sachin-Bhat

Copy link
Copy Markdown
Author

@decebal @bokuweb made the relevant changes! TS bumped to ^5.5.0 and resolves to 5.9.3 in package.json. Added ESNext.Disposable to tsconfig.*.

Verification:

  • Reproduced all 76 errors with TypeScript 4.9.3.
  • Node and web type checks now pass against 38 [Symbol.dispose] declarations.
  • pnpm install --frozen-lockfile passes.
  • All 382 Rust tests pass.
  • Full WASM build completes.

Caveat: Jest retains the two reported image snapshot byte differences: 89/91 tests and 185/187 snapshots pass.

Please review/merge when you find some time.

Thanks
Sachin

…en normalized before snapshotting

Signed-off-by: Sachin Bhat <sachubhat17@gmail.com>
@Sachin-Bhat

Copy link
Copy Markdown
Author

@bokuweb updated the test to stop snapshotting exact compressed PNG bytes. In the two failing tests earlier, assert that the decoded preview is a valid PNG, then replace image payloads with placeholders before snapshotting. This remains stable across zlib-rs versions. The earlier test would break everytime zlib-rs would have updated unless the test snapshots were updated running pnpm test -- -u. Please rerun the build once you get some time.

Thanks!

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.

2 participants