fix(nzb): handle gzipped NZBs end-to-end; centralize in internal/nzbfile#533
Merged
fix(nzb): handle gzipped NZBs end-to-end; centralize in internal/nzbfile#533
Conversation
Three related bugs from the .nzb.gz persistence change were producing user-visible failures: - Download handler returned 404 for queue items whose stored NzbPath did not match the on-disk extension variant. - External SABnzbd fallback uploaded raw gzip bytes, which SABnzbd rejected with "XML syntax error on line 2: illegal character code U+001F". - Collision suffixing treated ".nzb.gz" as a single-extension ".gz", producing names like "movie.nzb_1.gz" that broke downstream scanners and the download handler's suffix check. Consolidate gzip NZB I/O in a new internal/nzbfile package (Open, Compress, IsGzipped, PlainFilename, GzExtension, ResolveOnDisk) and route all callers through it: API download handler, importer service (persist + size calc + metadata regen), processor, migration, and the SABnzbd client. Delete the importer-local nzb_file.go helpers and move their tests into the new package. Download now streams via io.Copy(c.Response().BodyWriter(), rc) instead of buffering the whole decompressed payload. Collision logic moves into a testable nextCollisionPath helper using nzbtrim.TrimNzbExtension.
Replace filesystem collision-suffix scanning with a deterministic "<base>_<queue_id>.nzb.gz" scheme so duplicate names are impossible by construction. AddToQueue now inserts the DB row first (assigning item.ID), then moves/compresses the file; on persistence failure the row is rolled back via RemoveFromQueue so the queue can't leak orphan entries pointing at temp paths. Drops the nextCollisionPath helper and its table-driven test — the problem it solved no longer exists.
…gle to Import User-facing changes: - Queue download button now surfaces backend errors via a toast instead of swallowing them silently. For completed items a 404 shows a softer "NZB file already removed" info toast, since the file was almost certainly cleaned up post-import. - The "delete NZB after import" option moves from the Metadata section (labelled "Aggressive Cleanup") to the Import section where users actually look for import-lifecycle options. Renamed to a clearer "Delete NZB After Import" with an explicit note that downloads will no longer work after this runs. Config migration: - DeleteCompletedNzb moves from MetadataConfig to ImportConfig in Go. - Config.ShouldDeleteCompletedNzb() prefers the new import.* key but falls back to the legacy metadata.* key so existing config files keep working without manual edits. - Download-cleanup no longer clears nzb_path in the DB, so the queue list keeps displaying the original filename.
…sion - Delete the on-disk NZB whenever a queue row is removed (single delete, bulk delete, clear completed/failed/pending, system cleanup, system stats reset). Repository clear functions now return the deleted paths alongside the count; BulkOperationResult gained a DeletedPaths slice. A new Server.removeQueueNzbFiles helper handles the actual os.Remove with missing-file tolerance and warn-on-other-errors. - Switch persisted .nzb.gz compression from gzip.BestSpeed to gzip.BestCompression. Imports run once; smaller on-disk files are worth the one-time CPU cost. Existing files remain readable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three user-visible failures introduced with the
.nzb.gzpersistence feature, and centralizes all gzip NZB I/O in a newinternal/nzbfilepackage so new call sites can't reintroduce the same bugs.Bugs fixed
GET /api/queue/{id}/downloadreturnedNOT_FOUND/ "The NZB file no longer exists on disk" whenever the DB-storedNzbPathextension drifted from what was actually on disk (legacy.nzbrows against gzipped files, or vice versa). The handler now callsnzbfile.ResolveOnDisk, which falls back to the.gz-toggled variant before 404-ing.U+001F.SABnzbdClient.SendNZBFilestreamed raw gzip bytes into the multipart upload; external SABnzbd rejected them (XML syntax error on line 2: illegal character code U+001F). Now opens vianzbfile.Openso the upload is plain NZB XML and the uploaded filename strips.gz..nzb.gzcompound extension.ensurePersistentNzbusedfilepath.Ext+TrimSuffix, which treated.nzb.gzas.gz, producingmovie.nzb_1.gz. The newnextCollisionPathhelper usesnzbtrim.TrimNzbExtensionand yieldsmovie_1.nzb.gz, keeping the file a valid NZB for downstream scanners and the download handler.Refactor
New
internal/nzbfilepackage owns gzip NZB I/O:Open(transparent decompress),Compress,IsGzipped,PlainFilename,GzExtension, andResolveOnDisk. All existing callers now go through it:internal/api/queue_handlers.go— download handler (streams viaio.Copy(c.Response().BodyWriter(), rc)rather than buffering withio.ReadAll).internal/sabnzbd/client.go— external SABnzbd fallback.internal/importer/service.go— persist, size calculation, metadata regen, collision handling.internal/importer/processor.go,internal/importer/migration.go— usenzbfile.Open/nzbfile.Compress.The old
internal/importer/nzb_file.go/_test.gohelpers were deleted; their logic moved intointernal/nzbfile.RegenerateMetadatawas simplified to usenzbtrim.HasNzbExtension+nzbtrim.TrimNzbExtensioninstead of an inlineswitch. UnusedPlainExtensionconstant and a dead_ = dirline in the test were removed.Test plan
go build ./...passesgo vet ./...passesgo test -race ./internal/nzbfile/ ./internal/importer/ ./internal/api/passesinternal/nzbfile/nzbfile_test.go(Open/Compress roundtrip, ResolveOnDisk drift in both directions, IsGzipped/PlainFilename case preservation) andinternal/importer/collision_test.go(table-drivennextCollisionPathincluding compound.nzb.gz, uppercase, taken-suffix skipping, plain.nzb)name_1.nzb.gzGET /api/queue/{id}/downloadfor a gzipped item → 200, body is plain NZB XML,Content-Dispositionfilename ends in.nzbU+001Ferror)