Compare commits

..
12 Commits
Author SHA1 Message Date
Antoine AflaloandGitHub 868347cc55 Merge pull request #215 from Belphemur/renovate/actions-setup-go-7.x
chore(deps): update actions/setup-go action to v7
2026-07-21 20:26:52 -04:00
Antoine AflaloandGitHub bf21d3842f Merge pull request #217 from Belphemur/feat/keep-filenames
feat(converter): add --keep-filenames flag to preserve original page filenames
2026-07-21 20:26:36 -04:00
Antoine Aflalo b58999c6ec fix(loader): deduplicate original names by stem to prevent same-stem conversion race
CodeRabbit review on #217: allocateUniqueBaseName deduplicated by full
filename, but intermediatePageName strips extensions, so a/page.png and
b/page.jpg both converted to outputDir/page.webp concurrently. Track
used stems instead so colliding pairs resolve to e.g. page.png +
page_0001.jpg, keeping both intermediate and final names unique.
2026-07-21 20:13:02 -04:00
Antoine Aflalo 4cb5d10d3f fix(loader): normalize archive separators before preserving filenames
CodeRabbit review on #217: a ZIP entry named with Windows separators
(e.g. "..\outside.png") survives filepath.Base on Unix and would be
written verbatim as a ZIP entry name, which Windows consumers may
interpret as path traversal. Add archiveBaseName helper that replaces
"\" with "/" before deriving the bare base name for OriginalName.
2026-07-21 19:51:47 -04:00
Antoine Aflalo 21f92bc6f5 fix(converter): preserve OriginalName through conversion and fix race
- Fix critical bug: convertPageFile and splitAndConvert now copy
  OriginalName to the output PageFile so --keep-filenames works
  end-to-end on actually-converted pages.
- Fix concurrency race: ExtractChapter deduplicates OriginalName when
  source archive has same base filename in different subdirectories.
- Fix collision fallback: resolvePageName loops until an unused
  indexed name is found instead of single-shot assignment.
- Fix gofmt indentation in optimize_command.go.
2026-07-21 19:45:25 -04:00
Antoine Aflalo f3ba585c13 feat(converter): add --keep-filenames flag to preserve original page filenames
Add a new `--keep-filenames` boolean flag (no shorthand, default false) on
both `optimize` and `watch` commands. When enabled, page entries in the
output CBZ preserve their original base filenames instead of being
renumbered to `%04d` sequential indices. Extension is swapped on format
conversion (e.g. `001.png` → `001.webp`), split pages get `-NN` suffixes
on the original stem, and duplicate base names fall back to
`<stem>_<index>.<ext>`. Watch mode supports the flag via the
`keep-filenames` viper config key.

Design: data-driven `OriginalName` field on `manga.PageFile`, set at
extraction in `cbz_loader.ExtractChapter`. The `Converter` interface is
unchanged. Flag off → behavior identical to before.

Closes #216
2026-07-21 19:38:17 -04:00
renovate[bot]andGitHub e56b70b038 chore(deps): update actions/setup-go action to v7 2026-07-16 04:49:34 +00:00
Antoine AflaloandGitHub 650623b340 Merge pull request #214 from Belphemur/copilot/avoid-runnning-big-file-test
chore: remove Git LFS large-file test and fixture
2026-07-11 08:14:06 -04:00
4b961f18e3 chore: stop tracking build artifact binaries
Co-authored-by: Belphemur <197810+Belphemur@users.noreply.github.com>
2026-07-11 12:12:21 +00:00
2dfc39505c chore: remove Git LFS large-file test and fixture
Co-authored-by: Belphemur <197810+Belphemur@users.noreply.github.com>
2026-07-11 12:06:33 +00:00
e596a026dd chore: remove Git LFS large-file test and fixture
Co-authored-by: Belphemur <197810+Belphemur@users.noreply.github.com>
2026-07-11 12:05:02 +00:00
renovate[bot]GitHubrenovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
9423cde9f7 fix(deps): update module golang.org/x/image to v0.44.0 (#213)
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
2026-07-08 20:12:54 +00:00
26 changed files with 608 additions and 181 deletions
-1
View File
@@ -1 +0,0 @@
testdata/large/*.cbz filter=lfs diff=lfs merge=lfs -text
-9
View File
@@ -95,15 +95,6 @@ go test -v ./pkg/converter/...
go test -v ./internal/utils/... go test -v ./internal/utils/...
``` ```
A large-file integration test (`TestOptimizeIntegration_LargeFile`) exercises the
optimize pipeline against a ~1GB CBZ fixture stored via Git LFS
(`testdata/large/large_chapter.cbz`, tracked in `.gitattributes`). It is skipped
by default; fetch the fixture with `git lfs pull` and opt in with:
```bash
CBZ_RUN_LARGE_FILE_TEST=1 go test -v ./internal/utils/... -run TestOptimizeIntegration_LargeFile
```
### Linting ### Linting
```bash ```bash
+1 -1
View File
@@ -14,7 +14,7 @@ jobs:
uses: actions/checkout@v7 uses: actions/checkout@v7
- name: Set up Go - name: Set up Go
uses: actions/setup-go@v6 uses: actions/setup-go@v7
with: with:
go-version-file: go.mod go-version-file: go.mod
cache: true cache: true
+1 -1
View File
@@ -24,7 +24,7 @@ jobs:
with: with:
fetch-depth: 0 # this is important, otherwise it won't checkout the full tree (i.e. no previous tags) fetch-depth: 0 # this is important, otherwise it won't checkout the full tree (i.e. no previous tags)
- name: Set up Go - name: Set up Go
uses: actions/setup-go@v6 uses: actions/setup-go@v7
with: with:
go-version-file: go.mod go-version-file: go.mod
cache: true cache: true
+1 -1
View File
@@ -12,7 +12,7 @@ jobs:
- uses: actions/checkout@v7 - uses: actions/checkout@v7
- name: Set up Go - name: Set up Go
uses: actions/setup-go@v6 uses: actions/setup-go@v7
with: with:
go-version-file: go.mod go-version-file: go.mod
cache: true cache: true
+4
View File
@@ -11,6 +11,10 @@
# Test binary, built with `go test -c` # Test binary, built with `go test -c`
*.test *.test
# Build artifacts
/cbzoptimizer
/encoder-setup
test/ test/
# Output of the go coverage tool, specifically when used with LiteIDE # Output of the go coverage tool, specifically when used with LiteIDE
+7
View File
@@ -58,6 +58,12 @@ cbzconverter optimize [folder] --format=webp
cbzconverter optimize [folder] --format WEBP cbzconverter optimize [folder] --format WEBP
``` ```
Preserve original page filenames in the output CBZ instead of sequential renumbering:
```sh
cbzconverter optimize [folder] --keep-filenames --quality 85 --format webp
```
With timeout to avoid hanging on problematic chapters: With timeout to avoid hanging on problematic chapters:
```sh ```sh
@@ -103,6 +109,7 @@ docker run -v /path/to/comics:/comics ghcr.io/belphemur/cbzoptimizer:latest watc
- `--format`, `-f`: Format to convert the images to (currently supports: webp). Default is webp. - `--format`, `-f`: Format to convert the images to (currently supports: webp). Default is webp.
- Can be specified as: `--format webp`, `-f webp`, or `--format=webp` - Can be specified as: `--format webp`, `-f webp`, or `--format=webp`
- Case-insensitive: `webp`, `WEBP`, and `WebP` are all valid - Case-insensitive: `webp`, `WEBP`, and `WebP` are all valid
- `--keep-filenames`: Preserve each page's original base filename in the output CBZ (with the extension swapped for format conversion) instead of renumbering pages to `0000`, `0001`, ... Default is false. When two source pages share the same base filename, the second one falls back to the indexed form so the output stays a valid zip.
- `--timeout`, `-t`: Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout. Default is 0. - `--timeout`, `-t`: Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout. Default is 0.
- `--backfill`: *(`watch` only)* Optimize CBZ/CBR files that already exist in the watched folder at startup, before watching for new changes. Default is false. - `--backfill`: *(`watch` only)* Optimize CBZ/CBR files that already exist in the watched folder at startup, before watching for new changes. Default is false.
- `--log`, `-l`: Set log level; can be 'panic', 'fatal', 'error', 'warn', 'info', 'debug', or 'trace'. Default is info. - `--log`, `-l`: Set log level; can be 'panic', 'fatal', 'error', 'warn', 'info', 'debug', or 'trace'. Default is info.
BIN
View File
Binary file not shown.
+14
View File
@@ -70,6 +70,19 @@ func setupSplitFlag(cmd *cobra.Command, defaultValue bool, bindViper bool) {
} }
} }
// setupKeepFilenamesFlag sets up the keep-filenames flag for a command.
//
// Parameters:
// - cmd: The Cobra command to add the keep-filenames flag to
// - defaultValue: The default keep-filenames value
// - bindViper: If true, binds the flag to viper for configuration file support
func setupKeepFilenamesFlag(cmd *cobra.Command, defaultValue bool, bindViper bool) {
cmd.Flags().Bool("keep-filenames", defaultValue, "Preserve original page filenames instead of renumbering to sequential indices")
if bindViper {
_ = viper.BindPFlag("keep-filenames", cmd.Flags().Lookup("keep-filenames"))
}
}
// setupTimeoutFlag sets up the timeout flag for a command. // setupTimeoutFlag sets up the timeout flag for a command.
// //
// Parameters: // Parameters:
@@ -96,5 +109,6 @@ func setupCommonFlags(cmd *cobra.Command, converterType *constant.ConversionForm
setupQualityFlag(cmd, qualityDefault, bindViper) setupQualityFlag(cmd, qualityDefault, bindViper)
setupOverrideFlag(cmd, overrideDefault, bindViper) setupOverrideFlag(cmd, overrideDefault, bindViper)
setupSplitFlag(cmd, splitDefault, bindViper) setupSplitFlag(cmd, splitDefault, bindViper)
setupKeepFilenamesFlag(cmd, false, bindViper)
setupTimeoutFlag(cmd, bindViper) setupTimeoutFlag(cmd, bindViper)
} }
@@ -73,6 +73,13 @@ func ConvertCbzCommand(cmd *cobra.Command, args []string) error {
} }
log.Debug().Bool("split", split).Msg("Split parameter parsed") log.Debug().Bool("split", split).Msg("Split parameter parsed")
keepFilenames, err := cmd.Flags().GetBool("keep-filenames")
if err != nil {
log.Error().Err(err).Msg("Failed to parse keep-filenames flag")
return fmt.Errorf("invalid keep-filenames value")
}
log.Debug().Bool("keep-filenames", keepFilenames).Msg("Keep-filenames parameter parsed")
timeout, err := cmd.Flags().GetDuration("timeout") timeout, err := cmd.Flags().GetDuration("timeout")
if err != nil { if err != nil {
log.Error().Err(err).Msg("Failed to parse timeout flag") log.Error().Err(err).Msg("Failed to parse timeout flag")
@@ -127,6 +134,7 @@ func ConvertCbzCommand(cmd *cobra.Command, args []string) error {
Quality: quality, Quality: quality,
Override: override, Override: override,
Split: split, Split: split,
KeepFilenames: keepFilenames,
Timeout: timeout, Timeout: timeout,
}) })
if err != nil { if err != nil {
@@ -86,6 +86,7 @@ func TestConvertCbzCommand(t *testing.T) {
cmd.Flags().IntP("parallelism", "n", 2, "Number of chapters to convert in parallel") cmd.Flags().IntP("parallelism", "n", 2, "Number of chapters to convert in parallel")
cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files")
cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks")
cmd.Flags().Bool("keep-filenames", false, "Preserve original page filenames instead of renumbering to sequential indices")
cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout") cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout")
// Execute the command // Execute the command
@@ -197,6 +198,7 @@ func setupTestCommand(t *testing.T) (*cobra.Command, func()) {
cmd.Flags().IntP("parallelism", "n", 1, "Number of chapters to convert in parallel") cmd.Flags().IntP("parallelism", "n", 1, "Number of chapters to convert in parallel")
cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files")
cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks")
cmd.Flags().Bool("keep-filenames", false, "Preserve original page filenames instead of renumbering to sequential indices")
cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter") cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter")
// Reset converterType to default before test for consistency // Reset converterType to default before test for consistency
@@ -433,6 +435,7 @@ func TestConvertCbzCommand_ManyFiles_NoDeadlock(t *testing.T) {
cmd.Flags().IntP("parallelism", "n", 2, "Number of chapters to convert in parallel") cmd.Flags().IntP("parallelism", "n", 2, "Number of chapters to convert in parallel")
cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files")
cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks")
cmd.Flags().Bool("keep-filenames", false, "Preserve original page filenames instead of renumbering to sequential indices")
cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter") cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter")
converterType = constant.DefaultConversion converterType = constant.DefaultConversion
@@ -535,6 +538,7 @@ func TestConvertCbzCommand_HighParallelism_NoDeadlock(t *testing.T) {
cmd.Flags().IntP("parallelism", "n", 8, "Number of chapters to convert in parallel") cmd.Flags().IntP("parallelism", "n", 8, "Number of chapters to convert in parallel")
cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files")
cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks")
cmd.Flags().Bool("keep-filenames", false, "Preserve original page filenames instead of renumbering to sequential indices")
cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter") cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter")
converterType = constant.DefaultConversion converterType = constant.DefaultConversion
+4 -1
View File
@@ -65,6 +65,8 @@ func WatchCommand(_ *cobra.Command, args []string) error {
split := viper.GetBool("split") split := viper.GetBool("split")
keepFilenames := viper.GetBool("keep-filenames")
timeout := viper.GetDuration("timeout") timeout := viper.GetDuration("timeout")
backfill := viper.GetBool("backfill") backfill := viper.GetBool("backfill")
@@ -79,7 +81,7 @@ func WatchCommand(_ *cobra.Command, args []string) error {
if err != nil { if err != nil {
return fmt.Errorf("failed to prepare converter: %w", err) return fmt.Errorf("failed to prepare converter: %w", err)
} }
log.Info().Str("path", path).Bool("override", override).Uint8("quality", quality).Str("format", converterType.String()).Bool("split", split).Bool("backfill", backfill).Msg("Watching directory") log.Info().Str("path", path).Bool("override", override).Uint8("quality", quality).Str("format", converterType.String()).Bool("split", split).Bool("keep_filenames", keepFilenames).Bool("backfill", backfill).Msg("Watching directory")
watcher, err := fsnotify.NewWatcher() watcher, err := fsnotify.NewWatcher()
if err != nil { if err != nil {
@@ -98,6 +100,7 @@ func WatchCommand(_ *cobra.Command, args []string) error {
Quality: quality, Quality: quality,
Override: override, Override: override,
Split: split, Split: split,
KeepFilenames: keepFilenames,
Timeout: timeout, Timeout: timeout,
}) })
defer queue.Stop() defer queue.Stop()
BIN
View File
Binary file not shown.
+3 -3
View File
@@ -13,7 +13,7 @@ require (
github.com/spf13/viper v1.21.0 github.com/spf13/viper v1.21.0
github.com/stretchr/testify v1.11.1 github.com/stretchr/testify v1.11.1
github.com/thediveo/enumflag/v2 v2.2.1 github.com/thediveo/enumflag/v2 v2.2.1
golang.org/x/image v0.43.0 golang.org/x/image v0.44.0
) )
require ( require (
@@ -50,7 +50,7 @@ require (
go4.org v0.0.0-20230225012048-214862532bf5 // indirect go4.org v0.0.0-20230225012048-214862532bf5 // indirect
golang.org/x/net v0.56.0 // indirect golang.org/x/net v0.56.0 // indirect
golang.org/x/sys v0.46.0 // indirect golang.org/x/sys v0.46.0 // indirect
golang.org/x/text v0.38.0 // indirect golang.org/x/text v0.40.0 // indirect
golang.org/x/tools v0.46.0 // indirect golang.org/x/tools v0.47.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect
) )
+5
View File
@@ -217,6 +217,8 @@ golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMx
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/image v0.43.0 h1:FLxcP4ec2350nTfOC8ysKtqYSIFbk/QGjw1ZHNP4tsY= golang.org/x/image v0.43.0 h1:FLxcP4ec2350nTfOC8ysKtqYSIFbk/QGjw1ZHNP4tsY=
golang.org/x/image v0.43.0/go.mod h1:rrpelvGFt+kLPAjPM4HeWPgrl0FtafueU//e5N0qk/Q= golang.org/x/image v0.43.0/go.mod h1:rrpelvGFt+kLPAjPM4HeWPgrl0FtafueU//e5N0qk/Q=
golang.org/x/image v0.44.0 h1:+tDekMZED9+LrtB3G5xzRggpVh9CARjZqROla3R3R+I=
golang.org/x/image v0.44.0/go.mod h1:V8K3KE9KKKE+pLpQDOeN18w9oacNSvy1tDOirTu4xtY=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
golang.org/x/lint v0.0.0-20190301231843-5614ed5bae6f/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190301231843-5614ed5bae6f/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
@@ -299,6 +301,8 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/text v0.38.0 h1:sXmwo9DwP3OK9EZ7PqAdaooSGozfl/3a6/xJcbzPRhE= golang.org/x/text v0.38.0 h1:sXmwo9DwP3OK9EZ7PqAdaooSGozfl/3a6/xJcbzPRhE=
golang.org/x/text v0.38.0/go.mod h1:YXZt3QhHUKYT53r2lLKFIVi6Ao1jdzrTR/KQ09qyxF4= golang.org/x/text v0.38.0/go.mod h1:YXZt3QhHUKYT53r2lLKFIVi6Ao1jdzrTR/KQ09qyxF4=
golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs=
golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
@@ -328,6 +332,7 @@ golang.org/x/tools v0.0.0-20200212150539-ea181f53ac56/go.mod h1:TB2adYChydJhpapK
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
golang.org/x/tools v0.46.0 h1:7jTurBkPZu4moS/Uy4OQT1M+QBlsj3wejyZwsT8Z7rk= golang.org/x/tools v0.46.0 h1:7jTurBkPZu4moS/Uy4OQT1M+QBlsj3wejyZwsT8Z7rk=
golang.org/x/tools v0.46.0/go.mod h1:FrD85F8l+NWL+9XWBSyVSHO6Ne4jutsfIFba7AWQ5Ys= golang.org/x/tools v0.46.0/go.mod h1:FrD85F8l+NWL+9XWBSyVSHO6Ne4jutsfIFba7AWQ5Ys=
golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+59 -7
View File
@@ -5,6 +5,8 @@ import (
"fmt" "fmt"
"io" "io"
"os" "os"
"path/filepath"
"strings"
"time" "time"
"github.com/belphemur/CBZOptimizer/v2/internal/manga" "github.com/belphemur/CBZOptimizer/v2/internal/manga"
@@ -12,6 +14,56 @@ import (
"github.com/rs/zerolog/log" "github.com/rs/zerolog/log"
) )
// resolvePageName picks the final archive entry name for a page.
//
// Order of resolution:
// 1. When page.OriginalName is set (keep-filenames mode), use its stem with
// the current page.Extension. Split pages append the -NN suffix so all
// parts of the same source still land in the archive.
// 2. If that final name has already been used in this archive, fall back to
// the indexed form (stem + "_%04d" + extension) so the zip stays valid.
// 3. Otherwise (OriginalName is empty), use the historical %04d / %04d-NN
// sequential naming.
//
// usedNames tracks every name already chosen for this archive and is mutated
// in place so the caller can keep a single map across the whole chapter.
func resolvePageName(page *manga.PageFile, usedNames map[string]struct{}) string {
if page.OriginalName != "" {
stem := strings.TrimSuffix(page.OriginalName, filepath.Ext(page.OriginalName))
var candidate string
if page.IsSplitted {
candidate = fmt.Sprintf("%s-%02d%s", stem, page.SplitPartIndex, page.Extension)
} else {
candidate = stem + page.Extension
}
if _, taken := usedNames[candidate]; !taken {
usedNames[candidate] = struct{}{}
return candidate
}
// Collision: fall back to the indexed form so the entry still gets
// written, but make it visibly distinct from the preserved one.
// Loop in case the fallback name is itself already taken (e.g. a
// source file literally named "cover_0001.png" colliding with a
// page-index 1 fallback "cover_0001.webp").
for suffix := page.Index; ; suffix++ {
fallback := fmt.Sprintf("%s_%04d%s", stem, suffix, page.Extension)
if _, taken := usedNames[fallback]; !taken {
usedNames[fallback] = struct{}{}
return fallback
}
}
}
if page.IsSplitted {
name := fmt.Sprintf("%04d-%02d%s", page.Index, page.SplitPartIndex, page.Extension)
usedNames[name] = struct{}{}
return name
}
name := fmt.Sprintf("%04d%s", page.Index, page.Extension)
usedNames[name] = struct{}{}
return name
}
// WriteChapterToCBZ creates a CBZ file from a Chapter by streaming page files // WriteChapterToCBZ creates a CBZ file from a Chapter by streaming page files
// from disk directly into the zip archive. No image data is held in memory. // from disk directly into the zip archive. No image data is held in memory.
func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) (err error) { func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) (err error) {
@@ -34,14 +86,14 @@ func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) (err error
zipWriter := zip.NewWriter(zipFile) zipWriter := zip.NewWriter(zipFile)
defer errs.Capture(&err, zipWriter.Close, "failed to close .cbz writer") defer errs.Capture(&err, zipWriter.Close, "failed to close .cbz writer")
// Write each page to the archive by streaming from disk // Write each page to the archive by streaming from disk.
// Final name resolution: when a page carries an OriginalName (recorded by
// ExtractChapter when --keep-filenames is on), preserve its stem and only
// swap the extension to the current page.Extension. Duplicates in the
// archive fall back to the indexed naming so the output stays a valid zip.
usedNames := make(map[string]struct{}, len(chapter.Pages))
for _, page := range chapter.Pages { for _, page := range chapter.Pages {
var fileName string fileName := resolvePageName(page, usedNames)
if page.IsSplitted {
fileName = fmt.Sprintf("%04d-%02d%s", page.Index, page.SplitPartIndex, page.Extension)
} else {
fileName = fmt.Sprintf("%04d%s", page.Index, page.Extension)
}
log.Debug(). log.Debug().
Str("output_path", outputFilePath). Str("output_path", outputFilePath).
+64
View File
@@ -101,6 +101,70 @@ func TestWriteChapterToCBZ(t *testing.T) {
}, },
expectedFiles: []string{"0000-01.jpg"}, expectedFiles: []string{"0000-01.jpg"},
}, },
{
// --keep-filenames: the page stem is reused as-is with the
// current Extension, regardless of the source extension.
name: "Preserve original filename, extension swap",
chapter: func(t *testing.T, dir string) *manga.Chapter {
return &manga.Chapter{
Pages: []*manga.PageFile{
{
Index: 0,
Extension: ".webp",
FilePath: createTempPage(t, dir, "image data", ".webp"),
OriginalName: "page01.png",
},
},
}
},
expectedFiles: []string{"page01.webp"},
},
{
// --keep-filenames for a split page: stem is preserved, the
// split part index still gets appended to keep parts distinct.
name: "Split page with OriginalName",
chapter: func(t *testing.T, dir string) *manga.Chapter {
return &manga.Chapter{
Pages: []*manga.PageFile{
{
Index: 0,
Extension: ".webp",
FilePath: createTempPage(t, dir, "split data", ".webp"),
IsSplitted: true,
SplitPartIndex: 2,
OriginalName: "tall.png",
},
},
}
},
expectedFiles: []string{"tall-02.webp"},
},
{
// Two pages share the same OriginalName (rare but possible
// with subdirectories): the first keeps the stem, the second
// falls back to the indexed form to avoid duplicate zip
// entries.
name: "Duplicate OriginalName falls back to indexed form",
chapter: func(t *testing.T, dir string) *manga.Chapter {
return &manga.Chapter{
Pages: []*manga.PageFile{
{
Index: 0,
Extension: ".webp",
FilePath: createTempPage(t, dir, "first", ".webp"),
OriginalName: "cover.png",
},
{
Index: 1,
Extension: ".webp",
FilePath: createTempPage(t, dir, "second", ".webp"),
OriginalName: "cover.png",
},
},
}
},
expectedFiles: []string{"cover.webp", "cover_0001.webp"},
},
} }
for _, tc := range testCases { for _, tc := range testCases {
+98 -6
View File
@@ -142,7 +142,13 @@ func IsAlreadyConverted(ctx context.Context, filePath string) (converted bool, e
// ExtractChapter extracts an archive (CBZ/CBR) to a temp directory on disk. // ExtractChapter extracts an archive (CBZ/CBR) to a temp directory on disk.
// Pages are streamed directly to files — no image data is held in memory. // Pages are streamed directly to files — no image data is held in memory.
// Returns a Chapter with PageFile entries pointing to extracted files. // Returns a Chapter with PageFile entries pointing to extracted files.
func ExtractChapter(ctx context.Context, filePath string) (*manga.Chapter, error) { //
// When keepFilenames is true, each PageFile has its OriginalName set to the
// base filename of the entry inside the archive. Downstream code uses that
// name to preserve the original page identity in the output CBZ (with the
// extension swapped for format conversion). When false, OriginalName stays
// empty and the sequential %04d naming convention is used instead.
func ExtractChapter(ctx context.Context, filePath string, keepFilenames bool) (*manga.Chapter, error) {
log.Debug().Str("file_path", filePath).Msg("Extracting chapter to disk") log.Debug().Str("file_path", filePath).Msg("Extracting chapter to disk")
// Create temp directory for extraction // Create temp directory for extraction
@@ -162,6 +168,22 @@ func ExtractChapter(ctx context.Context, filePath string) (*manga.Chapter, error
TempDir: tempDir, TempDir: tempDir,
} }
// usedOriginalStems tracks the STEM (filename without extension) of every
// OriginalName handed out in this chapter so stems stay unique across
// pages. Tracking stems (not full names) prevents a downstream race in
// pkg/converter/webp: the converter strips the OriginalName extension
// and appends a target-format suffix (e.g. ".webp"), so two pages that
// share a stem but differ in extension — e.g. a/page.png and b/page.jpg
// (legal in zip) — would otherwise race on a single shared intermediate
// output path. The map covers both same-stem-same-ext collisions (e.g.
// a/page.png + b/page.png) and same-stem-different-ext collisions (e.g.
// a/page.png + b/page.jpg). Allocated lazily so the keepFilenames=false
// path stays allocation-free.
var usedOriginalStems map[string]struct{}
if keepFilenames {
usedOriginalStems = make(map[string]struct{})
}
// For CBZ files, read metadata from zip comment // For CBZ files, read metadata from zip comment
pathLower := strings.ToLower(filepath.Ext(filePath)) pathLower := strings.ToLower(filepath.Ext(filePath))
if pathLower == ".cbz" { if pathLower == ".cbz" {
@@ -279,6 +301,9 @@ func ExtractChapter(ctx context.Context, filePath string) (*manga.Chapter, error
Extension: ext, Extension: ext,
FilePath: outputPath, FilePath: outputPath,
} }
if keepFilenames {
page.OriginalName = allocateUniqueBaseName(archiveBaseName(path), pageIndex, usedOriginalStems)
}
chapter.Pages = append(chapter.Pages, page) chapter.Pages = append(chapter.Pages, page)
log.Debug(). log.Debug().
@@ -319,9 +344,76 @@ func isJunkFile(path string) bool {
return false return false
} }
// LoadChapter extracts the chapter from a CBZ/CBR file to disk. // archiveBaseName returns the bare base name of an archive entry, with
// It delegates to ExtractChapter and always extracts all pages. // Windows-style backslash separators normalized to forward slashes before
// Use IsAlreadyConverted for a fast conversion status check without extraction. // the last-segment split. The archives library surfaces zip entry names
func LoadChapter(filePath string) (*manga.Chapter, error) { // verbatim, so a name like "..\evil.png" (common from Windows-created
return ExtractChapter(context.Background(), filePath) // archives) would otherwise pass through filepath.Base unchanged on non-
// Windows hosts and be written into the output CBZ as a path-traversal
// shape. Normalizing the separators first and then taking the segment
// after the final one guarantees the returned name is a bare base name
// with no directory components, no backslashes, and no forward slashes —
// which is what the downstream writer expects as a safe ZIP entry name.
//
// This intentionally does NOT touch filepath.Base calls in isJunkFile or
// the ComicInfo.xml / converted.txt detection: those comparisons just
// fail to match, and the entry then falls through to the non-image
// extension filter, so no traversal-shaped data escapes the loader.
func archiveBaseName(path string) string {
normalized := strings.ReplaceAll(path, "\\", "/")
if idx := strings.LastIndex(normalized, "/"); idx >= 0 {
return normalized[idx+1:]
}
return normalized
}
// allocateUniqueBaseName returns baseName when its stem is not already taken
// by an earlier page in this chapter, or a collision-resolved variant
// otherwise.
//
// Stem-uniqueness contract: the function guarantees that the STEM
// (filename without extension) of the returned OriginalName is unique
// among all names previously handed out from this chapter. That contract
// matches what downstream code relies on —
// pkg/converter/webp.intermediatePageName strips the OriginalName's
// extension and appends a target-format suffix (e.g. ".webp"), so two
// OriginalNames that share a stem would otherwise race on a single
// intermediate output path. The stem is computed the same way downstream
// does it (strings.TrimSuffix(name, filepath.Ext(name))) to keep both
// definitions in lock-step.
//
// On collision the resolved name matches the existing fallback style used
// by cbz_creator's resolvePageName: stem + "_%04d" + extension. The loop
// checks the CANDIDATE's stem (not the full generated name) so two files
// that share a stem but differ in extension — e.g. "page.png" and
// "page.jpg" — resolve to, e.g., "page.png" and "page_0001.jpg", each
// preserving its original extension. The starting suffix is pageIndex so
// the resolved name stays in the same neighborhood as the page's archive
// position. If the candidate's stem is itself already taken
// (astronomically rare: the source archive would need both the original
// name and a matching indexed name), the suffix is incremented until a
// free stem is found. The chosen name's stem is recorded in usedStems so
// subsequent calls cannot pick it again.
func allocateUniqueBaseName(baseName string, pageIndex uint16, usedStems map[string]struct{}) string {
ext := filepath.Ext(baseName)
stem := strings.TrimSuffix(baseName, ext)
if _, taken := usedStems[stem]; !taken {
usedStems[stem] = struct{}{}
return baseName
}
for suffix := int(pageIndex); ; suffix++ {
candidateStem := fmt.Sprintf("%s_%04d", stem, suffix)
if _, taken := usedStems[candidateStem]; !taken {
usedStems[candidateStem] = struct{}{}
return candidateStem + ext
}
}
}
// LoadChapter extracts the chapter from a CBZ/CBR file to disk.
// It delegates to ExtractChapter with keepFilenames=false and always
// extracts all pages. Use IsAlreadyConverted for a fast conversion status
// check without extraction.
func LoadChapter(filePath string) (*manga.Chapter, error) {
return ExtractChapter(context.Background(), filePath, false)
} }
+252 -7
View File
@@ -6,6 +6,8 @@ import (
"fmt" "fmt"
"os" "os"
"path/filepath" "path/filepath"
"regexp"
"strings"
"testing" "testing"
"time" "time"
@@ -157,12 +159,12 @@ func TestIsAlreadyConverted_CBZWithNonDateComment(t *testing.T) {
} }
func TestExtractChapter_NonexistentFile(t *testing.T) { func TestExtractChapter_NonexistentFile(t *testing.T) {
_, err := ExtractChapter(context.Background(), "/nonexistent/file.cbz") _, err := ExtractChapter(context.Background(), "/nonexistent/file.cbz", false)
require.Error(t, err) require.Error(t, err)
} }
func TestExtractChapter_PageExtensions(t *testing.T) { func TestExtractChapter_PageExtensions(t *testing.T) {
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz") chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", false)
require.NoError(t, err) require.NoError(t, err)
defer func() { _ = chapter.Cleanup() }() defer func() { _ = chapter.Cleanup() }()
@@ -174,17 +176,260 @@ func TestExtractChapter_PageExtensions(t *testing.T) {
} }
func TestExtractChapter_PagesHaveSequentialIndices(t *testing.T) { func TestExtractChapter_PagesHaveSequentialIndices(t *testing.T) {
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz") t.Run("default sequential naming", func(t *testing.T) {
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", false)
require.NoError(t, err) require.NoError(t, err)
defer func() { _ = chapter.Cleanup() }() defer func() { _ = chapter.Cleanup() }()
for i, page := range chapter.Pages { for i, page := range chapter.Pages {
assert.Equal(t, uint16(i), page.Index) assert.Equal(t, uint16(i), page.Index)
assert.Empty(t, page.OriginalName, "keep-filenames disabled: OriginalName must stay empty")
} }
})
t.Run("keep-filenames records OriginalName", func(t *testing.T) {
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", true)
require.NoError(t, err)
defer func() { _ = chapter.Cleanup() }()
require.NotEmpty(t, chapter.Pages)
for i, page := range chapter.Pages {
assert.Equal(t, uint16(i), page.Index)
assert.NotEmpty(t, page.OriginalName, "keep-filenames enabled: OriginalName must be set for every page")
// OriginalName must be a bare base name (no directory prefix), even
// when the source archive nests pages in subdirectories.
assert.Equal(t, filepath.Base(page.OriginalName), page.OriginalName,
"OriginalName should be a base filename with no path components")
}
})
}
// writeCollisionCBZ builds a CBZ containing the same image base name in two
// different subdirectories plus a normal unique page. The test fixture is
// specifically crafted to reproduce the race fixed in ExtractChapter: when
// keep-filenames naively copied filepath.Base(path) into OriginalName, both
// a/page.png and b/page.png would have ended up with the same stem and the
// WebP converter would have raced on a single shared intermediate path.
func writeCollisionCBZ(t *testing.T, dir string) string {
t.Helper()
cbzPath := filepath.Join(dir, "collisions.cbz")
f, err := os.Create(cbzPath)
require.NoError(t, err)
w := zip.NewWriter(f)
for _, entry := range []string{"a/page.png", "b/page.png", "cover.png"} {
fw, err := w.Create(entry)
require.NoError(t, err)
_, err = fw.Write([]byte("dummy image bytes for " + entry))
require.NoError(t, err)
}
require.NoError(t, w.Close())
require.NoError(t, f.Close())
return cbzPath
}
func TestExtractChapter_KeepFilenames_DeduplicatesCollidingNames(t *testing.T) {
tmpDir := t.TempDir()
cbzPath := writeCollisionCBZ(t, tmpDir)
t.Run("keep-filenames resolves colliding base names", func(t *testing.T) {
chapter, err := ExtractChapter(context.Background(), cbzPath, true)
require.NoError(t, err)
defer func() { _ = chapter.Cleanup() }()
require.Len(t, chapter.Pages, 3, "expected 3 pages: two colliding + one unique")
// All OriginalNames must be non-empty bare base names and unique.
seen := make(map[string]struct{}, len(chapter.Pages))
for _, page := range chapter.Pages {
assert.NotEmpty(t, page.OriginalName, "page %d must have OriginalName set", page.Index)
assert.Equal(t, filepath.Base(page.OriginalName), page.OriginalName,
"page %d OriginalName should be a bare base filename", page.Index)
if _, dup := seen[page.OriginalName]; dup {
t.Errorf("duplicate OriginalName across pages: %q", page.OriginalName)
}
seen[page.OriginalName] = struct{}{}
}
// The unique page keeps its original name; the two colliding pages
// must come out as one bare "page.png" and one indexed variant.
assert.Contains(t, seen, "page.png", "one colliding page should keep the bare page.png name")
assert.Contains(t, seen, "cover.png", "the unique page should keep its original name")
indexedPattern := regexp.MustCompile(`^page_\d{4}\.png$`)
indexedCount := 0
for name := range seen {
if indexedPattern.MatchString(name) {
indexedCount++
}
}
assert.Equal(t, 1, indexedCount,
"exactly one colliding page should be resolved to the page_NNNN.png pattern, got %v", seen)
})
t.Run("keep-filenames off leaves OriginalName empty (regression guard)", func(t *testing.T) {
// Same collision-prone archive, but with keep-filenames off: the
// dedup logic must not run, so OriginalName stays empty for every
// page. This is the historical default and must not regress.
chapter, err := ExtractChapter(context.Background(), cbzPath, false)
require.NoError(t, err)
defer func() { _ = chapter.Cleanup() }()
require.Len(t, chapter.Pages, 3)
for _, page := range chapter.Pages {
assert.Empty(t, page.OriginalName, "page %d must have empty OriginalName when keep-filenames is off", page.Index)
}
})
}
// writeBackslashCBZ builds a CBZ whose entry names mix Windows-style
// backslash separators with a normal forward-slash page. The fixture
// reproduces the CodeRabbit finding for PR #217: archive entries like
// "subdir\page.png" and "..\evil.png" must not surface verbatim as the
// OriginalName written into the output CBZ, since Windows ZIP consumers
// can interpret backslashes as path traversal.
func writeBackslashCBZ(t *testing.T, dir string) string {
t.Helper()
cbzPath := filepath.Join(dir, "backslash.cbz")
f, err := os.Create(cbzPath)
require.NoError(t, err)
w := zip.NewWriter(f)
// archive/zip stores entry names verbatim, so the bytes on the wire
// carry the backslashes through to fs.WalkDir in ExtractChapter.
for _, entry := range []string{`subdir\page.png`, `..\evil.png`, "cover.png"} {
fw, err := w.Create(entry)
require.NoError(t, err)
_, err = fw.Write([]byte("dummy image bytes for " + entry))
require.NoError(t, err)
}
require.NoError(t, w.Close())
require.NoError(t, f.Close())
return cbzPath
}
func TestExtractChapter_KeepFilenames_NormalizesWindowsSeparators(t *testing.T) {
tmpDir := t.TempDir()
cbzPath := writeBackslashCBZ(t, tmpDir)
chapter, err := ExtractChapter(context.Background(), cbzPath, true)
require.NoError(t, err)
defer func() { _ = chapter.Cleanup() }()
require.Len(t, chapter.Pages, 3, "expected 3 pages: two backslash entries + one normal")
// Every OriginalName must be a bare base name: no backslashes, no
// forward slashes, and unique across the chapter.
seen := make(map[string]struct{}, len(chapter.Pages))
for _, page := range chapter.Pages {
assert.NotEmpty(t, page.OriginalName, "page %d must have OriginalName set", page.Index)
assert.NotContains(t, page.OriginalName, `\`, "page %d OriginalName must not contain backslash, got %q", page.Index, page.OriginalName)
assert.NotContains(t, page.OriginalName, "/", "page %d OriginalName must not contain forward slash, got %q", page.Index, page.OriginalName)
assert.Equal(t, filepath.Base(page.OriginalName), page.OriginalName,
"page %d OriginalName should be a bare base filename", page.Index)
if _, dup := seen[page.OriginalName]; dup {
t.Errorf("duplicate OriginalName across pages: %q", page.OriginalName)
}
seen[page.OriginalName] = struct{}{}
}
// The two backslash entries must collapse to their bare bases; the
// "..\evil.png" entry's leading "..\evil" must not be carried through.
assert.Contains(t, seen, "page.png", "subdir\\page.png should normalize to bare page.png")
assert.Contains(t, seen, "evil.png", "..\\evil.png should normalize to bare evil.png")
assert.Contains(t, seen, "cover.png", "cover.png should pass through unchanged")
}
// writeSameStemDifferentExtCBZ builds a CBZ containing two same-stem pages
// with different extensions plus a distinct unique page. The fixture
// reproduces the CodeRabbit finding for PR #217: a/page.png and b/page.jpg
// share a stem but do NOT share a full filename, so naive full-name
// deduplication lets both OriginalNames through and the WebP converter then
// races on a single shared intermediate output path (outputDir/page.webp).
// The fix flips the dedup tracking to STEM-uniqueness, so one of the
// colliding pair must come out as a bare "page.<ext>" and the other as
// "page_NNNN.<ext>" — each preserving its original extension.
func writeSameStemDifferentExtCBZ(t *testing.T, dir string) string {
t.Helper()
cbzPath := filepath.Join(dir, "same_stem.cbz")
f, err := os.Create(cbzPath)
require.NoError(t, err)
w := zip.NewWriter(f)
for _, entry := range []string{"a/page.png", "b/page.jpg", "cover.png"} {
fw, err := w.Create(entry)
require.NoError(t, err)
_, err = fw.Write([]byte("dummy image bytes for " + entry))
require.NoError(t, err)
}
require.NoError(t, w.Close())
require.NoError(t, f.Close())
return cbzPath
}
func TestExtractChapter_KeepFilenames_DeduplicatesSameStemDifferentExtensions(t *testing.T) {
tmpDir := t.TempDir()
cbzPath := writeSameStemDifferentExtCBZ(t, tmpDir)
chapter, err := ExtractChapter(context.Background(), cbzPath, true)
require.NoError(t, err)
defer func() { _ = chapter.Cleanup() }()
require.Len(t, chapter.Pages, 3, "expected 3 pages: two same-stem + one unique")
// Every OriginalName must be a bare base name; full names AND stems
// must each be unique across the chapter. The stem-uniqueness check is
// the contract the WebP converter relies on (it strips the extension
// and appends ".webp" when computing intermediatePageName).
seenNames := make(map[string]struct{}, len(chapter.Pages))
seenStems := make(map[string]struct{}, len(chapter.Pages))
for _, page := range chapter.Pages {
assert.NotEmpty(t, page.OriginalName, "page %d must have OriginalName set", page.Index)
assert.Equal(t, filepath.Base(page.OriginalName), page.OriginalName,
"page %d OriginalName should be a bare base filename", page.Index)
if _, dup := seenNames[page.OriginalName]; dup {
t.Errorf("duplicate OriginalName full name across pages: %q", page.OriginalName)
}
seenNames[page.OriginalName] = struct{}{}
stem := strings.TrimSuffix(page.OriginalName, filepath.Ext(page.OriginalName))
if _, dup := seenStems[stem]; dup {
t.Errorf("duplicate OriginalName stem across pages: %q (from %q)", stem, page.OriginalName)
}
seenStems[stem] = struct{}{}
}
// The unique page keeps its original name untouched.
assert.Contains(t, seenNames, "cover.png", "the unique page should keep its original name")
// The colliding pair must split into one bare "page.<ext>" and one
// "page_NNNN.<ext>". The extensions must differ — each must keep the
// extension of the archive entry it came from. Walk order is not
// guaranteed, so the test checks both shapes regardless of which
// entry comes first.
barePattern := regexp.MustCompile(`^page\.(png|jpg)$`)
indexedPattern := regexp.MustCompile(`^page_\d{4}\.(png|jpg)$`)
var bareExt, indexedExt string
for name := range seenNames {
if barePattern.MatchString(name) {
bareExt = filepath.Ext(name)
}
if indexedPattern.MatchString(name) {
indexedExt = filepath.Ext(name)
}
}
assert.NotEmpty(t, bareExt,
"expected one bare page.<ext> OriginalName from the colliding pair, got %v", seenNames)
assert.NotEmpty(t, indexedExt,
"expected one page_NNNN.<ext> OriginalName from the colliding pair, got %v", seenNames)
assert.NotEqual(t, bareExt, indexedExt,
"the bare and indexed variants must keep their original different extensions, got bare=%q indexed=%q",
bareExt, indexedExt)
} }
func TestExtractChapter_Cleanup(t *testing.T) { func TestExtractChapter_Cleanup(t *testing.T) {
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz") chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", false)
require.NoError(t, err) require.NoError(t, err)
tempDir := chapter.TempDir tempDir := chapter.TempDir
@@ -199,7 +444,7 @@ func TestExtractChapter_Cleanup(t *testing.T) {
} }
func TestExtractChapter_CBR(t *testing.T) { func TestExtractChapter_CBR(t *testing.T) {
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 1.cbr") chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 1.cbr", false)
require.NoError(t, err) require.NoError(t, err)
defer func() { _ = chapter.Cleanup() }() defer func() { _ = chapter.Cleanup() }()
@@ -212,7 +457,7 @@ func TestExtractChapter_CBR(t *testing.T) {
} }
func TestExtractChapter_ConvertedStatus(t *testing.T) { func TestExtractChapter_ConvertedStatus(t *testing.T) {
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 10_converted.cbz") chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 10_converted.cbz", false)
require.NoError(t, err) require.NoError(t, err)
defer func() { _ = chapter.Cleanup() }() defer func() { _ = chapter.Cleanup() }()
@@ -356,7 +601,7 @@ func TestExtractChapter_WithConvertedTxt(t *testing.T) {
_ = w.Close() _ = w.Close()
_ = f.Close() _ = f.Close()
chapter, err := ExtractChapter(context.Background(), cbzPath) chapter, err := ExtractChapter(context.Background(), cbzPath, false)
require.NoError(t, err) require.NoError(t, err)
defer func() { _ = chapter.Cleanup() }() defer func() { _ = chapter.Cleanup() }()
+6
View File
@@ -13,4 +13,10 @@ type PageFile struct {
IsSplitted bool IsSplitted bool
// SplitPartIndex is the part index when the page was split. // SplitPartIndex is the part index when the page was split.
SplitPartIndex uint16 SplitPartIndex uint16
// OriginalName is the base filename (e.g. "page01.png") of the page as
// it appeared in the source archive, recorded when the --keep-filenames
// flag is enabled. Empty when the flag is off or the source name is
// unknown. When set, downstream code uses this stem (with the final
// Extension swapped in) instead of the default %04d sequential name.
OriginalName string
} }
+7 -1
View File
@@ -21,6 +21,11 @@ type OptimizeOptions struct {
Quality uint8 Quality uint8
Override bool Override bool
Split bool Split bool
// KeepFilenames preserves the original base filename of each page inside
// the output CBZ (with the extension swapped for format conversion)
// instead of the historical %04d sequential naming. Off by default so
// existing behavior is unchanged.
KeepFilenames bool
Timeout time.Duration Timeout time.Duration
} }
@@ -38,6 +43,7 @@ func Optimize(options *OptimizeOptions) error {
Uint8("quality", options.Quality). Uint8("quality", options.Quality).
Bool("override", options.Override). Bool("override", options.Override).
Bool("split", options.Split). Bool("split", options.Split).
Bool("keep_filenames", options.KeepFilenames).
Msg("Optimization parameters") Msg("Optimization parameters")
// Step 1: Fast conversion check before extracting (new requirement) // Step 1: Fast conversion check before extracting (new requirement)
@@ -64,7 +70,7 @@ func Optimize(options *OptimizeOptions) error {
extractCtx = context.Background() extractCtx = context.Background()
} }
chapter, err := cbz.ExtractChapter(extractCtx, options.Path) chapter, err := cbz.ExtractChapter(extractCtx, options.Path, options.KeepFilenames)
if err != nil { if err != nil {
log.Error().Str("file", options.Path).Err(err).Msg("Failed to extract chapter") log.Error().Str("file", options.Path).Err(err).Msg("Failed to extract chapter")
return fmt.Errorf("failed to extract chapter: %w", err) return fmt.Errorf("failed to extract chapter: %w", err)
@@ -51,13 +51,6 @@ func TestOptimizeIntegration(t *testing.T) {
if err != nil { if err != nil {
return err return err
} }
// Skip the "large" fixtures directory: it holds the Git LFS-tracked
// fixture used exclusively by TestOptimizeIntegration_LargeFile,
// which may only be a small LFS pointer file if the content wasn't
// fetched, and shouldn't be exercised by this generic test.
if info.IsDir() && filepath.Base(path) == "large" {
return filepath.SkipDir
}
if !info.IsDir() { if !info.IsDir() {
fileName := strings.ToLower(info.Name()) fileName := strings.ToLower(info.Name())
if (strings.HasSuffix(fileName, ".cbz") || strings.HasSuffix(fileName, ".cbr")) && !strings.Contains(fileName, "converted") { if (strings.HasSuffix(fileName, ".cbz") || strings.HasSuffix(fileName, ".cbr")) && !strings.Contains(fileName, "converted") {
@@ -1,121 +0,0 @@
package utils
import (
"fmt"
"io"
"os"
"path/filepath"
"runtime"
"strings"
"testing"
"github.com/belphemur/CBZOptimizer/v2/internal/utils/errs"
"github.com/belphemur/CBZOptimizer/v2/pkg/converter"
"github.com/belphemur/CBZOptimizer/v2/pkg/converter/constant"
"github.com/rs/zerolog/log"
)
// largeTestFile is a ~1GB synthetic CBZ fixture stored via Git LFS (see
// .gitattributes). It is used to exercise the optimize pipeline with a
// chapter large enough to make in-memory-only handling impractical, and to
// validate that converted pages are streamed to/from a staging temp folder
// (see manga.Page.TempFilePath / manga.Chapter.TempDir) instead of blowing
// up memory usage.
const largeTestFile = "../../testdata/large/large_chapter.cbz"
// TestOptimizeIntegration_LargeFile is opt-in (set CBZ_RUN_LARGE_FILE_TEST=1)
// since it processes a ~1GB fixture and can take a while to run. It is
// automatically skipped if the fixture is unavailable (e.g. Git LFS content
// wasn't fetched, leaving only a pointer file) or in short mode.
func TestOptimizeIntegration_LargeFile(t *testing.T) {
if testing.Short() {
t.Skip("Skipping large file integration test in short mode")
}
if os.Getenv("CBZ_RUN_LARGE_FILE_TEST") == "" {
t.Skip("Skipping large file integration test; set CBZ_RUN_LARGE_FILE_TEST=1 to run it")
}
info, err := os.Stat(largeTestFile)
if err != nil {
t.Skipf("large test fixture not found: %v", err)
}
// If Git LFS content wasn't fetched (e.g. `actions/checkout` without
// `lfs: true`), the file on disk is just a small pointer text file
// rather than the real ~1GB fixture. Detect and skip gracefully instead
// of failing the whole suite.
const minExpectedSize = 500 * 1024 * 1024 // 500MB
if info.Size() < minExpectedSize {
t.Skipf("large test fixture looks like a Git LFS pointer (size=%d), skipping; run `git lfs pull`", info.Size())
}
tempDir, err := os.MkdirTemp("", "test_optimize_large_file")
if err != nil {
t.Fatal(err)
}
defer errs.CaptureGeneric(&err, os.RemoveAll, tempDir, "failed to remove temporary directory")
converterInstance, err := converter.Get(constant.WebP)
if err != nil {
t.Skip("WebP converter not available, skipping large file integration test")
}
if err := converterInstance.PrepareConverter(); err != nil {
t.Skip("Failed to prepare WebP converter, skipping large file integration test")
}
cbzFile := filepath.Join(tempDir, "large_chapter.cbz")
if err := copyFile(largeTestFile, cbzFile); err != nil {
t.Fatal(err)
}
var memBefore, memAfter runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&memBefore)
options := &OptimizeOptions{
ChapterConverter: converterInstance,
Path: cbzFile,
Quality: 85,
Override: false,
Split: true,
}
err = Optimize(options)
if err != nil {
t.Fatalf("failed to optimize large chapter: %v", err)
}
runtime.GC()
runtime.ReadMemStats(&memAfter)
log.Info().
Uint64("heap_alloc_before", memBefore.HeapAlloc).
Uint64("heap_alloc_after", memAfter.HeapAlloc).
Int64("input_size", info.Size()).
Msg("Large file integration test memory usage")
outputFile := strings.TrimSuffix(cbzFile, ".cbz") + "_converted.cbz"
if _, err := os.Stat(outputFile); err != nil {
t.Fatalf("expected converted output file %s to exist: %v", outputFile, err)
}
}
// copyFile copies src to dst using streaming file I/O so that the whole
// file content is never held in memory at once, which matters for the
// large fixture used by this test.
func copyFile(src, dst string) (err error) {
in, err := os.Open(src)
if err != nil {
return fmt.Errorf("failed to open source file: %w", err)
}
defer errs.Capture(&err, in.Close, "failed to close source file")
out, err := os.Create(dst)
if err != nil {
return fmt.Errorf("failed to create destination file: %w", err)
}
defer errs.Capture(&err, out.Close, "failed to close destination file")
if _, err := io.Copy(out, in); err != nil {
return fmt.Errorf("failed to copy file contents: %w", err)
}
return nil
}
+29 -4
View File
@@ -25,6 +25,26 @@ import (
const webpMaxHeight = 16383 const webpMaxHeight = 16383
// intermediatePageName returns the on-disk filename used for a page's WebP
// intermediate during conversion. When the page carries an OriginalName
// (recorded by --keep-filenames), its stem is reused so the temp file line
// up with the name the archive writer will pick. Otherwise the historical
// %04d indexed naming is kept. The splitSuffix argument is appended verbatim
// after the stem (e.g. "-00", "-01") for split parts and is empty for the
// happy-path single output. A leading dash is only added when both the
// split suffix and the original-name stem are present, so the indexed form
// stays as %04d-%02d.
func intermediatePageName(page *manga.PageFile, splitSuffix string) string {
if page.OriginalName != "" {
stem := strings.TrimSuffix(page.OriginalName, filepath.Ext(page.OriginalName))
return stem + splitSuffix + ".webp"
}
if splitSuffix == "" {
return fmt.Sprintf("%04d.webp", page.Index)
}
return fmt.Sprintf("%04d%s.webp", page.Index, splitSuffix)
}
type Converter struct { type Converter struct {
maxHeight int maxHeight int
cropHeight int cropHeight int
@@ -218,22 +238,26 @@ func (converter *Converter) convertPageFile(ctx context.Context, page *manga.Pag
Str("input", page.FilePath). Str("input", page.FilePath).
Msg("Converting page file") Msg("Converting page file")
// If the page is already WebP, just return it as-is // If the page is already WebP, just return it as-is. The returned page
// keeps any OriginalName set during extraction so the archive writer can
// honor --keep-filenames for the final entry name.
if strings.ToLower(page.Extension) == ".webp" { if strings.ToLower(page.Extension) == ".webp" {
log.Debug().Uint16("page_index", page.Index).Msg("Page already WebP, skipping") log.Debug().Uint16("page_index", page.Index).Msg("Page already WebP, skipping")
return []*manga.PageFile{page}, nil return []*manga.PageFile{page}, nil
} }
// Try direct file-to-file conversion first (happy path — no memory allocation) // Try direct file-to-file conversion first (happy path — no memory allocation)
outputPath := filepath.Join(outputDir, fmt.Sprintf("%04d.webp", page.Index)) outputPath := filepath.Join(outputDir, intermediatePageName(page, ""))
err := EncodeFile(page.FilePath, outputPath, uint(quality)) err := EncodeFile(page.FilePath, outputPath, uint(quality))
if err == nil { if err == nil {
// Success! No image decoding needed. // Success! No image decoding needed. Preserve OriginalName so
// --keep-filenames carries through to the final zip entry name.
return []*manga.PageFile{{ return []*manga.PageFile{{
Index: page.Index, Index: page.Index,
Extension: ".webp", Extension: ".webp",
FilePath: outputPath, FilePath: outputPath,
OriginalName: page.OriginalName,
}}, nil }}, nil
} }
@@ -316,7 +340,7 @@ func (converter *Converter) splitAndConvert(ctx context.Context, page *manga.Pag
partHeight = height - yOffset partHeight = height - yOffset
} }
outputPath := filepath.Join(outputDir, fmt.Sprintf("%04d-%02d.webp", page.Index, i)) outputPath := filepath.Join(outputDir, intermediatePageName(page, fmt.Sprintf("-%02d", i)))
err := EncodeFileWithCrop(page.FilePath, outputPath, uint(quality), 0, yOffset, width, partHeight) err := EncodeFileWithCrop(page.FilePath, outputPath, uint(quality), 0, yOffset, width, partHeight)
if err != nil { if err != nil {
@@ -334,6 +358,7 @@ func (converter *Converter) splitAndConvert(ctx context.Context, page *manga.Pag
FilePath: outputPath, FilePath: outputPath,
IsSplitted: true, IsSplitted: true,
SplitPartIndex: uint16(i), SplitPartIndex: uint16(i),
OriginalName: page.OriginalName,
}) })
} }
+33
View File
@@ -485,3 +485,36 @@ func TestEncodeFileWithCrop(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
assert.Greater(t, info.Size(), int64(0)) assert.Greater(t, info.Size(), int64(0))
} }
// TestIntermediatePageName_StemsAreUnique pins the contract that
// intermediatePageName must produce a different on-disk filename for two
// pages whose OriginalNames share a stem via the original archive entry
// name. ExtractChapter guarantees stem-unique OriginalNames per chapter
// (so, e.g., a/page.png and b/page.jpg come out as "page.png" and
// "page_0001.jpg"), and the converter must reflect that: stripping the
// extension and appending ".webp" must not collapse two distinct stems
// onto the same intermediate path.
func TestIntermediatePageName_StemsAreUnique(t *testing.T) {
tests := []struct {
name string
originalName string
splitSuffix string
}{
{name: "bare stem", originalName: "page.png", splitSuffix: ""},
{name: "indexed stem (post-fix)", originalName: "page_0001.jpg", splitSuffix: ""},
{name: "bare stem + split suffix", originalName: "page.png", splitSuffix: "-00"},
{name: "indexed stem + split suffix", originalName: "page_0001.jpg", splitSuffix: "-00"},
}
names := make(map[string]struct{}, len(tests))
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
page := &manga.PageFile{Index: 0, OriginalName: tt.originalName}
got := intermediatePageName(page, tt.splitSuffix)
if _, dup := names[got]; dup {
t.Errorf("intermediate name %q collides with an earlier page's intermediate name", got)
}
names[got] = struct{}{}
})
}
}
-3
View File
@@ -1,3 +0,0 @@
version https://git-lfs.github.com/spec/v1
oid sha256:8880c8909521c3716a339a39a29a4bf1114921a8b4651ff8ca04da7ce3a54b6a
size 1001668690