Fix block reference handling in ByteArrayBackedDataSource

[lurongjiang]

• Modify the ByteArrayBackedDataSource.read method to handle positions beyond the file size.
• Return a zero-padded buffer instead of throwing an exception when the position exceeds the file size.

[lurongjiang]

side effect

[lurongjiang]

Changes Made

1. Removed zero-padding logic

   • During standard .doc file parsing, zero-padding introduced unexpected exceptions due to invalid buffer content.
   • Added test cases for both standard (Microsoft Office 97-2003 .doc) and non-standard (WPS-compatible) formats to ensure robustness.

2. Enhanced out-of-bounds handling in FileBackedDataSource

   • Discovered that file-based operations rely on FileBackedDataSource, which previously threw exceptions when position >= size().
   • Now returns an empty ByteBuffer.allocate(length) instead of throwing an exception, ensuring consistent behavior with ByteArrayBackedDataSource.
   • Added corresponding test cases to validate boundary conditions for file-backed reads.

Impact Assessment

• Since the original implementation threw exceptions on out-of-bounds access, the new behavior (returning an empty buffer) maintains backward compatibility for valid cases.
• For invalid cases (e.g., corrupted files or incomplete reads), the change prevents abrupt exceptions, allowing callers to handle incomplete data gracefully (e.g., failing silently or logging errors).
• No functional regression expected, as the new logic aligns with the previous exception-throwing behavior in terms of outcome (failure to parse invalid data).

Testing Validation

• Verified that:
  • Standard .doc files parse correctly (no zero-padding artifacts).
  • Non-standard WPS files remain supported.
  • Out-of-bounds reads (both byte-array and file-backed) now return empty buffers instead of exceptions.

[pjfanning]

target main branch - not 5.5.x

[pjfanning]

add a comment about why this is allowed - like in the other class above

[pjfanning]

must test the actual text - it being not null is not enough

[pjfanning]

I must admit that I'm basically -1 on this.

I don't see why we should be returning empty ByteBuffers.

I think we should fail when we read corrupt files.

Feel free to fork POI and hack it how you please but I can't in good conscience agree to a change like this.

If it was an option where this behaviour only kicks in if you set a flag on the extractor to say ignore corrupt blocks - I would possibly agree.

[pjfanning]

relates to #1041

[lurongjiang]

Key Changes:

1. Added Configurable Tolerance Mode (org.apache.poi.poifs.allowCorruptBlocks)

   • Modified ByteArrayBackedDataSource.java and FileBackedDataSource.java
   • Default behavior: Strict mode - throws IndexOutOfBoundsException when encountering blocks beyond EOF
   • Optional tolerant mode: Set system property to true to allow reading corrupt files with missing blocks
   • System property is checked dynamically at runtime, allowing flexible configuration

2. Improved Test Coverage

   • Updated TestHWPFParser.java with comprehensive tests:
     • testDocRead(): Verifies file can be read in tolerant mode with actual text content validation
     • testDocReadStrictMode(): Verifies strict mode properly rejects corrupt files
     • testWpsDocByFs(): Tests file system-based reading with content validation
     • testOffice97_2003DocRead(): Validates normal document reading
   • All tests now verify actual text content (not just non-null), checking:
     • Text is not null
     • Text is not empty
     • Text is not blank (after trimming)

3. Clear Error Messages

   • Exception messages guide users on how to enable tolerant mode if needed
   • Example: "Position X is beyond EOF (Y). Set system property 'org.apache.poi.poifs.allowCorruptBlocks' to true to allow reading corrupt files with missing blocks."

Design Rationale:

• Fail-fast by default: Aligns with Apache POI's principle of strict validation
• Opt-in tolerance: Users who need to handle damaged documents can enable it via system property
• No API changes: Backward compatible, uses standard Java system properties
• Follows existing patterns: Similar to other POI configuration options like org.apache.poi.ss.ignoreMissingFontSystem

Testing:

All tests pass successfully, verifying both strict and tolerant modes work as expected.

@pjfanning

[pjfanning]

This is a WPS bug so I don't think we should make hacks in fundamental parts of POI code:
#1041 (comment)

Report the issue to WPS

[lurongjiang]

Thank you for the review and feedback. I understand your concerns.

My original intention was that since POI aims to support various Office document formats, I thought it should be able to handle WPS-generated files as well. When I encountered parsing failures with certain WPS documents, I attempted to add a tolerance mechanism as a workaround.

However, I now understand your point. This is not really a WPS bug, but rather a limitation of POI's current implementation - it cannot handle certain non-standard file formats generated by WPS. Adding workarounds in POI's core components (like ByteArrayBackedDataSource and FileBackedDataSource) would compromise the integrity of the codebase.

I'll close this PR as suggested. Users who need to handle such files can implement workarounds at the application level rather than modifying the library itself.

[pjfanning]

Thank you for the review and feedback. I understand your concerns.

My original intention was that since POI aims to support various Office document formats, I thought it should be able to handle WPS-generated files as well. When I encountered parsing failures with certain WPS documents, I attempted to add a tolerance mechanism as a workaround.

However, I now understand your point. This is not really a WPS bug, but rather a limitation of POI's current implementation - it cannot handle certain non-standard file formats generated by WPS. Adding workarounds in POI's core components (like ByteArrayBackedDataSource and FileBackedDataSource) would compromise the integrity of the codebase.

I'll close this PR as suggested. Users who need to handle such files can implement workarounds at the application level rather than modifying the library itself.

I can't open the file using Microsoft Word. So it is a WPS bug.