Skip to content

Handle missing numbering level values safely, NPE in ListLevel.java - #689

Open
zyplayer wants to merge 1 commit into
plutext:VERSION_17_0_3from
zyplayer:listlevel_npe
Open

Handle missing numbering level values safely, NPE in ListLevel.java#689
zyplayer wants to merge 1 commit into
plutext:VERSION_17_0_3from
zyplayer:listlevel_npe

Conversation

@zyplayer

@zyplayer zyplayer commented Aug 7, 2026

Copy link
Copy Markdown

Normalize missing abstract numbering level IDs without overwriting existing levels.

Centralize paragraph numbering level resolution and default omitted or malformed values to level 0 across HTML and FO conversion paths.

Add regression tests for direct construction, malformed paragraph levels, and linked numbering styles.

What does this PR do?

#593

This PR improves the handling of missing or malformed numbering level values and prevents NullPointerExceptions during HTML and FO conversion.

It:

  • Normalizes missing w:lvl/@w:ilvl values when abstract numbering definitions are loaded.
  • Assigns the first available level ID without overwriting existing levels, including levels populated through linked numbering styles.
  • Centralizes paragraph numbering level resolution in Emulator.
  • Defaults omitted or malformed w:numPr/w:ilvl values to level 0 when the level is consumed.
  • Applies the same null-safe behavior across HTML, XSLT, visitor-based, and FO conversion paths.
  • Preserves nullable explicit-level handling where style resolution may still be required.
  • Adds regression tests covering:
    • Direct ListLevel construction with a missing level ID.
    • Missing paragraph numbering levels.
    • Numbering levels with a missing val.
    • Allocation when level 0 is already occupied.
    • Linked numbering styles without overwriting existing levels.

Checklist

  • Each commit carries a Signed-off-by: line added by me (DCO — see
    CONTRIBUTING.md), certifying I have the right to submit this under the
    Apache License v2.
  • Any AI assistance is disclosed via a commit trailer
    (Assisted-by: / Generated-by: / Co-Authored-By:), and I have
    personally reviewed, understood, and can explain all of the code.
  • Tests pass locally (mvn test -pl docx4j-core-tests -am), and I added
    or extended a test where practical.
  • CHANGELOG.md is updated if the change is user-visible.

Normalize missing abstract numbering level IDs without overwriting existing levels.

Centralize paragraph numbering level resolution and default omitted or malformed values to level 0 across HTML and FO conversion paths.

Add regression tests for direct construction, malformed paragraph levels, and linked numbering styles.
@plutext

plutext commented Aug 18, 2026

Copy link
Copy Markdown
Owner

One change I'd like before merging: Please don't mutate the JAXB objects.

Both ListLevel and AbstractListNumberingDefinition call setIlvl(...) on the caller's Lvl. That makes a read path write to the document model: converting to HTML or PDF would silently add w:ilvl attributes the user never authored, and if they save the package afterwards those end up in their docx.

It isn't needed, either. ListLevel:143 is the only place that requires Lvl.getIlvl() to be non-null — MainDocumentPart:340 only logs it, and ListNumberingDefinition:216 is LvlOverride.getIlvl(), which is already guarded. So deriving the id locally is enough:

if (levelNode.getIlvl() == null) {
log.warn("Missing @w:ilvl on w:lvl; treating as 0");
this.id = "0";
} else {
this.id = levelNode.getIlvl().toString();
}

and similarly in readListLevelsFromAbsNode, where you can pass the level number you worked out to readLevel/ListLevel rather than stamping it onto the Lvl. The tests would need the corresponding change, since at the moment they assert the mutation.

A minor thing: Level assignment for a malformed abstractNum. Lowest-unused is arbitrary: for [lvl-without-ilvl, lvl0] it makes the first element level 1. Positional index (level n is the nth w:lvl) seems more natural, and I'd expect it to match what Word does with such a file?

@plutext

plutext commented Aug 18, 2026

Copy link
Copy Markdown
Owner

@dcoapp recheck

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