f7fd0f7c9f
- Analyze current test assertions in error_recovery_integration.rs - Analyze current test assertions in stream_decoder_fixtures.rs - Identify 5 major coverage gaps in truncated-flate recovery testing - Document missing diagnostic emission in FlateDecoder - Document lack of actual extraction in integration test - Map out recommendations for new assertions at high/medium/low priority Closes bf-303t6.
300 lines
10 KiB
Markdown
300 lines
10 KiB
Markdown
# Test Coverage Analysis: Truncated-Flate Recovery
|
|
|
|
**Bead:** bf-303t6
|
|
**Date:** 2026-07-05
|
|
**Analysis:** Current test assertions and coverage gaps for truncated-flate error recovery
|
|
|
|
## Current Test Landscape
|
|
|
|
### 1. Integration-Level Test: `error_recovery_integration.rs::test_truncated_mid_stream()`
|
|
|
|
**Location:** `crates/pdftract-core/tests/error_recovery_integration.rs:165-188`
|
|
|
|
**Current Assertions:**
|
|
```rust
|
|
fn test_truncated_mid_stream() {
|
|
let fixture_path = fixture_path("truncated_mid_stream.pdf");
|
|
let expected = load_expected_diagnostics(&fixture_path);
|
|
|
|
let result = assert_no_panic("test_truncated_mid_stream", || {
|
|
let pdf_data = fs::read(&fixture_path).expect("fixture should exist");
|
|
assert!(pdf_data.starts_with(b"%PDF-"), "Should be a valid PDF");
|
|
|
|
// Verify expected: STREAM_DECODE_ERROR
|
|
let stream_diags: Vec<_> = expected
|
|
.expected_diagnostics
|
|
.iter()
|
|
.filter(|d| d.code.contains("STREAM_DECODE"))
|
|
.collect();
|
|
|
|
assert!(
|
|
!stream_diags.is_empty(),
|
|
"Should expect STREAM_DECODE_ERROR diagnostic"
|
|
);
|
|
});
|
|
|
|
assert!(result.is_ok(), "Test should not panic");
|
|
}
|
|
```
|
|
|
|
**What's Validated:**
|
|
- ✅ Fixture file exists and is valid PDF structure
|
|
- ✅ Expected diagnostics JSON file exists
|
|
- ✅ Expected diagnostics includes `STREAM_DECODE_ERROR`
|
|
- ✅ Test doesn't panic (INV-8 compliance)
|
|
|
|
**What's NOT Validated:**
|
|
- ❌ Actual extraction is never run
|
|
- ❌ No diagnostics are actually emitted or verified
|
|
- ❌ No verification of partial output behavior
|
|
- ❌ No verification that extraction succeeds despite truncation
|
|
|
|
### 2. Stream Decoder Test: `stream_decoder_fixtures.rs`
|
|
|
|
**Location:** `crates/pdftract-core/tests/stream_decoder_fixtures.rs:61-65`
|
|
|
|
**Fixture Configuration:**
|
|
```rust
|
|
FixtureInfo {
|
|
name: "flate_truncated",
|
|
filter: FixtureFilter::Single("FlateDecode", None),
|
|
expected_diags: vec![], // ⚠️ EMPTY - no diagnostics expected!
|
|
bomb_limit: None,
|
|
},
|
|
```
|
|
|
|
**Current Assertions:**
|
|
```rust
|
|
// Decode the fixture
|
|
let result = decode_fixture(&fixture, &input);
|
|
let decoded = match result {
|
|
Ok(data) => data,
|
|
Err(e) => {
|
|
failures.push(format!("{}: {}", fixture.name, e));
|
|
continue;
|
|
}
|
|
};
|
|
|
|
// Compare against expected
|
|
if &decoded[..expected_bytes.len().min(decoded.len())] != expected_bytes {
|
|
failures.push(format!(
|
|
"{}: output mismatch (expected {} bytes, got {} bytes)",
|
|
fixture.name,
|
|
expected.len(),
|
|
decoded.len()
|
|
));
|
|
continue;
|
|
}
|
|
```
|
|
|
|
**What's Validated:**
|
|
- ✅ Decoder doesn't return `Err` (returns `Ok` with partial bytes)
|
|
- ✅ Output matches expected file (0 bytes for truncated fixture)
|
|
- ✅ No panic occurs
|
|
|
|
**What's NOT Validated:**
|
|
- ❌ `STREAM_DECODE_ERROR` diagnostic is NOT verified (expected_diags is empty!)
|
|
- ❌ Metadata says "expects partial bytes + STREAM_DECODE_ERROR" but test doesn't verify
|
|
- ❌ No verification that error is properly propagated
|
|
- ❌ No distinction between success and partial recovery
|
|
|
|
### 3. Implementation: `FlateDecoder::decode_impl()`
|
|
|
|
**Location:** `crates/pdftract-core/src/parser/stream.rs:524-558`
|
|
|
|
**Error Handling:**
|
|
```rust
|
|
fn decode_impl<R: std::io::Read>(
|
|
mut decoder: R,
|
|
doc_counter: &mut u64,
|
|
max_bytes: u64,
|
|
) -> Vec<u8> {
|
|
let mut output = Vec::new();
|
|
let mut chunk = vec![0u8; BOMB_CHECK_CHUNK];
|
|
|
|
loop {
|
|
match decoder.read(&mut chunk) {
|
|
Ok(0) => break,
|
|
Ok(n) => {
|
|
// Check bomb limit BEFORE adding bytes to output
|
|
if *doc_counter + output.len() as u64 + n as u64 > max_bytes {
|
|
// Bomb limit exceeded - return partial bytes
|
|
let remaining = (max_bytes - *doc_counter - output.len() as u64) as usize;
|
|
let to_add = remaining.min(n);
|
|
output.extend_from_slice(&chunk[..to_add]);
|
|
return output;
|
|
}
|
|
output.extend_from_slice(&chunk[..n]);
|
|
}
|
|
Err(e) if e.kind() == std::io::ErrorKind::UnexpectedEof => {
|
|
// Truncated stream - return partial bytes (INV-8)
|
|
break; // ⚠️ SILENT - no diagnostic emitted
|
|
}
|
|
Err(_) => {
|
|
// Other decoder errors - return partial bytes decoded so far
|
|
break; // ⚠️ SILENT - no diagnostic emitted
|
|
}
|
|
}
|
|
}
|
|
|
|
output // ⚠️ Returns Vec<u8> - caller can't detect error occurred
|
|
}
|
|
```
|
|
|
|
**Critical Gaps:**
|
|
- ❌ `UnexpectedEof` is caught but no diagnostic is emitted
|
|
- ❌ Other decoder errors are silently caught
|
|
- ❌ Return type is `Vec<u8>`, not `Result<Vec<u8>, FilterError>`
|
|
- ❌ Caller has no way to distinguish success from partial recovery
|
|
|
|
## Coverage Gap Summary
|
|
|
|
### Gap 1: Missing Diagnostic Emission
|
|
**Severity:** HIGH
|
|
**Issue:** FlateDecoder catches `UnexpectedEof` but never emits `STREAM_DECODE_ERROR` diagnostic
|
|
|
|
**Evidence:**
|
|
- Metadata file says: "expects partial bytes + STREAM_DECODE_ERROR"
|
|
- Implementation catches error but doesn't emit diagnostic
|
|
- Test has `expected_diags: vec![]` (empty)
|
|
|
|
**Impact:**
|
|
- Diagnostics system is bypassed
|
|
- Users can't detect truncated streams in their PDFs
|
|
- INV-8 requires graceful recovery, but silent recovery hides issues
|
|
|
|
### Gap 2: No Error Indication in Return Type
|
|
**Severity:** HIGH
|
|
**Issue:** `decode_impl()` returns `Vec<u8>` directly, not `Result`
|
|
|
|
**Evidence:**
|
|
- Method signature: `fn decode_impl(...) -> Vec<u8>`
|
|
- Both success and error return `Vec<u8>`
|
|
- Caller can't distinguish partial recovery from clean decode
|
|
|
|
**Impact:**
|
|
- No way for callers to know truncation occurred
|
|
- Violates principle of least surprise
|
|
- Makes debugging harder
|
|
|
|
### Gap 3: Integration Test Doesn't Extract
|
|
**Severity:** MEDIUM
|
|
**Issue:** `test_truncated_mid_stream()` validates JSON but doesn't run extraction
|
|
|
|
**Evidence:**
|
|
- Test reads PDF data but doesn't call `pdftract::extract()`
|
|
- Only validates fixture structure and expected diagnostics JSON
|
|
- Comment says: `// TODO: Extract with pdftract once API is available`
|
|
|
|
**Impact:**
|
|
- End-to-end behavior is untested
|
|
- Integration between stream decoder and extraction pipeline is unverified
|
|
- Real-world behavior with actual PDF extraction is unknown
|
|
|
|
### Gap 4: No Verification of Partial Output Quality
|
|
**Severity:** MEDIUM
|
|
**Issue:** Tests don't verify partial output is usable or meaningful
|
|
|
|
**Evidence:**
|
|
- `flate_truncated.expected` is 0 bytes (trivially passes)
|
|
- No test verifies partial data represents valid prefix
|
|
- No test verifies remaining bytes are sensible
|
|
|
|
**Impact:**
|
|
- Can't detect if partial output is garbage
|
|
- Can't verify recovery produces useful data
|
|
- Quality of partial recovery is untested
|
|
|
|
### Gap 5: Stream Decoder Test Mismatch with Metadata
|
|
**Severity:** LOW
|
|
**Issue:** Test expects no diagnostics, metadata says STREAM_DECODE_ERROR expected
|
|
|
|
**Evidence:**
|
|
- `stream_decoder_fixtures.rs`: `expected_diags: vec![]`
|
|
- `flate_truncated.meta`: "expects partial bytes + STREAM_DECODE_ERROR"
|
|
|
|
**Impact:**
|
|
- Conflicting documentation
|
|
- Unclear what the actual expectation is
|
|
- Test may be out of sync with requirements
|
|
|
|
## Test Files Examined
|
|
|
|
1. **`crates/pdftract-core/tests/error_recovery_integration.rs`** (315 lines)
|
|
- Integration-level adversarial test corpus
|
|
- Tests 7 fixtures including `truncated_mid_stream.pdf`
|
|
|
|
2. **`crates/pdftract-core/tests/stream_decoder_fixtures.rs`** (394 lines)
|
|
- Unit-level stream decoder tests
|
|
- Tests 15+ fixtures including `flate_truncated`
|
|
|
|
3. **`crates/pdftract-core/src/parser/stream.rs`** (600+ lines)
|
|
- FlateDecoder implementation
|
|
- Error handling in `decode_impl()`
|
|
|
|
4. **Fixture files:**
|
|
- `/tests/stream_decoder/fixtures/flate_truncated.bin` (10 bytes, truncated zlib)
|
|
- `/tests/stream_decoder/fixtures/flate_truncated.expected` (0 bytes)
|
|
- `/tests/stream_decoder/fixtures/flate_truncated.meta` ("expects partial bytes + STREAM_DECODE_ERROR")
|
|
- `/tests/error_recovery/fixtures/truncated_mid_stream.pdf`
|
|
- `/tests/error_recovery/fixtures/truncated_mid_stream.expected_diagnostics.json`
|
|
|
|
## Recommendations for New Assertions
|
|
|
|
### High Priority (Required for Complete Coverage)
|
|
|
|
1. **Verify diagnostic emission in FlateDecoder**
|
|
- Assert `STREAM_DECODE_ERROR` is emitted on `UnexpectedEof`
|
|
- Update test: `expected_diags: vec![DiagCode::StreamDecodeError]`
|
|
|
|
2. **Add integration test that actually extracts**
|
|
- Call `pdftract::extract()` on `truncated_mid_stream.pdf`
|
|
- Verify extraction succeeds despite truncation
|
|
- Verify diagnostics contain `STREAM_DECODE_ERROR`
|
|
|
|
3. **Verify partial output is meaningful**
|
|
- Create fixture with known valid prefix
|
|
- Assert decoded bytes match expected prefix
|
|
- Verify partial data isn't garbage
|
|
|
|
### Medium Priority (Improves Robustness)
|
|
|
|
4. **Test error propagation through extraction pipeline**
|
|
- Verify stream decoder errors are properly reported
|
|
- Test diagnostics are collected and surfaced
|
|
- Verify no panic occurs (INV-8)
|
|
|
|
5. **Add test for multiple truncation scenarios**
|
|
- Truncated at different points (early, mid, late)
|
|
- Truncated with different filter types (Flate, LZW, etc.)
|
|
- Truncated in filter arrays (multi-stage decoding)
|
|
|
|
### Low Priority (Nice to Have)
|
|
|
|
6. **Test bomb limit interaction with truncation**
|
|
- Verify truncation before bomb limit
|
|
- Verify truncation after bomb limit
|
|
- Verify both diagnostics can co-occur
|
|
|
|
7. **Add regression test for specific real-world PDFs**
|
|
- Collect corpus of PDFs with truncated streams
|
|
- Test each produces reasonable output
|
|
- Track quality of partial recovery over time
|
|
|
|
## Conclusion
|
|
|
|
The current test coverage for truncated-flate recovery has **significant gaps**:
|
|
|
|
- ✅ **Basic INV-8 compliance is tested**: No panics occur
|
|
- ❌ **Diagnostic emission is NOT tested**: Stream decoder doesn't emit diagnostics
|
|
- ❌ **Integration behavior is NOT tested**: PDF extraction is never called
|
|
- ❌ **Output quality is NOT verified**: Partial data quality is unchecked
|
|
- ❌ **Error detection is impossible**: Return type can't indicate errors
|
|
|
|
**Most Critical Gap:** The FlateDecoder silently catches `UnexpectedEof` but never emits `STREAM_DECODE_ERROR` diagnostic, making it impossible for users or downstream code to detect that truncation occurred.
|
|
|
|
**Path Forward:**
|
|
1. Fix FlateDecoder to emit diagnostics
|
|
2. Update stream decoder test to expect diagnostics
|
|
3. Make integration test actually run extraction
|
|
4. Add assertions for partial output quality
|