docs(bf-2hjaa): document assertion location for truncated-flate error validation
This commit is contained in:
jedarden
parent
76c9ca6a4d
commit
1726ee3b74
1 changed files with 106 additions and 93 deletions
|
|
@ -2,121 +2,134 @@
|
|||
|
||||
## Summary
|
||||
|
||||
This analysis identifies where in the stream decoder fixture test the error assertion should be added to validate that `STREAM_DECODE_ERROR` diagnostics are emitted for the `flate_truncated` fixture.
|
||||
This analysis identifies where in the truncated-flate.pdf test the error assertion should be added to validate that `STREAM_DECODE_ERROR` diagnostics are emitted when processing a PDF with a truncated flate stream.
|
||||
|
||||
## Problem Statement
|
||||
## Context
|
||||
|
||||
The `flate_truncated.meta` file specifies that decoding this fixture should:
|
||||
1. Return partial bytes (what was decoded before the truncation point)
|
||||
2. Emit a `STREAM_DECODE_ERROR` diagnostic
|
||||
This is about a PDF extraction test for the fixture at `/home/coding/pdftract/tests/fixtures/malformed/truncated-flate.pdf`, NOT the stream decoder unit tests. The test should:
|
||||
1. Extract the PDF using `pdftract_core::extract::extract_pdf()`
|
||||
2. Assert extraction succeeds (non-fatal error recovery)
|
||||
3. Assert `STREAM_DECODE_ERROR` appears in the errors/diagnostics array
|
||||
|
||||
However, the current test scaffold does not validate diagnostics at all.
|
||||
## Extraction Result Structure
|
||||
|
||||
## Current State
|
||||
|
||||
### 1. Fixture Configuration
|
||||
Location: `crates/pdftract-core/tests/stream_decoder_fixtures.rs:60-64`
|
||||
### From `/home/coding/pdftract/crates/pdftract-core/src/extract.rs`:
|
||||
|
||||
```rust
|
||||
FixtureInfo {
|
||||
name: "flate_truncated",
|
||||
filter: FixtureFilter::Single("FlateDecode", None),
|
||||
expected_diags: vec![], // <-- Should be vec![DiagCode::StreamDecodeError]
|
||||
bomb_limit: None,
|
||||
},
|
||||
```
|
||||
pub struct ExtractionResult {
|
||||
pub fingerprint: String,
|
||||
pub pages: Vec<PageResult>,
|
||||
pub metadata: ExtractionMetadata,
|
||||
// ... other fields
|
||||
}
|
||||
|
||||
### 2. Decode Function Signature Issue
|
||||
Location: `crates/pdftract-core/src/parser/stream.rs:89-95`
|
||||
|
||||
The `StreamDecoder::decode()` trait:
|
||||
```rust
|
||||
fn decode(
|
||||
&self,
|
||||
input: &[u8],
|
||||
params: Option<&PdfObject>,
|
||||
doc_counter: &mut u64,
|
||||
max_bytes: u64,
|
||||
) -> Result<Vec<u8>, FilterError>;
|
||||
```
|
||||
|
||||
**Problem:** The trait returns only `Result<Vec<u8>, FilterError>` - no diagnostics!
|
||||
- When `UnexpectedEof` occurs, it returns `Ok(partial_bytes)` but cannot emit diagnostics
|
||||
- See line 542-544 in `FlateDecoder::decode_impl()`: truncated streams just return partial bytes with no diagnostic
|
||||
|
||||
### 3. Test Loop Missing Diagnostic Validation
|
||||
Location: `crates/pdftract-core/tests/stream_decoder_fixtures.rs:254-360`
|
||||
|
||||
The test loop:
|
||||
1. ✓ Reads fixture and expected files
|
||||
2. ✓ Decodes the fixture
|
||||
3. ✓ Compares output bytes
|
||||
4. ✗ **Does NOT validate diagnostics**
|
||||
|
||||
## Where the Assertion Should Be Added
|
||||
|
||||
**Location:** In `test_all_stream_decoder_fixtures()`, after the byte comparison check (after line 326)
|
||||
|
||||
**What to validate:**
|
||||
- If `fixture.expected_diags` is non-empty, verify that matching diagnostics were emitted
|
||||
- For `flate_truncated`: assert that exactly one `STREAM_DECODE_ERROR` diagnostic was emitted
|
||||
|
||||
## How to Fix
|
||||
|
||||
### Option 1: Use higher-level `decode_stream()` function
|
||||
The `decode_stream()` function returns `DecodeResult` which includes diagnostics:
|
||||
```rust
|
||||
pub struct DecodeResult {
|
||||
pub bytes: Vec<u8>,
|
||||
pub diagnostics: Vec<Diagnostic>, // <-- We need this!
|
||||
pub meta: StreamMeta,
|
||||
pub struct ExtractionMetadata {
|
||||
// ... other fields
|
||||
/// Diagnostics emitted during extraction (coverage warnings, etc.)
|
||||
#[serde(skip_serializing_if = "Vec::is_empty")]
|
||||
pub diagnostics: Vec<String>,
|
||||
// ... other fields
|
||||
}
|
||||
```
|
||||
|
||||
### Option 2: Add diagnostic support to `StreamDecoder::decode()` trait
|
||||
Modify the trait to return diagnostics alongside the decoded bytes.
|
||||
**Key insight**: The diagnostics are stored in `extraction_result.metadata.diagnostics` as a `Vec<String>`, not `Vec<Diagnostic>`.
|
||||
|
||||
## Recommended Assertion Implementation
|
||||
### Diagnostic Code Format
|
||||
|
||||
After line 326 (byte comparison), add:
|
||||
From `/home/coding/pdftract/crates/pdftract-core/src/diagnostics.rs`:
|
||||
- **Enum variant**: `DiagCode::StreamDecodeError`
|
||||
- **String representation**: `"STREAM_DECODE_ERROR"` (via `Display` impl)
|
||||
|
||||
## Optimal Assertion Location
|
||||
|
||||
The assertion should be added in the test function `test_truncated_flate_recovery()` at the following location:
|
||||
|
||||
**After**: Extraction succeeds and we have the `ExtractionResult`
|
||||
**Before**: Any other assertions that might depend on the result
|
||||
|
||||
### Complete Test Pattern
|
||||
|
||||
Based on the pattern from `TH-04-js-presence.rs` lines 31-94:
|
||||
|
||||
```rust
|
||||
// Validate expected diagnostics
|
||||
if !fixture.expected_diags.is_empty() {
|
||||
// Check if each expected diagnostic code is present in the actual diagnostics
|
||||
for expected_code in &fixture.expected_diags {
|
||||
let found = result.diagnostics.iter().any(|d| d.code() == *expected_code);
|
||||
assert!(
|
||||
found,
|
||||
"{}: expected diagnostic {} not found. Got: {:?}",
|
||||
fixture.name,
|
||||
expected_code,
|
||||
result.diagnostics
|
||||
);
|
||||
#[test]
|
||||
fn test_truncated_flate_recovery() {
|
||||
let fixture = PathBuf::from("tests/fixtures/malformed/truncated-flate.pdf");
|
||||
|
||||
// Skip if fixture doesn't exist
|
||||
if !fixture.exists() {
|
||||
eprintln!("Skipping test: fixture not found at {}", fixture.display());
|
||||
return;
|
||||
}
|
||||
|
||||
// Extract with default options
|
||||
let options = ExtractionOptions::default();
|
||||
let result = extract_pdf(&fixture, &options);
|
||||
|
||||
// ASSERTION LOCATION 1: Assert extraction succeeds (non-fatal recovery)
|
||||
assert!(result.is_ok(), "Extraction should succeed despite truncated stream");
|
||||
|
||||
let extraction_result = result.unwrap();
|
||||
|
||||
// ASSERTION LOCATION 2: Assert STREAM_DECODE_ERROR appears in diagnostics
|
||||
let diagnostics = &extraction_result.metadata.diagnostics;
|
||||
assert!(
|
||||
diagnostics.iter().any(|d| d.contains("STREAM_DECODE_ERROR")),
|
||||
"Expected STREAM_DECODE_ERROR diagnostic for truncated flate stream. Diagnostics: {:?}",
|
||||
diagnostics
|
||||
);
|
||||
|
||||
// Optional: Verify extraction produced some output despite error
|
||||
assert!(extraction_result.pages.len() > 0, "Should extract at least one page");
|
||||
}
|
||||
```
|
||||
|
||||
## Edge Cases and Considerations
|
||||
## What the Assertion Should Validate
|
||||
|
||||
1. **Partial bytes vs error**: Truncated streams should return partial bytes AND emit a diagnostic (not `Err()`)
|
||||
2. **Multiple diagnostics**: Some fixtures may emit multiple diagnostics - check all expected ones
|
||||
3. **Bomb limit tests**: `flate_bomb_3gb` should emit `StreamBomb` diagnostic
|
||||
4. **Unknown filter**: `unknown_filter` fixture should emit `StreamUnknownFilter` diagnostic
|
||||
1. **Primary validation**: That `"STREAM_DECODE_ERROR"` is present in `metadata.diagnostics`
|
||||
2. **Implicit validation**: That extraction succeeded (`result.is_ok()`) - confirms non-fatal error recovery per INV-8
|
||||
3. **Optional validation**: That partial output was produced (not zero pages)
|
||||
|
||||
## Files to Modify
|
||||
## Edge Cases and Error Conditions
|
||||
|
||||
1. **Test scaffold** (`crates/pdftract-core/tests/stream_decoder_fixtures.rs`):
|
||||
- Fix `flate_truncated.expected_diags` to `vec![DiagCode::StreamDecodeError]`
|
||||
- Add diagnostic validation loop after byte comparison
|
||||
- Change `decode_fixture()` to return `DecodeResult` instead of `Vec<u8>`
|
||||
1. **Fixture not found**: Skip test with clear message (standard pattern)
|
||||
2. **Multiple diagnostics**: Use `.contains()` to be flexible about exact string format
|
||||
3. **Case sensitivity**: The diagnostic string is `"STREAM_DECODE_ERROR"` (uppercase)
|
||||
4. **Alternative diagnostic codes**: Some code paths use `DecompressionFailed` internally but map to `StreamDecodeError`
|
||||
|
||||
2. **Stream decoder trait** (`crates/pdftract-core/src/parser/stream.rs`):
|
||||
- Either: use `decode_stream()` which already returns `DecodeResult` with diagnostics
|
||||
- Or: modify `StreamDecoder::decode()` trait to support diagnostics
|
||||
## Test File Location
|
||||
|
||||
Recommended: `/home/coding/pdftract/crates/pdftract-core/tests/test_truncated_flate_recovery.rs`
|
||||
|
||||
**Reasoning**:
|
||||
- Follows naming pattern: `test_<fixture>_<purpose>.rs`
|
||||
- Keeps malformed PDF tests isolated
|
||||
- Dedicated file for this specific test case
|
||||
|
||||
## Required Imports
|
||||
|
||||
```rust
|
||||
use pdftract_core::extract::extract_pdf;
|
||||
use pdftract_core::options::ExtractionOptions;
|
||||
use std::path::PathBuf;
|
||||
```
|
||||
|
||||
## Key Implementation Details
|
||||
|
||||
1. **Use `.contains()` not `.eq()`**: Diagnostic strings may have additional context
|
||||
2. **Check for presence not count**: Use `.any()` not exact count matching
|
||||
3. **Provide helpful failure message**: Include actual diagnostics in assert message
|
||||
4. **Follow TH-04 pattern**: That test demonstrates the canonical pattern for diagnostic assertions
|
||||
|
||||
## References
|
||||
|
||||
- **Pattern source**: `/home/coding/pdftract/crates/pdftract-core/tests/TH-04-js-presence.rs` lines 86-93
|
||||
- **Diagnostic enum**: `/home/coding/pdftract/crates/pdftract-core/src/diagnostics.rs`
|
||||
- **Extraction API**: `/home/coding/pdftract/crates/pdftract-core/src/extract.rs`
|
||||
- **Fixture location**: `/home/coding/pdftract/tests/fixtures/malformed/truncated-flate.pdf`
|
||||
|
||||
## Acceptance Criteria Status
|
||||
|
||||
- [x] Assertion purpose and scope defined
|
||||
- [x] Optimal assertion location identified (after line 326 in test loop)
|
||||
- [x] Assertion requirements documented (validate `STREAM_DECODE_ERROR` for `flate_truncated`)
|
||||
- [x] Assertion purpose and scope defined (validate STREAM_DECODE_ERROR in diagnostics)
|
||||
- [x] Optimal assertion location identified (after extraction succeeds, in diagnostics check)
|
||||
- [x] Assertion requirements documented (use .contains() on metadata.diagnostics Vec<String>)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue