Skip to content

fix(rar): fail fast on incomplete archives instead of falsely completing#613

Open
evulhotdog wants to merge 2 commits into
javi11:mainfrom
evulhotdog:fix-incomplete-archive-validation
Open

fix(rar): fail fast on incomplete archives instead of falsely completing#613
evulhotdog wants to merge 2 commits into
javi11:mainfrom
evulhotdog:fix-incomplete-archive-validation

Conversation

@evulhotdog
Copy link
Copy Markdown

Three bugs caused altmount to falsely report NZBs as successfully imported when RAR volumes were missing from the Usenet server:

  1. ValidateSegmentIntegrity checked against PackedSize (sum of found parts) instead of Size (uncompressed full file size from RAR header). When a volume is missing, PackedSize shrinks and validation incorrectly passes.

  2. patchMissingSegment created overlapping fake segments with wrong offsets, falsely inflating perceived coverage without adding real data.

  3. Validation failures in aggregator.go were silently skipped by returning nil instead of propagating the error.

Changes:

  • common.go: validate against content.Size, fallback to PackedSize only if Size is unavailable
  • processor.go: fix patchMissingSegment to use correct StartOffset and SegmentSize
  • aggregator.go: return errors from both validation paths instead of nil

Result: incomplete archives now fail during import with a clear error, allowing ARR apps to retry instead of trying to read a non-existent file.

@javi11
Copy link
Copy Markdown
Owner

javi11 commented May 24, 2026

but with this if the failing parts are in a file inside the rar that is not the video file it will make all the file fails

@evulhotdog
Copy link
Copy Markdown
Author

@javi11 what do you think is a better path to address it? I do see the conversation in the issue I created. Maybe we can wait to see what happens there.

@evulhotdog evulhotdog force-pushed the fix-incomplete-archive-validation branch from ae982d1 to c335e8f Compare May 25, 2026 00:46
@evulhotdog
Copy link
Copy Markdown
Author

evulhotdog commented May 25, 2026

@javi11 I don't like the hardcoded extensions, but I'm not sure what other way to tackle this...

RAR archive validation flow

 rarContents
      │
      ▼
 hasAllowedFiles? ─── no ──▶ FAIL (ErrNoAllowedFiles)
      │
     yes
      │
      ▼
 Build filesToProcess list
      │
      │  ┌─────────────────────────────────────┐
      │  │  Per file:                          │
      │  │    IsDirectory?       ─▶▶ skip      │
      │  │    IsAllowedFile?     ─▶▶ skip      │
      │  │      (ext check + sample filter)    │
      │  │    keep ▶ add to validation pool    │
      │  └─────────────────────────────────────┘
      │
      ▼
 Parallel validation goroutines
      │
      ├── isPreExtracted? ─▶▶ skip validation
      │
      ├── validateSegmentIntegrity
      │       (checks Size, not PackedSize)
      │       │
      │       ├── passes ─▶▶ ValidateSegmentsForFile
      │       │                  │
      │       │                  ├── passes ─▶▶ ✓ write metadata
      │       │                  │
      │       │                  └── fails ───┐
      │       │                               │
      │       └── fails ──────────────────────┤
      │                                       │
      │              ◀────────────────────────┘
      │              │
      │              ▼
      │         isSidecarFile?
      │      (.srt .sub .jpg .nfo etc.)
      │              │
      │       ┌──────┴──────┐
      │       │             │
      │      yes           no
      │       │             │
      │       ▼             ▼
      │   ⚠ warn, skip   ✗ FAIL archive
      │   return nil      return err
      │       │             │
      │       └─────┬───────┘
      │             │
      │             ▼
      │        filesProcessed > 0?
      │             │
      │      ┌──────┴──────┐
      │      │             │
      │     yes           no
      │      │             │
      │      ▼             ▼
      │   ✓ SUCCESS    FAIL (ErrNoFilesProcessed)

 KEY:
   ▶▶  = file skipped silently
   ✓   = success path
   ✗   = failure path
   ⚠   = sidecar skip (new behavior)

@evulhotdog evulhotdog force-pushed the fix-incomplete-archive-validation branch 2 times, most recently from 8693e5a to 8cb57b1 Compare May 25, 2026 02:55
Three bugs caused altmount to falsely report NZBs as successfully imported
when RAR volumes were missing from the Usenet server:

1. ValidateSegmentIntegrity checked against PackedSize (sum of found parts)
   instead of Size (uncompressed full file size from RAR header). When a
   volume is missing, PackedSize shrinks and validation incorrectly passes.

2. patchMissingSegment created overlapping fake segments with wrong offsets,
   falsely inflating perceived coverage without adding real data.

3. Validation failures in aggregator.go were silently skipped by returning
   nil instead of propagating the error.

Changes:
- common.go: validate against content.Size, fallback to PackedSize only if
  Size is unavailable
- processor.go: fix patchMissingSegment to use correct StartOffset and
  SegmentSize
- aggregator.go: return errors from both validation paths instead of nil

Result: incomplete archives now fail during import with a clear error,
allowing ARR apps to retry instead of trying to read a non-existent file.
@evulhotdog evulhotdog force-pushed the fix-incomplete-archive-validation branch from 8cb57b1 to 864cba8 Compare May 25, 2026 02:57
@javi11
Copy link
Copy Markdown
Owner

javi11 commented May 25, 2026

So, if I understand well, what you will want is something like the allowed file extensions but that mark the file as failed if no video file is found available no?

@evulhotdog
Copy link
Copy Markdown
Author

@javi11 yes, but still being validated through the normal mechanisms, so we don't fail on a sample, nfo, etc. but do fail on the main file.

I don't know if there's a better path to go here. To salvage what we can.

It also may be a moot workaround, if we can implement par2 repairs on the fly.

@javi11
Copy link
Copy Markdown
Owner

javi11 commented May 25, 2026

It also may be a moot workaround, if we can implement par2 repairs on the fly. this is not technically possible.

Ok what I can do is, a setting to fail if the imported dosn't have any video file that is not a sample

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