diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..4c48209 --- /dev/null +++ b/.gitignore @@ -0,0 +1,21 @@ +# Build output +/rip +/rip.exe +*.test + +# Local runtime state +*.db +*.db-wal +*.db-shm +config.toml +*.bak-* + +# Scratch files +albums.txt +*.log +coverage.out + +# Editor / OS +.DS_Store +.idea/ +.vscode/ diff --git a/cmd/rip/help.go b/cmd/rip/help.go index 240d1d7..f81fa88 100644 --- a/cmd/rip/help.go +++ b/cmd/rip/help.go @@ -151,10 +151,10 @@ Inspect local download databases. func printDeveloperHelp(w io.Writer) { fmt.Fprint(w, `Developer smoke commands: soundcloud-smoke - qobuz-smoke, qobuz-rip-smoke, qobuz-convert-rip-smoke - qobuz-album-rip-smoke, qobuz-playlist-rip-smoke, qobuz-artist-rip-smoke, qobuz-label-rip-smoke - qobuz-search-smoke + qobuz-smoke, qobuz-convert-rip-smoke, qobuz-search-smoke tidal-search-smoke, tidal-metadata-smoke, tidal-video-smoke - tidal-rip-smoke, tidal-album-rip-smoke, tidal-playlist-rip-smoke, tidal-artist-rip-smoke + +The per-media-type rip smoke commands were removed; use the equivalent + rip id `) } diff --git a/cmd/rip/main.go b/cmd/rip/main.go index f726c34..53df584 100644 --- a/cmd/rip/main.go +++ b/cmd/rip/main.go @@ -20,40 +20,47 @@ import ( ) func main() { + os.Exit(run()) +} + +// run executes the CLI and returns the process exit code. It exists so that +// deferred cleanup (SQLite handles, progress bars, artwork temp dirs) always +// runs; calling os.Exit directly from the command bodies skipped every defer. +func run() int { gopts, err := parseGlobalArgs(os.Args[1:]) if err != nil { fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) + return 2 } if gopts.help { printMainHelp(os.Stdout) - return + return 0 } if gopts.command == "" { printMainHelp(os.Stdout) - os.Exit(2) + return 2 } if gopts.command == "help" { if len(gopts.commandArgs) == 0 || isHelpArg(gopts.commandArgs[0]) { printMainHelp(os.Stdout) - return + return 0 } if printCommandHelp(os.Stdout, gopts.commandArgs[0]) { - return + return 0 } fmt.Fprintf(os.Stderr, "unknown help topic: %s\n", gopts.commandArgs[0]) - os.Exit(2) + return 2 } if gopts.command == "dev-help" { printDeveloperHelp(os.Stdout) - return + return 0 } if commandWantsHelp(gopts.commandArgs) { if printCommandHelp(os.Stdout, gopts.command) { - return + return 0 } fmt.Fprintf(os.Stderr, "unknown command: %s\n", gopts.command) - os.Exit(2) + return 2 } cfg, err := config.Load(gopts.configPath) @@ -63,7 +70,7 @@ func main() { if upErr != nil { fmt.Fprintf(os.Stderr, "config error: %v\n", err) fmt.Fprintf(os.Stderr, "config auto-upgrade failed: %v\n", upErr) - os.Exit(1) + return 1 } fmt.Fprintf(os.Stderr, "config upgraded at %s\n", resolvedPath) cfg, err = config.Load(gopts.configPath) @@ -71,7 +78,7 @@ func main() { } if err != nil { fmt.Fprintf(os.Stderr, "config error: %v\n", err) - os.Exit(1) + return 1 } applyGlobalConfigOverrides(cfg, gopts) verbose.SetLevel(gopts.verbose) @@ -87,19 +94,16 @@ func main() { defer stop() switch os.Args[1] { - case "dev-help": - printDeveloperHelp(os.Stdout) - return case "url": if len(os.Args) < 3 { printCommandHelp(os.Stdout, "url") - os.Exit(2) + return 2 } mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() @@ -123,22 +127,22 @@ func main() { if added == 0 { fmt.Println("nothing to rip") - return + return 0 } if err = mainApp.Resolve(ctx); err != nil { fmt.Fprintf(os.Stderr, "resolve error: %s\n", errorWithActionableHint(err, gopts)) - os.Exit(1) + return 1 } if err = mainApp.Rip(ctx); err != nil { fmt.Fprintf(os.Stderr, "rip error: %s\n", errorWithActionableHint(err, gopts)) - os.Exit(1) + return 1 } fmt.Printf("url rip complete (%d item(s))\n", added) case "file": if len(os.Args) < 3 { printCommandHelp(os.Stdout, "file") - os.Exit(2) + return 2 } ignoreDB := false @@ -148,26 +152,26 @@ func main() { ignoreDB = true default: fmt.Fprintf(os.Stderr, "option error: unknown option %q\n", arg) - os.Exit(2) + return 2 } } content, err := os.ReadFile(os.Args[2]) if err != nil { fmt.Fprintf(os.Stderr, "read file error: %v\n", err) - os.Exit(1) + return 1 } idItems, urls, repeated, jsonInput, err := parseFileInput(content) if err != nil { fmt.Fprintf(os.Stderr, "file parse error: %v\n", err) - os.Exit(2) + return 2 } mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() mainApp.IgnoreDB = ignoreDB || gopts.noDB @@ -196,22 +200,22 @@ func main() { if added == 0 { fmt.Println("nothing to rip") - return + return 0 } if err = mainApp.Resolve(ctx); err != nil { fmt.Fprintf(os.Stderr, "resolve error: %s\n", errorWithActionableHint(err, gopts)) - os.Exit(1) + return 1 } if err = mainApp.Rip(ctx); err != nil { fmt.Fprintf(os.Stderr, "rip error: %s\n", errorWithActionableHint(err, gopts)) - os.Exit(1) + return 1 } fmt.Printf("file rip complete (%d item(s))\n", added) case "config": if len(os.Args) < 3 { printCommandHelp(os.Stdout, "config") - os.Exit(2) + return 2 } switch os.Args[2] { case "open": @@ -222,13 +226,13 @@ func main() { vim = true default: fmt.Fprintf(os.Stderr, "option error: unknown option %q\n", arg) - os.Exit(2) + return 2 } } fmt.Printf("opening file at %s\n", cfg.Path) if err = openConfigInEditor(cfg.Path, vim); err != nil { fmt.Fprintf(os.Stderr, "open config error: %v\n", err) - os.Exit(1) + return 1 } case "reset": yes := false @@ -238,22 +242,22 @@ func main() { yes = true default: fmt.Fprintf(os.Stderr, "option error: unknown option %q\n", arg) - os.Exit(2) + return 2 } } if !yes { if !term.IsTerminal(int(os.Stdin.Fd())) { fmt.Fprintln(os.Stderr, "reset requires --yes in non-interactive mode") - os.Exit(2) + return 2 } ok, askErr := promptYesNo(fmt.Sprintf("Are you sure you want to reset the config file at %s? [y/N]: ", cfg.Path)) if askErr != nil { fmt.Fprintf(os.Stderr, "prompt error: %v\n", askErr) - os.Exit(1) + return 1 } if !ok { fmt.Println("reset aborted") - return + return 0 } } def := config.DefaultConfigData() @@ -261,23 +265,23 @@ func main() { cfg.Session = def if err = cfg.SaveFile(); err != nil { fmt.Fprintf(os.Stderr, "reset config error: %v\n", err) - os.Exit(1) + return 1 } fmt.Printf("reset the config file at %s\n", cfg.Path) case "path": if len(os.Args) > 3 { fmt.Fprintf(os.Stderr, "option error: unexpected argument %q\n", os.Args[3]) - os.Exit(2) + return 2 } fmt.Printf("config path: '%s'\n", cfg.Path) default: fmt.Fprintf(os.Stderr, "unknown config command: %s\n", os.Args[2]) - os.Exit(2) + return 2 } case "database": if len(os.Args) < 4 || os.Args[2] != "browse" { printCommandHelp(os.Stdout, "database") - os.Exit(2) + return 2 } table := strings.ToLower(strings.TrimSpace(os.Args[3])) switch table { @@ -285,7 +289,7 @@ func main() { rows, listErr := listDownloadsRows(cfg.Session.Database.DownloadsPath) if listErr != nil { fmt.Fprintf(os.Stderr, "database browse error: %v\n", listErr) - os.Exit(1) + return 1 } fmt.Println("downloads database") fmt.Println("row id") @@ -299,7 +303,7 @@ func main() { } if listErr != nil { fmt.Fprintf(os.Stderr, "database browse error: %v\n", listErr) - os.Exit(1) + return 1 } fmt.Println("failed downloads database") fmt.Println("row source media_type id") @@ -308,12 +312,12 @@ func main() { } default: fmt.Fprintf(os.Stderr, "invalid database %q. choose downloads or failed\n", table) - os.Exit(2) + return 2 } case "id": if len(os.Args) < 5 { printCommandHelp(os.Stdout, "id") - os.Exit(2) + return 2 } source := strings.ToLower(strings.TrimSpace(os.Args[2])) @@ -323,14 +327,14 @@ func main() { opts, err := parseSmokeOptions(os.Args[5:], 0, 4) if err != nil { fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) + return 2 } if opts.qualitySet { switch source { case "qobuz": if opts.quality < 1 { fmt.Fprintf(os.Stderr, "quality error: qobuz quality must be 1-4\n") - os.Exit(2) + return 2 } cfg.Session.Qobuz.Quality = opts.quality case "tidal": @@ -340,7 +344,7 @@ func main() { case "beatport": if opts.quality < 1 { fmt.Fprintf(os.Stderr, "quality error: beatport quality must be 1-4\n") - os.Exit(2) + return 2 } cfg.Session.Beatport.Quality = opts.quality } @@ -349,22 +353,22 @@ func main() { mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB if err = mainApp.AddByID(ctx, source, mediaType, itemID); err != nil { fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) + return 1 } if err = mainApp.Resolve(ctx); err != nil { fmt.Fprintf(os.Stderr, "resolve error: %s\n", errorWithActionableHint(err, gopts)) - os.Exit(1) + return 1 } if err = mainApp.Rip(ctx); err != nil { fmt.Fprintf(os.Stderr, "rip error: %s\n", errorWithActionableHint(err, gopts)) - os.Exit(1) + return 1 } fmt.Printf("id rip complete: source=%s type=%s id=%s\n", source, mediaType, itemID) case "search": @@ -373,12 +377,12 @@ func main() { if len(os.Args) < 5 { if !term.IsTerminal(int(os.Stdin.Fd())) { printCommandHelp(os.Stdout, "search") - os.Exit(2) + return 2 } source, mediaType, sopts, err = promptSearchInteractive(cfg.Session.CLI.MaxSearchResults) if err != nil { fmt.Fprintf(os.Stderr, "search prompt error: %v\n", err) - os.Exit(2) + return 2 } } else { source = strings.ToLower(strings.TrimSpace(os.Args[2])) @@ -388,60 +392,60 @@ func main() { if err != nil { fmt.Fprintf(os.Stderr, "search option error: %v\n", err) - os.Exit(2) + return 2 } if sopts.first && sopts.outputFile != "" { fmt.Fprintln(os.Stderr, "cannot choose --first and --output-file together") - os.Exit(2) + return 2 } if !isAllowedSearchSource(source) { fmt.Fprintf(os.Stderr, "unsupported search source %q\n", source) - os.Exit(2) + return 2 } if !isAllowedMediaType(mediaType) { fmt.Fprintf(os.Stderr, "unsupported media type %q\n", mediaType) - os.Exit(2) + return 2 } if source == "soundcloud" && mediaType != "track" && mediaType != "playlist" { fmt.Fprintln(os.Stderr, "soundcloud search currently supports media types track and playlist") - os.Exit(2) + return 2 } if source == "yandex" && mediaType != "track" && mediaType != "album" && mediaType != "playlist" && mediaType != "artist" { fmt.Fprintln(os.Stderr, "yandex search currently supports media types track, album, playlist, and artist") - os.Exit(2) + return 2 } if source == "beatport" && mediaType != "track" && mediaType != "album" && mediaType != "label" { fmt.Fprintln(os.Stderr, "beatport search currently supports media types track, album, and label") - os.Exit(2) + return 2 } if sopts.query == "" { fmt.Fprintln(os.Stderr, "search query cannot be empty") - os.Exit(2) + return 2 } mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() provider, err := mainApp.GetLoggedInProvider(ctx, source) if err != nil { fmt.Fprintf(os.Stderr, "%s login error: %v\n", source, err) - os.Exit(1) + return 1 } pages, err := provider.Search(ctx, mediaType, sopts.query, sopts.limit) if err != nil { fmt.Fprintf(os.Stderr, "search error: %v\n", err) - os.Exit(1) + return 1 } results := normalizeSearchResults(source, mediaType, pages) if len(results) == 0 { fmt.Println("no results") - return + return 0 } if sopts.outputFile != "" { fmt.Printf("results: %d\n", len(results)) @@ -450,10 +454,10 @@ func main() { } if err = writeSearchResultsToFile(source, mediaType, results, sopts.outputFile); err != nil { fmt.Fprintf(os.Stderr, "write results error: %v\n", err) - os.Exit(1) + return 1 } fmt.Printf("wrote %d results to %s\n", len(results), sopts.outputFile) - return + return 0 } if sopts.first { fmt.Printf("results: %d\n", len(results)) @@ -465,7 +469,7 @@ func main() { for i, result := range results { fmt.Printf("%2d. id=%s | %s\n", i+1, result.ID, result.Title) } - return + return 0 } if !sopts.first { fmt.Printf("results: %d\n", len(results)) @@ -498,33 +502,33 @@ func main() { } else { fmt.Println("nothing selected to download") } - return + return 0 } if err = mainApp.Resolve(ctx); err != nil { fmt.Fprintf(os.Stderr, "resolve error: %s\n", errorWithActionableHint(err, gopts)) - os.Exit(1) + return 1 } if err = mainApp.Rip(ctx); err != nil { fmt.Fprintf(os.Stderr, "rip error: %s\n", errorWithActionableHint(err, gopts)) - os.Exit(1) + return 1 } fmt.Printf("search download complete (%d item(s))\n", added) - return + return 0 } if !term.IsTerminal(int(os.Stdin.Fd())) { fmt.Println("non-interactive input; use `rip id` to download specific results") - return + return 0 } selection, err := promptSearchSelectionMenu(source, mediaType, sopts.query, results) if err != nil { fmt.Fprintf(os.Stderr, "selection error: %v\n", err) - os.Exit(2) + return 2 } if len(selection) == 0 { fmt.Println("download cancelled") - return + return 0 } mainApp.IgnoreDB = sopts.ignoreDB || gopts.noDB @@ -552,15 +556,15 @@ func main() { } else { fmt.Println("nothing selected to download") } - return + return 0 } if err = mainApp.Resolve(ctx); err != nil { fmt.Fprintf(os.Stderr, "resolve error: %s\n", errorWithActionableHint(err, gopts)) - os.Exit(1) + return 1 } if err = mainApp.Rip(ctx); err != nil { fmt.Fprintf(os.Stderr, "rip error: %s\n", errorWithActionableHint(err, gopts)) - os.Exit(1) + return 1 } fmt.Printf("search download complete (%d item(s))\n", added) case "lastfm": @@ -568,13 +572,13 @@ func main() { if parseErr != nil { fmt.Fprintf(os.Stderr, "lastfm option error: %v\n", parseErr) printCommandHelp(os.Stdout, "lastfm") - os.Exit(2) + return 2 } mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() mainApp.IgnoreDB = gopts.noDB @@ -582,22 +586,22 @@ func main() { title, tracks, err := fetchLastFMPlaylist(ctx, cfg.Session.Downloads.VerifySSL, opts.PlaylistURL) if err != nil { fmt.Fprintf(os.Stderr, "lastfm parse error: %v\n", err) - os.Exit(1) + return 1 } if len(tracks) == 0 { fmt.Println("no tracks found in playlist") - return + return 0 } fmt.Printf("lastfm playlist: %s (%d tracks)\n", title, len(tracks)) resolvedTracks, err := resolveLastFMTracks(ctx, mainApp, opts, tracks) if err != nil { fmt.Fprintf(os.Stderr, "lastfm resolve error: %v\n", err) - os.Exit(1) + return 1 } if len(resolvedTracks) == 0 { fmt.Println("no lastfm tracks resolved") - return + return 0 } playlistID := fmt.Sprintf("lastfm:%s", strings.ToLower(strings.ReplaceAll(title, " ", "_"))) @@ -608,42 +612,42 @@ func main() { if addErr := mainApp.AddMixedPlaylistByTrackRefs(ctx, playlistID, title, refs); addErr != nil { fmt.Printf("playlist queue failed: err=%v\n", addErr) fmt.Println("no lastfm playlists queued") - return + return 0 } fmt.Printf("queued lastfm playlist: %s (%d tracks)\n", title, len(refs)) if len(refs) == 0 { fmt.Println("no lastfm playlists queued") - return + return 0 } if err = mainApp.Resolve(ctx); err != nil { fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) + return 1 } if err = mainApp.Rip(ctx); err != nil { fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) + return 1 } fmt.Printf("lastfm rip complete (%d track(s) across 1 playlist)\n", len(resolvedTracks)) case "soundcloud-smoke": if len(os.Args) < 3 { fmt.Println("usage: rip soundcloud-smoke ") - os.Exit(2) + return 2 } meta, err := fetchSoundcloudOEmbed(ctx, cfg.Session.Downloads.VerifySSL, strings.TrimSpace(os.Args[2])) if err != nil { fmt.Fprintf(os.Stderr, "soundcloud smoke error: %v\n", err) - os.Exit(1) + return 1 } fmt.Printf("soundcloud oembed ok: title=%q author=%q provider=%q\n", asString(meta["title"]), asString(meta["author_name"]), asString(meta["provider_name"])) case "qobuz-smoke": if len(os.Args) < 3 { fmt.Println("usage: rip qobuz-smoke [quality]") - os.Exit(2) + return 2 } opts, err := parseSmokeOptions(os.Args[3:], 1, 4) if err != nil { fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) + return 2 } if opts.qualitySet { cfg.Session.Qobuz.Quality = opts.quality @@ -652,78 +656,41 @@ func main() { mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() provider, err := mainApp.GetLoggedInProvider(ctx, "qobuz") if err != nil { fmt.Fprintf(os.Stderr, "qobuz login error: %v\n", err) - os.Exit(1) + return 1 } trackID := os.Args[2] meta, err := provider.GetMetadata(ctx, trackID, "track") if err != nil { fmt.Fprintf(os.Stderr, "metadata error: %v\n", err) - os.Exit(1) + return 1 } title, _ := meta["title"].(string) d, err := provider.GetDownloadable(ctx, trackID, cfg.Session.Qobuz.Quality) if err != nil { fmt.Fprintf(os.Stderr, "downloadable error: %v\n", err) - os.Exit(1) + return 1 } fmt.Printf("qobuz ok: title=%q quality=%d ext=%s\n", title, cfg.Session.Qobuz.Quality, d.Extension) fmt.Printf("stream_url=%s\n", d.URL) - case "qobuz-rip-smoke": - if len(os.Args) < 3 { - fmt.Println("usage: rip qobuz-rip-smoke [quality] [--force|--ignore-db]") - os.Exit(2) - } - opts, err := parseSmokeOptions(os.Args[3:], 1, 4) - if err != nil { - fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) - } - if opts.qualitySet { - cfg.Session.Qobuz.Quality = opts.quality - } - - mainApp, err := app.New(cfg) - if err != nil { - fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) - } - defer func() { _ = mainApp.Close() }() - mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB - - trackID := os.Args[2] - if err = mainApp.AddByID(ctx, "qobuz", "track", trackID); err != nil { - fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Resolve(ctx); err != nil { - fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Rip(ctx); err != nil { - fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) - } - - fmt.Println("qobuz rip smoke complete") case "qobuz-convert-rip-smoke": if len(os.Args) < 4 { fmt.Println("usage: rip qobuz-convert-rip-smoke [quality] [--force|--ignore-db]") - os.Exit(2) + return 2 } opts, err := parseSmokeOptions(os.Args[4:], 1, 4) if err != nil { fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) + return 2 } if opts.qualitySet { cfg.Session.Qobuz.Quality = opts.quality @@ -734,7 +701,7 @@ func main() { mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB @@ -742,182 +709,41 @@ func main() { trackID := os.Args[2] if err = mainApp.AddByID(ctx, "qobuz", "track", trackID); err != nil { fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) + return 1 } if err = mainApp.Resolve(ctx); err != nil { fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) + return 1 } if err = mainApp.Rip(ctx); err != nil { fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) + return 1 } fmt.Printf("qobuz convert rip smoke complete (codec=%s)\n", cfg.Session.Conversion.Codec) - case "qobuz-album-rip-smoke": - if len(os.Args) < 3 { - fmt.Println("usage: rip qobuz-album-rip-smoke [quality] [--force|--ignore-db]") - os.Exit(2) - } - opts, err := parseSmokeOptions(os.Args[3:], 1, 4) - if err != nil { - fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) - } - if opts.qualitySet { - cfg.Session.Qobuz.Quality = opts.quality - } - - mainApp, err := app.New(cfg) - if err != nil { - fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) - } - defer func() { _ = mainApp.Close() }() - mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB - - albumID := os.Args[2] - if err = mainApp.AddByID(ctx, "qobuz", "album", albumID); err != nil { - fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Resolve(ctx); err != nil { - fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Rip(ctx); err != nil { - fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) - } - - fmt.Println("qobuz album rip smoke complete") - case "qobuz-playlist-rip-smoke": - if len(os.Args) < 3 { - fmt.Println("usage: rip qobuz-playlist-rip-smoke [quality] [--force|--ignore-db]") - os.Exit(2) - } - opts, err := parseSmokeOptions(os.Args[3:], 1, 4) - if err != nil { - fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) - } - if opts.qualitySet { - cfg.Session.Qobuz.Quality = opts.quality - } - - mainApp, err := app.New(cfg) - if err != nil { - fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) - } - defer func() { _ = mainApp.Close() }() - mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB - - playlistID := os.Args[2] - if err = mainApp.AddByID(ctx, "qobuz", "playlist", playlistID); err != nil { - fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Resolve(ctx); err != nil { - fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Rip(ctx); err != nil { - fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) - } - fmt.Println("qobuz playlist rip smoke complete") - case "qobuz-artist-rip-smoke": - if len(os.Args) < 3 { - fmt.Println("usage: rip qobuz-artist-rip-smoke [quality] [--force|--ignore-db]") - os.Exit(2) - } - opts, err := parseSmokeOptions(os.Args[3:], 1, 4) - if err != nil { - fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) - } - if opts.qualitySet { - cfg.Session.Qobuz.Quality = opts.quality - } - mainApp, err := app.New(cfg) - if err != nil { - fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) - } - defer func() { _ = mainApp.Close() }() - mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB - artistID := os.Args[2] - if err = mainApp.AddByID(ctx, "qobuz", "artist", artistID); err != nil { - fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Resolve(ctx); err != nil { - fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Rip(ctx); err != nil { - fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) - } - fmt.Println("qobuz artist rip smoke complete") - case "qobuz-label-rip-smoke": - if len(os.Args) < 3 { - fmt.Println("usage: rip qobuz-label-rip-smoke [quality] [--force|--ignore-db]") - os.Exit(2) - } - opts, err := parseSmokeOptions(os.Args[3:], 1, 4) - if err != nil { - fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) - } - if opts.qualitySet { - cfg.Session.Qobuz.Quality = opts.quality - } - mainApp, err := app.New(cfg) - if err != nil { - fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) - } - defer func() { _ = mainApp.Close() }() - mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB - labelID := os.Args[2] - if err = mainApp.AddByID(ctx, "qobuz", "label", labelID); err != nil { - fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Resolve(ctx); err != nil { - fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Rip(ctx); err != nil { - fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) - } - fmt.Println("qobuz label rip smoke complete") case "qobuz-search-smoke": if len(os.Args) < 3 { fmt.Println("usage: rip qobuz-search-smoke ") - os.Exit(2) + return 2 } mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() provider, err := mainApp.GetLoggedInProvider(ctx, "qobuz") if err != nil { fmt.Fprintf(os.Stderr, "qobuz login error: %v\n", err) - os.Exit(1) + return 1 } query := strings.Join(os.Args[2:], " ") pages, err := provider.Search(ctx, "album", query, 10) if err != nil { fmt.Fprintf(os.Stderr, "search error: %v\n", err) - os.Exit(1) + return 1 } for _, page := range pages { albums, ok := page["albums"].(map[string]any) @@ -947,27 +773,27 @@ func main() { case "tidal-search-smoke": if len(os.Args) < 3 { fmt.Println("usage: rip tidal-search-smoke ") - os.Exit(2) + return 2 } mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() provider, err := mainApp.GetLoggedInProvider(ctx, "tidal") if err != nil { fmt.Fprintf(os.Stderr, "tidal login error: %v\n", err) - os.Exit(1) + return 1 } query := strings.Join(os.Args[2:], " ") pages, err := provider.Search(ctx, "album", query, 10) if err != nil { fmt.Fprintf(os.Stderr, "search error: %v\n", err) - os.Exit(1) + return 1 } for _, page := range pages { items, ok := page["items"].([]any) @@ -989,20 +815,20 @@ func main() { case "tidal-metadata-smoke": if len(os.Args) < 4 { fmt.Println("usage: rip tidal-metadata-smoke ") - os.Exit(2) + return 2 } mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() provider, err := mainApp.GetLoggedInProvider(ctx, "tidal") if err != nil { fmt.Fprintf(os.Stderr, "tidal login error: %v\n", err) - os.Exit(1) + return 1 } mediaType := os.Args[2] @@ -1010,7 +836,7 @@ func main() { meta, err := provider.GetMetadata(ctx, itemID, mediaType) if err != nil { fmt.Fprintf(os.Stderr, "metadata error: %v\n", err) - os.Exit(1) + return 1 } title := asString(meta["title"]) if title == "" { @@ -1028,39 +854,39 @@ func main() { case "tidal-video-smoke": if len(os.Args) < 3 { fmt.Println("usage: rip tidal-video-smoke ") - os.Exit(2) + return 2 } mainApp, err := app.New(cfg) if err != nil { fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) + return 1 } defer func() { _ = mainApp.Close() }() providerClient, err := mainApp.GetLoggedInProvider(ctx, "tidal") if err != nil { fmt.Fprintf(os.Stderr, "tidal login error: %v\n", err) - os.Exit(1) + return 1 } videoProvider, ok := providerClient.(interface { GetVideoDownloadable(context.Context, string) (*provider.Downloadable, error) }) if !ok { fmt.Fprintln(os.Stderr, "tidal provider does not support video downloadable") - os.Exit(1) + return 1 } videoID := strings.TrimSpace(os.Args[2]) meta, err := providerClient.GetMetadata(ctx, videoID, "video") if err != nil { fmt.Fprintf(os.Stderr, "video metadata error: %v\n", err) - os.Exit(1) + return 1 } d, err := videoProvider.GetVideoDownloadable(ctx, videoID) if err != nil { fmt.Fprintf(os.Stderr, "video downloadable error: %v\n", err) - os.Exit(1) + return 1 } title := asString(meta["title"]) if title == "" { @@ -1068,150 +894,10 @@ func main() { } fmt.Printf("tidal video ok: id=%s title=%q ext=%s\n", videoID, title, d.Extension) fmt.Printf("stream_url=%s\n", d.URL) - case "tidal-rip-smoke": - if len(os.Args) < 3 { - fmt.Println("usage: rip tidal-rip-smoke [quality] [--force|--ignore-db]") - os.Exit(2) - } - opts, err := parseSmokeOptions(os.Args[3:], 0, 4) - if err != nil { - fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) - } - if opts.qualitySet { - cfg.Session.Tidal.Quality = opts.quality - } - - mainApp, err := app.New(cfg) - if err != nil { - fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) - } - defer func() { _ = mainApp.Close() }() - mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB - - trackID := os.Args[2] - if err = mainApp.AddByID(ctx, "tidal", "track", trackID); err != nil { - fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Resolve(ctx); err != nil { - fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Rip(ctx); err != nil { - fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) - } - fmt.Println("tidal rip smoke complete") - case "tidal-album-rip-smoke": - if len(os.Args) < 3 { - fmt.Println("usage: rip tidal-album-rip-smoke [quality] [--force|--ignore-db]") - os.Exit(2) - } - opts, err := parseSmokeOptions(os.Args[3:], 0, 4) - if err != nil { - fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) - } - if opts.qualitySet { - cfg.Session.Tidal.Quality = opts.quality - } - - mainApp, err := app.New(cfg) - if err != nil { - fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) - } - defer func() { _ = mainApp.Close() }() - mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB - - albumID := os.Args[2] - if err = mainApp.AddByID(ctx, "tidal", "album", albumID); err != nil { - fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Resolve(ctx); err != nil { - fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Rip(ctx); err != nil { - fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) - } - fmt.Println("tidal album rip smoke complete") - case "tidal-playlist-rip-smoke": - if len(os.Args) < 3 { - fmt.Println("usage: rip tidal-playlist-rip-smoke [quality] [--force|--ignore-db]") - os.Exit(2) - } - opts, err := parseSmokeOptions(os.Args[3:], 0, 4) - if err != nil { - fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) - } - if opts.qualitySet { - cfg.Session.Tidal.Quality = opts.quality - } - - mainApp, err := app.New(cfg) - if err != nil { - fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) - } - defer func() { _ = mainApp.Close() }() - mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB - - playlistID := os.Args[2] - if err = mainApp.AddByID(ctx, "tidal", "playlist", playlistID); err != nil { - fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Resolve(ctx); err != nil { - fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Rip(ctx); err != nil { - fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) - } - fmt.Println("tidal playlist rip smoke complete") - case "tidal-artist-rip-smoke": - if len(os.Args) < 3 { - fmt.Println("usage: rip tidal-artist-rip-smoke [quality] [--force|--ignore-db]") - os.Exit(2) - } - opts, err := parseSmokeOptions(os.Args[3:], 0, 4) - if err != nil { - fmt.Fprintf(os.Stderr, "option error: %v\n", err) - os.Exit(2) - } - if opts.qualitySet { - cfg.Session.Tidal.Quality = opts.quality - } - mainApp, err := app.New(cfg) - if err != nil { - fmt.Fprintf(os.Stderr, "app init error: %v\n", err) - os.Exit(1) - } - defer func() { _ = mainApp.Close() }() - mainApp.IgnoreDB = opts.ignoreDB || gopts.noDB - artistID := os.Args[2] - if err = mainApp.AddByID(ctx, "tidal", "artist", artistID); err != nil { - fmt.Fprintf(os.Stderr, "add error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Resolve(ctx); err != nil { - fmt.Fprintf(os.Stderr, "resolve error: %v\n", err) - os.Exit(1) - } - if err = mainApp.Rip(ctx); err != nil { - fmt.Fprintf(os.Stderr, "rip error: %v\n", err) - os.Exit(1) - } - fmt.Println("tidal artist rip smoke complete") default: fmt.Fprintf(os.Stderr, "unknown command: %s\n", os.Args[1]) - os.Exit(2) + return 2 } + + return 0 } diff --git a/internal/app/app.go b/internal/app/app.go index 92a45eb..03a2c37 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -11,6 +11,7 @@ import ( "strconv" "strings" "sync" + "time" "streamrip-go/internal/artwork" "streamrip-go/internal/audio/convert" @@ -87,7 +88,7 @@ var ( ) type trackTagger interface { - TagFLAC(path string, meta tag.Metadata, coverPath string) error + TagFLAC(ctx context.Context, path string, meta tag.Metadata, coverPath string) error } type videoDownloadableProvider interface { @@ -161,6 +162,79 @@ func (m *Main) logf(format string, args ...any) { fmt.Printf(format, args...) } +// bookkeepingTimeout bounds the detached store writes below. They must not +// inherit a cancelled context, or a single Ctrl-C silently loses the record of +// which tracks failed. +const bookkeepingTimeout = 5 * time.Second + +// markFailed records a failed download. It deliberately detaches from ctx so +// the write still lands when the rip was cancelled, which is exactly when the +// failure record matters most. +func (m *Main) markFailed(ctx context.Context, source, mediaType, id string) { + if m.Store == nil { + return + } + writeCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), bookkeepingTimeout) + defer cancel() + if err := m.Store.MarkFailed(writeCtx, source, mediaType, id); err != nil { + m.logf("warning: could not record failure for %s/%s/%s: %v\n", source, mediaType, id, err) + } +} + +// markDownloaded records a completed download, detached from ctx for the same +// reason as markFailed: the file is already on disk, so losing the record would +// cause a needless re-download later. +func (m *Main) markDownloaded(ctx context.Context, source, id string) error { + if m.Store == nil { + return nil + } + writeCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), bookkeepingTimeout) + defer cancel() + return m.Store.MarkDownloaded(writeCtx, source, id) +} + +// safeJoin joins base with parts and asserts the result stays inside the +// configured downloads folder. +// +// naming.CleanName already rejects "." and ".." components, so this should +// never fire; it is a second line of defence because every component here is +// built from provider-supplied metadata, and writing outside the download root +// is the one failure mode worth being paranoid about. +func (m *Main) safeJoin(base string, parts ...string) string { + joined := filepath.Join(append([]string{base}, parts...)...) + + root := strings.TrimSpace(m.Config.Session.Downloads.Folder) + if root == "" { + return joined + } + if pathWithin(root, joined) { + return joined + } + + m.logf("warning: refusing to write outside the download folder (%q); using %q instead\n", joined, root) + return filepath.Join(root, "Unknown") +} + +// pathWithin reports whether target is root itself or sits beneath it. +func pathWithin(root, target string) bool { + absRoot, err := filepath.Abs(root) + if err != nil { + return false + } + absTarget, err := filepath.Abs(target) + if err != nil { + return false + } + rel, err := filepath.Rel(absRoot, absTarget) + if err != nil { + return false + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return false + } + return true +} + func (m *Main) GetLoggedInProvider(ctx context.Context, source string) (provider.Client, error) { p, ok := m.Providers[source] if !ok { @@ -184,7 +258,7 @@ func (m *Main) AddByID(ctx context.Context, source, mediaType, id string) error ResolveFn: func(ctx context.Context) (media.Media, error) { meta, err := p.GetMetadata(ctx, id, mediaType) if err != nil { - _ = m.Store.MarkFailed(ctx, source, mediaType, id) + m.markFailed(ctx, source, mediaType, id) return nil, err } @@ -346,7 +420,7 @@ func (m *Main) ripTrackCollection(ctx context.Context, p provider.Client, source if m.Config.Session.Downloads.SourceSubdirectories { base = filepath.Join(base, jsonutil.TitleCase(source)) } - folder := filepath.Join(base, naming.CleanName(name, naming.Config{RestrictCharacters: m.Config.Session.Filepaths.RestrictCharacters, TruncateTo: m.Config.Session.Filepaths.TruncateTo})) + folder := m.safeJoin(base, naming.CleanName(name, naming.Config{RestrictCharacters: m.Config.Session.Filepaths.RestrictCharacters, TruncateTo: m.Config.Session.Filepaths.TruncateTo})) tracksMap, ok := meta["tracks"].(map[string]any) if !ok { return fmt.Errorf("%s missing tracks data", strings.ToLower(kind)) @@ -388,6 +462,9 @@ func (m *Main) ripTrackCollection(ctx context.Context, p provider.Client, source } if !m.Config.Session.Downloads.Concurrency || m.Config.Session.Downloads.MaxConnections == 1 { for i, trackID := range ids { + if ctx.Err() != nil { + break + } runOne(i+1, trackID) } } else { @@ -399,6 +476,9 @@ func (m *Main) ripTrackCollection(ctx context.Context, p provider.Client, source var wg sync.WaitGroup var mu sync.Mutex for i, trackID := range ids { + if ctx.Err() != nil { + break + } wg.Add(1) sem <- struct{}{} go func(pos int, tid string) { @@ -440,18 +520,18 @@ func (m *Main) ripVideo(ctx context.Context, p provider.Client, source, videoID d, err := vp.GetVideoDownloadable(ctx, videoID) if err != nil { - _ = m.Store.MarkFailed(ctx, source, "video", videoID) + m.markFailed(ctx, source, "video", videoID) return fmt.Errorf("id=%s get_video_downloadable: %w", videoID, err) } title := titleFromMetadata(meta, videoID) outPath := m.videoOutputPath(source, videoID, title, d.Extension) if err = m.DL.FileVideo(ctx, d.URL, outPath); err != nil { - _ = m.Store.MarkFailed(ctx, source, "video", videoID) + m.markFailed(ctx, source, "video", videoID) return fmt.Errorf("id=%s title=%q video download: %w", videoID, title, err) } - if err = m.Store.MarkDownloaded(ctx, source, videoID); err != nil { + if err = m.markDownloaded(ctx, source, videoID); err != nil { return err } return nil @@ -631,11 +711,17 @@ func (m *Main) Rip(ctx context.Context) error { m.logf("Ripping %d media item(s)\n", len(m.Media)) failures := 0 for _, item := range m.Media { + if err := ctx.Err(); err != nil { + return err + } if err := item.Rip(ctx); err != nil { failures++ m.logf("media item failed: %v\n", err) } } + if err := ctx.Err(); err != nil { + return err + } if failures > 0 { return fmt.Errorf("%d media item(s) failed", failures) } @@ -708,6 +794,9 @@ func (m *Main) ripAlbum(ctx context.Context, p provider.Client, source, albumID if !m.Config.Session.Downloads.Concurrency || m.Config.Session.Downloads.MaxConnections == 1 { for i, trackID := range trackIDs { + if ctx.Err() != nil { + break + } opts := ripTrackOptions{albumFolder: folder, albumEmbedCover: artRes.EmbedPath, albumArtist: albumArtist, index: i + 1, total: total, albumDiscTotal: discTotal} if i == 0 { opts.prefetched = prefetched @@ -731,6 +820,9 @@ func (m *Main) ripAlbum(ctx context.Context, p provider.Client, source, albumID var wg sync.WaitGroup errCh := make(chan error, len(trackIDs)) for i, trackID := range trackIDs { + if ctx.Err() != nil { + break + } wg.Add(1) sem <- struct{}{} go func(idx int, tid string) { @@ -770,7 +862,7 @@ func (m *Main) ripPlaylist(ctx context.Context, p provider.Client, source, playl if m.Config.Session.Downloads.SourceSubdirectories { base = filepath.Join(base, jsonutil.TitleCase(source)) } - folder := filepath.Join(base, naming.CleanName(name, naming.Config{ + folder := m.safeJoin(base, naming.CleanName(name, naming.Config{ RestrictCharacters: m.Config.Session.Filepaths.RestrictCharacters, TruncateTo: m.Config.Session.Filepaths.TruncateTo, })) @@ -827,6 +919,9 @@ func (m *Main) ripPlaylist(ctx context.Context, p provider.Client, source, playl if !m.Config.Session.Downloads.Concurrency || m.Config.Session.Downloads.MaxConnections == 1 { for i, id := range ids { + if ctx.Err() != nil { + break + } runOne(i+1, id) } } else { @@ -838,6 +933,9 @@ func (m *Main) ripPlaylist(ctx context.Context, p provider.Client, source, playl var wg sync.WaitGroup var mu sync.Mutex for i, id := range ids { + if ctx.Err() != nil { + break + } wg.Add(1) sem <- struct{}{} go func(pos int, tid string) { @@ -876,7 +974,7 @@ func (m *Main) ripPlaylistMixed(ctx context.Context, playlistID, name string, re } } - folder := filepath.Join(m.Config.Session.Downloads.Folder, naming.CleanName(name, naming.Config{ + folder := m.safeJoin(m.Config.Session.Downloads.Folder, naming.CleanName(name, naming.Config{ RestrictCharacters: m.Config.Session.Filepaths.RestrictCharacters, TruncateTo: m.Config.Session.Filepaths.TruncateTo, })) @@ -938,6 +1036,9 @@ func (m *Main) requireSourceDownloadAuth(source string) error { } func (m *Main) ripTrack(ctx context.Context, p provider.Client, source, id, fallbackTitle string, opts ripTrackOptions) error { + if err := ctx.Err(); err != nil { + return err + } if !m.IgnoreDB { alreadyDownloaded, err := m.Store.IsDownloaded(ctx, source, id) if err == nil && alreadyDownloaded { @@ -952,7 +1053,7 @@ func (m *Main) ripTrack(ctx context.Context, p provider.Client, source, id, fall meta, err := p.GetMetadata(ctx, id, "track") if err != nil { - _ = m.Store.MarkFailed(ctx, source, "track", id) + m.markFailed(ctx, source, "track", id) return fmt.Errorf("id=%s metadata: %w", id, err) } @@ -969,7 +1070,7 @@ func (m *Main) ripTrack(ctx context.Context, p provider.Client, source, id, fall if d == nil { d, err = p.GetDownloadable(ctx, id, m.qualityForSource(source)) if err != nil { - _ = m.Store.MarkFailed(ctx, source, "track", id) + m.markFailed(ctx, source, "track", id) return fmt.Errorf("id=%s title=%q get_downloadable: %w", id, title, err) } } @@ -992,9 +1093,15 @@ func (m *Main) ripTrack(ctx context.Context, p provider.Client, source, id, fall return m.DL.File(ctx, d.URL, outPath) } if err = downloadOnce(); err != nil { + // Never spend retries or a provider fallback on a cancelled context: + // otherwise one Ctrl-C costs three doomed attempts per remaining track. + if ctxErr := ctx.Err(); ctxErr != nil { + m.markFailed(ctx, source, "track", id) + return fmt.Errorf("id=%s title=%q download: %w", id, title, err) + } m.logf("retry: %s (%v)\n", filepath.Base(outPath), err) if err = downloadOnce(); err != nil { - if fallbackProvider, ok := p.(trackFallbackDownloader); ok { + if fallbackProvider, ok := p.(trackFallbackDownloader); ok && ctx.Err() == nil { m.logf("fallback: %s via provider backup flow\n", filepath.Base(outPath)) if fbErr := fallbackProvider.DownloadTrackFallback(ctx, id, m.qualityForSource(source), outPath); fbErr == nil { goto downloaded @@ -1002,7 +1109,7 @@ func (m *Main) ripTrack(ctx context.Context, p provider.Client, source, id, fall m.logf("fallback failed: %s (%v)\n", filepath.Base(outPath), fbErr) } } - _ = m.Store.MarkFailed(ctx, source, "track", id) + m.markFailed(ctx, source, "track", id) return fmt.Errorf("id=%s title=%q download: %w", id, title, err) } } @@ -1040,24 +1147,24 @@ downloaded: coverPath = tag.CoverPathForTrack(outPath, opts.albumFolder) } } - if err = m.Tagger.TagFLAC(outPath, tagMeta, coverPath); err != nil { + if err = m.Tagger.TagFLAC(ctx, outPath, tagMeta, coverPath); err != nil { if isFFmpegMissingError(err) { - _ = m.Store.MarkFailed(ctx, source, "track", id) + m.markFailed(ctx, source, "track", id) return fmt.Errorf("id=%s title=%q tag: %w", id, title, err) } m.logf("warning: tag failed for %s: %v\n", filepath.Base(outPath), err) } if m.Config.Session.Conversion.Enabled { - convertedPath, convErr := convert.Convert(outPath, m.Config.Session.Conversion) + convertedPath, convErr := convert.Convert(ctx, outPath, m.Config.Session.Conversion) if convErr != nil { - _ = m.Store.MarkFailed(ctx, source, "track", id) + m.markFailed(ctx, source, "track", id) return fmt.Errorf("id=%s title=%q convert: %w", id, title, convErr) } outPath = convertedPath } - return m.Store.MarkDownloaded(ctx, source, id) + return m.markDownloaded(ctx, source, id) } func (m *Main) qualityForSource(source string) int { @@ -1206,7 +1313,7 @@ func (m *Main) albumFolderPath(source, albumID, albumTitle, albumArtist, year st TruncateTo: m.Config.Session.Filepaths.TruncateTo, }) - return filepath.Join(base, folderName) + return m.safeJoin(base, folderName) } func (m *Main) trackOutputPath(source, id, title, ext string, d *provider.Downloadable, trackMeta map[string]any, albumFolder string, albumDiscTotal int) string { @@ -1279,7 +1386,7 @@ func (m *Main) trackOutputPath(source, id, title, ext string, d *provider.Downlo RestrictCharacters: m.Config.Session.Filepaths.RestrictCharacters, TruncateTo: m.Config.Session.Filepaths.TruncateTo, }) - return filepath.Join(base, fileName+"."+ext) + return m.safeJoin(base, fileName+"."+ext) } func (m *Main) videoOutputPath(source, id, title, ext string) string { @@ -1297,7 +1404,7 @@ func (m *Main) videoOutputPath(source, id, title, ext string) string { if fileName == "" { fileName = id } - return filepath.Join(base, fileName+"."+ext) + return m.safeJoin(base, fileName+"."+ext) } func titleFromMetadata(meta map[string]any, fallback string) string { diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 364dc72..7868664 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -20,13 +20,13 @@ import ( type noopTagger struct{} -func (n noopTagger) TagFLAC(string, tag.Metadata, string) error { return nil } +func (n noopTagger) TagFLAC(context.Context, string, tag.Metadata, string) error { return nil } type failingTagger struct { err error } -func (f failingTagger) TagFLAC(string, tag.Metadata, string) error { return f.err } +func (f failingTagger) TagFLAC(context.Context, string, tag.Metadata, string) error { return f.err } type fakeProvider struct { url string diff --git a/internal/artwork/artwork.go b/internal/artwork/artwork.go index 47683cc..496f32e 100644 --- a/internal/artwork/artwork.go +++ b/internal/artwork/artwork.go @@ -6,10 +6,12 @@ import ( "fmt" "image" "image/jpeg" + _ "image/png" "os" "path/filepath" "strings" "sync" + "sync/atomic" "golang.org/x/image/draw" @@ -31,6 +33,52 @@ var ( tempDirs = map[string]struct{}{} ) +var ( + folderLocksMu sync.Mutex + folderLocks = map[string]*sync.Mutex{} +) + +// lockFolder serializes Prepare calls that target the same album folder and +// returns the unlock function. +// +// ripTrack calls Prepare once per track with the same albumFolder for every +// worker in a playlist rip, and the cover handling below is check-then-write. +// Without this, two workers both saw a missing cover.jpg, both created it, and +// the one that failed removed the file the other had just finished writing. +func lockFolder(folder string) func() { + folderLocksMu.Lock() + mu, ok := folderLocks[folder] + if !ok { + mu = &sync.Mutex{} + folderLocks[folder] = mu + } + folderLocksMu.Unlock() + + mu.Lock() + return mu.Unlock +} + +// fetchAtomic downloads sourceURL to destPath via a unique temp file in the +// same directory, then renames it into place. Downloading straight to destPath +// meant a failed attempt could delete a file another worker had completed. +func fetchAtomic(ctx context.Context, dl Downloader, sourceURL, destPath string, maxWidth int) bool { + tmpPath := fmt.Sprintf("%s.part-%d-%d", destPath, os.Getpid(), atomic.AddUint64(&tmpCounter, 1)) + if err := dl.FileNoProgress(ctx, sourceURL, tmpPath); err != nil { + _ = os.Remove(tmpPath) + return false + } + if maxWidth > 0 { + _ = downscaleImage(tmpPath, maxWidth) + } + if err := os.Rename(tmpPath, destPath); err != nil { + _ = os.Remove(tmpPath) + return false + } + return true +} + +var tmpCounter uint64 + func Prepare(ctx context.Context, dl Downloader, folder string, albumMeta map[string]any, cfg config.ArtworkConfig, forPlaylist bool) (Result, error) { saveArtwork := cfg.SaveArtwork if forPlaylist { @@ -51,30 +99,25 @@ func Prepare(ctx context.Context, dl Downloader, folder string, albumMeta map[st embedURL = largestURL } + // Everything below is check-then-write against shared paths. + unlock := lockFolder(folder) + defer unlock() + result := Result{} if saveArtwork && largestURL != "" { savedPath := filepath.Join(folder, "cover.jpg") - if fileExists(savedPath) { - result.SavedPath = savedPath - } else if err := dl.FileNoProgress(ctx, largestURL, savedPath); err == nil { - if cfg.SavedMaxWidth > 0 { - _ = downscaleImage(savedPath, cfg.SavedMaxWidth) - } + if fileExists(savedPath) || fetchAtomic(ctx, dl, largestURL, savedPath, cfg.SavedMaxWidth) { result.SavedPath = savedPath } } if cfg.Embed && embedURL != "" { embedDir := sessionEmbedDir(folder) - if err := os.MkdirAll(embedDir, 0o755); err == nil { + // 0o700: predictable path under the shared system temp dir. + if err := os.MkdirAll(embedDir, 0o700); err == nil { registerTempDir(embedDir) embedPath := filepath.Join(embedDir, embedFilename(embedURL)) - if fileExists(embedPath) { - result.EmbedPath = embedPath - } else if err := dl.FileNoProgress(ctx, embedURL, embedPath); err == nil { - if cfg.EmbedMaxWidth > 0 { - _ = downscaleImage(embedPath, cfg.EmbedMaxWidth) - } + if fileExists(embedPath) || fetchAtomic(ctx, dl, embedURL, embedPath, cfg.EmbedMaxWidth) { result.EmbedPath = embedPath } } diff --git a/internal/artwork/artwork_test.go b/internal/artwork/artwork_test.go new file mode 100644 index 0000000..78c5613 --- /dev/null +++ b/internal/artwork/artwork_test.go @@ -0,0 +1,177 @@ +package artwork + +import ( + "context" + "errors" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "streamrip-go/internal/config" +) + +// fakeDownloader models the real downloader's behaviour for this race: the +// first attempt at a given output path succeeds, and any later attempt at that +// same path fails and removes its partial output (downloader.go does +// os.Remove(outputPath) on failure). That is exactly how a losing worker used +// to delete the cover a winning worker had just written. +type fakeDownloader struct { + payload []byte + mu sync.Mutex + seen map[string]bool + delay time.Duration +} + +func newFakeDownloader(payload []byte, delay time.Duration) *fakeDownloader { + return &fakeDownloader{payload: payload, seen: map[string]bool{}, delay: delay} +} + +func (f *fakeDownloader) File(ctx context.Context, sourceURL, outputPath string) error { + return f.FileNoProgress(ctx, sourceURL, outputPath) +} + +func (f *fakeDownloader) FileNoProgress(_ context.Context, _, outputPath string) error { + f.mu.Lock() + first := !f.seen[outputPath] + f.seen[outputPath] = true + f.mu.Unlock() + + if !first { + _ = os.Remove(outputPath) + return errors.New("download failed") + } + if err := os.MkdirAll(filepath.Dir(outputPath), 0o700); err != nil { + return err + } + // Widen the window between another worker's existence check and this write. + time.Sleep(f.delay) + return os.WriteFile(outputPath, f.payload, 0o600) +} + +func artworkCfg() config.ArtworkConfig { + return config.ArtworkConfig{Embed: true, EmbedSize: "large", SaveArtwork: true} +} + +func albumMeta() map[string]any { + return map[string]any{"image": map[string]any{ + "original": "https://example.invalid/large.jpg", + "large": "https://example.invalid/large.jpg", + }} +} + +// TestPrepareConcurrentSameFolder is the regression test for the cover race: +// many workers calling Prepare for one album folder must leave exactly one +// intact cover.jpg. Run with -race. +func TestPrepareConcurrentSameFolder(t *testing.T) { + folder := t.TempDir() + payload := []byte("jpeg-bytes") + dl := newFakeDownloader(payload, 5*time.Millisecond) + + const workers = 16 + var wg sync.WaitGroup + results := make([]Result, workers) + for i := 0; i < workers; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + res, err := Prepare(context.Background(), dl, folder, albumMeta(), artworkCfg(), false) + if err != nil { + t.Errorf("Prepare: %v", err) + } + results[idx] = res + }(i) + } + wg.Wait() + + saved := filepath.Join(folder, "cover.jpg") + got, err := os.ReadFile(saved) + if err != nil { + t.Fatalf("cover.jpg missing after %d concurrent Prepare calls: %v", workers, err) + } + if string(got) != string(payload) { + t.Fatalf("cover.jpg corrupted: got %q want %q", got, payload) + } + + // No .part- temp files may survive. + entries, err := os.ReadDir(folder) + if err != nil { + t.Fatal(err) + } + for _, e := range entries { + if filepath.Ext(e.Name()) != ".jpg" { + t.Errorf("leftover temp artifact in album folder: %s", e.Name()) + } + } + + for i, res := range results { + if res.SavedPath != saved { + t.Errorf("worker %d: SavedPath = %q want %q", i, res.SavedPath, saved) + } + if res.EmbedPath == "" { + t.Errorf("worker %d: EmbedPath empty", i) + } + } +} + +// TestPrepareFailedDownloadKeepsExistingCover covers the specific corruption +// mode: a failing download must not remove a cover that is already on disk. +func TestPrepareFailedDownloadKeepsExistingCover(t *testing.T) { + folder := t.TempDir() + saved := filepath.Join(folder, "cover.jpg") + if err := os.WriteFile(saved, []byte("good-cover"), 0o600); err != nil { + t.Fatal(err) + } + + // Mark the saved path as already attempted so any download of it fails. + dl := newFakeDownloader([]byte("new"), 0) + dl.seen[saved] = true + + if _, err := Prepare(context.Background(), dl, folder, albumMeta(), artworkCfg(), false); err != nil { + t.Fatalf("Prepare: %v", err) + } + + got, err := os.ReadFile(saved) + if err != nil { + t.Fatalf("cover.jpg removed: %v", err) + } + if string(got) != "good-cover" { + t.Fatalf("existing cover overwritten: got %q", got) + } +} + +// TestFetchAtomicFailureLeavesNoPartial asserts a failed fetch neither creates +// the destination nor leaves a temp file behind. +func TestFetchAtomicFailureLeavesNoPartial(t *testing.T) { + folder := t.TempDir() + dest := filepath.Join(folder, "cover.jpg") + + dl := newFakeDownloader([]byte("x"), 0) + dl.seen[dest+".forced"] = true + // Force failure by pre-marking every temp path is impractical, so use a + // downloader that always fails instead. + always := &failingDownloader{} + + if ok := fetchAtomic(context.Background(), always, "https://example.invalid/a.jpg", dest, 0); ok { + t.Fatal("fetchAtomic reported success on a failing download") + } + if _, err := os.Stat(dest); !os.IsNotExist(err) { + t.Fatalf("destination should not exist, stat err = %v", err) + } + entries, err := os.ReadDir(folder) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Fatalf("temp files left behind: %v", entries) + } +} + +type failingDownloader struct{} + +func (failingDownloader) File(context.Context, string, string) error { return errors.New("nope") } +func (failingDownloader) FileNoProgress(_ context.Context, _, outputPath string) error { + _ = os.WriteFile(outputPath, []byte("partial"), 0o600) + return errors.New("nope") +} diff --git a/internal/audio/convert/convert.go b/internal/audio/convert/convert.go index 6dcf75a..2fc216f 100644 --- a/internal/audio/convert/convert.go +++ b/internal/audio/convert/convert.go @@ -1,6 +1,7 @@ package convert import ( + "context" "fmt" "os" "os/exec" @@ -27,7 +28,7 @@ var profiles = map[string]profile{ "AAC": {codecLib: "aac", ext: "m4a", lossless: false}, } -func Convert(path string, cfg config.ConversionConfig) (string, error) { +func Convert(ctx context.Context, path string, cfg config.ConversionConfig) (string, error) { if !cfg.Enabled { return path, nil } @@ -45,7 +46,7 @@ func Convert(path string, cfg config.ConversionConfig) (string, error) { tmpPath := finalPath + ".tmp." + p.ext args := buildFFmpegArgs(path, tmpPath, p, cfg) - cmd := exec.Command("ffmpeg", args...) + cmd := exec.CommandContext(ctx, "ffmpeg", args...) output, err := cmd.CombinedOutput() if err != nil { _ = os.Remove(tmpPath) diff --git a/internal/audio/convert/convert_test.go b/internal/audio/convert/convert_test.go index 243b390..3c58e49 100644 --- a/internal/audio/convert/convert_test.go +++ b/internal/audio/convert/convert_test.go @@ -1,6 +1,7 @@ package convert import ( + "context" "os" "path/filepath" "strings" @@ -86,7 +87,7 @@ func TestConvertKeepsSourceWhenRenameFails(t *testing.T) { } cfg := config.ConversionConfig{Enabled: true, Codec: "ALAC"} - out, err := Convert(in, cfg) + out, err := Convert(context.Background(), in, cfg) if err == nil { t.Fatalf("expected rename failure") } diff --git a/internal/audio/tag/tagger.go b/internal/audio/tag/tagger.go index 4d83842..853696b 100644 --- a/internal/audio/tag/tagger.go +++ b/internal/audio/tag/tagger.go @@ -1,6 +1,7 @@ package tag import ( + "context" "encoding/binary" "fmt" "os" @@ -45,17 +46,17 @@ func New() *Tagger { return &Tagger{} } -func (t *Tagger) TagFLAC(path string, meta Metadata, coverPath string) error { +func (t *Tagger) TagFLAC(ctx context.Context, path string, meta Metadata, coverPath string) error { if _, err := exec.LookPath("ffmpeg"); err != nil { return fmt.Errorf("ffmpeg not found: %w", err) } ext := strings.TrimPrefix(strings.ToLower(filepath.Ext(path)), ".") - forceMP4Muxer := shouldForceMP4Muxer(path, ext) + forceMP4Muxer := shouldForceMP4Muxer(ctx, path, ext) tmpPath := taggedTempPath(path) runTag := func(cover string) ([]byte, error) { args := buildFFmpegArgs(path, tmpPath, meta, cover, ext, forceMP4Muxer) - cmd := exec.Command("ffmpeg", args...) + cmd := exec.CommandContext(ctx, "ffmpeg", args...) return cmd.CombinedOutput() } @@ -125,7 +126,7 @@ func buildFFmpegArgs(inputPath, outputPath string, meta Metadata, coverPath, ext return args } -func shouldForceMP4Muxer(path, ext string) bool { +func shouldForceMP4Muxer(ctx context.Context, path, ext string) bool { switch strings.TrimPrefix(strings.ToLower(ext), ".") { case "m4a", "mp4": default: @@ -134,7 +135,7 @@ func shouldForceMP4Muxer(path, ext string) bool { if _, err := exec.LookPath("ffprobe"); err != nil { return false } - out, err := exec.Command("ffprobe", "-v", "error", "-select_streams", "a:0", "-show_entries", "stream=codec_name", "-of", "default=nokey=1:noprint_wrappers=1", path).Output() + out, err := exec.CommandContext(ctx, "ffprobe", "-v", "error", "-select_streams", "a:0", "-show_entries", "stream=codec_name", "-of", "default=nokey=1:noprint_wrappers=1", path).Output() if err != nil { return false } diff --git a/internal/config/config.go b/internal/config/config.go index 559e2f8..6c50c67 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -5,6 +5,8 @@ import ( "fmt" "os" "path/filepath" + "strings" + "sync" "github.com/pelletier/go-toml/v2" ) @@ -13,8 +15,17 @@ const CurrentConfigVersion = "2.2.0" var ErrOutdatedConfig = errors.New("config version mismatch") +// Config holds the on-disk config (File) and the runtime view with CLI +// overrides applied (Session). +// +// mu guards File, Session and the write to Path. Provider clients refresh OAuth +// tokens from inside concurrent download workers, so both the struct fields and +// the config-file write have to be serialized; without this, two workers +// refreshing at once raced each other and issued two concurrent os.WriteFile +// calls against the same path. type Config struct { Path string + mu sync.Mutex File ConfigData Session ConfigData } @@ -227,9 +238,35 @@ func UpgradeOutdated(path string) (string, error) { } func (c *Config) SaveFile() error { + c.mu.Lock() + defer c.mu.Unlock() + return c.saveFileLocked() +} + +func (c *Config) saveFileLocked() error { return saveConfigData(c.Path, c.File) } +// UpdateTokens applies fn to both the on-disk and session copies of the config +// under the config lock, then persists the result. Provider token refresh must +// go through this rather than mutating c.File/c.Session directly, because it +// runs concurrently from download workers. +func (c *Config) UpdateTokens(fn func(file, session *ConfigData)) error { + if fn == nil { + return nil + } + c.mu.Lock() + defer c.mu.Unlock() + fn(&c.File, &c.Session) + // An empty Path means there is no config file backing this Config (tests, + // and callers that loaded defaults only); the in-memory update still has to + // happen, but there is nothing to write. + if strings.TrimSpace(c.Path) == "" { + return nil + } + return c.saveFileLocked() +} + func DefaultConfigData() ConfigData { home, _ := os.UserHomeDir() appDir := defaultAppDir() @@ -326,11 +363,16 @@ func DefaultConfigData() ConfigData { } func resolvePath(path string) (string, error) { + // 0o700: this directory holds the credential-bearing config and the + // download databases. if path != "" { - return path, os.MkdirAll(filepath.Dir(path), 0o755) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return "", err + } + return path, nil } appDir := defaultAppDir() - if err := os.MkdirAll(appDir, 0o755); err != nil { + if err := os.MkdirAll(appDir, 0o700); err != nil { return "", err } return filepath.Join(appDir, "config.toml"), nil @@ -345,14 +387,20 @@ func defaultAppDir() string { } func saveConfigData(path string, data ConfigData) error { - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + // 0o700/0o600: this file holds plaintext provider passwords, Deezer ARL + // cookies and OAuth access/refresh tokens, so it must not be world-readable. + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { return err } b, err := toml.Marshal(data) if err != nil { return err } - return os.WriteFile(path, b, 0o644) + if err := os.WriteFile(path, b, 0o600); err != nil { + return err + } + // Tighten an existing file that a previous version created as 0o644. + return os.Chmod(path, 0o600) } func applyRuntimeDefaults(data *ConfigData) { diff --git a/internal/naming/naming.go b/internal/naming/naming.go index 411a61e..3287580 100644 --- a/internal/naming/naming.go +++ b/internal/naming/naming.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" "unicode" + "unicode/utf8" ) type Config struct { @@ -14,7 +15,7 @@ type Config struct { } var tokenRe = regexp.MustCompile(`\{([a-z_]+)(?::0?(\d+))?\}`) -var invalidPathRe = regexp.MustCompile(`[<>:"/\\|?*\x00-\x1F]`) +var invalidPathRe = regexp.MustCompile(`[<>:"/\\|?*\x00-\x1F\x7F]`) func FormatTemplate(template string, values map[string]string) string { return tokenRe.ReplaceAllStringFunc(template, func(m string) string { @@ -35,6 +36,28 @@ func FormatTemplate(template string, values map[string]string) string { }) } +// maxNameBytes is the per-component limit on ext4, APFS and most other modern +// filesystems. TruncateTo counts runes, so a 120-rune CJK title is ~360 bytes +// and would fail with ENAMETOOLONG without this second, byte-based cap. +const maxNameBytes = 255 + +// windowsReserved are device names that cannot be used as a path component on +// Windows, with or without an extension. +var windowsReserved = map[string]struct{}{ + "CON": {}, "PRN": {}, "AUX": {}, "NUL": {}, + "COM1": {}, "COM2": {}, "COM3": {}, "COM4": {}, "COM5": {}, + "COM6": {}, "COM7": {}, "COM8": {}, "COM9": {}, + "LPT1": {}, "LPT2": {}, "LPT3": {}, "LPT4": {}, "LPT5": {}, + "LPT6": {}, "LPT7": {}, "LPT8": {}, "LPT9": {}, +} + +// CleanName turns a remote title into a single safe path component. +// +// Everything it receives is attacker-controlled: album, playlist and track +// titles come straight from provider APIs, and on SoundCloud and Tidal they are +// user-generated. The result is joined onto the download root by the caller, so +// it must never be "." or ".." (which would escape or collapse into the parent +// directory) and must never contain a separator. func CleanName(in string, cfg Config) string { s := strings.TrimSpace(in) s = invalidPathRe.ReplaceAllString(s, "_") @@ -53,13 +76,45 @@ func CleanName(in string, cfg Config) string { s = string(runes[:cfg.TruncateTo]) } } + s = truncateBytes(s, maxNameBytes) + + // Trailing dots and spaces are stripped by Windows, which turns "foo." into + // "foo" and makes ".." survive as a traversal component. + s = strings.TrimRight(s, ". ") s = strings.TrimSpace(s) - if s == "" { + + if s == "" || isAllDots(s) { return "Unknown" } + if _, bad := windowsReserved[strings.ToUpper(s)]; bad { + return s + "_" + } return s } +// truncateBytes shortens s to at most max bytes without splitting a rune. +func truncateBytes(s string, max int) string { + if len(s) <= max { + return s + } + cut := s[:max] + for len(cut) > 0 && !utf8.ValidString(cut) { + cut = cut[:len(cut)-1] + } + return cut +} + +// isAllDots reports whether s consists only of "." characters, i.e. "." or +// ".." or any longer run that some filesystems also treat as relative. +func isAllDots(s string) bool { + for _, ch := range s { + if ch != '.' { + return false + } + } + return len(s) > 0 +} + func YearFromDate(date string) string { if len(date) >= 4 { prefix := date[:4] diff --git a/internal/naming/naming_test.go b/internal/naming/naming_test.go index a665a65..fe1c560 100644 --- a/internal/naming/naming_test.go +++ b/internal/naming/naming_test.go @@ -1,6 +1,10 @@ package naming -import "testing" +import ( + "path/filepath" + "strings" + "testing" +) func TestFormatTemplate(t *testing.T) { got := FormatTemplate("{tracknumber:02}. {artist} - {title}{explicit}", map[string]string{ @@ -15,8 +19,127 @@ func TestFormatTemplate(t *testing.T) { } func TestCleanName(t *testing.T) { - got := CleanName(" Dreams/Live ", Config{RestrictCharacters: false, TruncateTo: 120}) - if got != "Dreams_Live" { - t.Fatalf("got %q", got) + cfg := Config{RestrictCharacters: false, TruncateTo: 120} + tests := []struct { + name string + in string + want string + }{ + {"trims and replaces separator", " Dreams/Live ", "Dreams_Live"}, + {"backslash", `AC\DC`, "AC_DC"}, + {"colon", "Album: Deluxe", "Album_ Deluxe"}, + {"control chars", "Song\x01\x02", "Song__"}, + {"del char", "Song\x7f", "Song_"}, + {"empty", "", "Unknown"}, + {"whitespace only", " ", "Unknown"}, + + // Path traversal: these must never be returned verbatim, or the + // caller's filepath.Join escapes the download root. + {"dotdot", "..", "Unknown"}, + {"single dot", ".", "Unknown"}, + {"many dots", "....", "Unknown"}, + {"dotdot padded", " .. ", "Unknown"}, + {"trailing dot", "Album.", "Album"}, + {"trailing dots", "Album...", "Album"}, + {"leading dot kept", ".hidden", ".hidden"}, + + // Windows device names must not be used as a bare component. + {"reserved CON", "CON", "CON_"}, + {"reserved nul lowercase", "nul", "nul_"}, + {"reserved COM1", "COM1", "COM1_"}, + {"not reserved", "CONCERT", "CONCERT"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := CleanName(tc.in, cfg) + if got != tc.want { + t.Fatalf("CleanName(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +// TestCleanNameNeverEscapesRoot is the property that actually matters: whatever +// a provider returns, joining the result onto a root must stay under that root. +func TestCleanNameNeverEscapesRoot(t *testing.T) { + root := "/music" + hostile := []string{ + "..", ".", "../..", "../../etc", "..\\..", "/etc/passwd", + "....//....", " .. ", "", "...", "C:\\Windows", + "\x00evil", "a/../../b", + } + for _, in := range hostile { + for _, restrict := range []bool{false, true} { + cfg := Config{RestrictCharacters: restrict, TruncateTo: 120} + name := CleanName(in, cfg) + + if strings.ContainsAny(name, `/\`) { + t.Errorf("CleanName(%q) = %q contains a path separator", in, name) + } + if name == "." || name == ".." { + t.Errorf("CleanName(%q) = %q is a relative component", in, name) + } + + joined := filepath.Join(root, name) + rel, err := filepath.Rel(root, joined) + if err != nil { + t.Errorf("CleanName(%q) = %q: Rel failed: %v", in, name, err) + continue + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + t.Errorf("CleanName(%q) = %q escapes root: joined=%q rel=%q", in, name, joined, rel) + } + } + } +} + +func TestCleanNameTruncation(t *testing.T) { + // TruncateTo counts runes. + got := CleanName(strings.Repeat("a", 200), Config{TruncateTo: 10}) + if len(got) != 10 { + t.Fatalf("rune truncation: got %d chars (%q)", len(got), got) + } + + // A long multi-byte title must still fit the 255-byte component limit, + // and must remain valid UTF-8. + long := strings.Repeat("日", 200) // 3 bytes each = 600 bytes + got = CleanName(long, Config{TruncateTo: 200}) + if len(got) > maxNameBytes { + t.Fatalf("byte truncation: got %d bytes, want <= %d", len(got), maxNameBytes) + } + if !utf8Valid(got) { + t.Fatalf("truncation split a rune: %q", got) + } +} + +func utf8Valid(s string) bool { + for _, r := range s { + if r == '\uFFFD' { + return false + } + } + return true +} + +func TestCleanNameRestrictCharacters(t *testing.T) { + got := CleanName("Björk – Jóga", Config{RestrictCharacters: true, TruncateTo: 120}) + if strings.ContainsAny(got, "ö–ó") { + t.Fatalf("non-ascii survived restrict_characters: %q", got) + } +} + +func TestYearFromDate(t *testing.T) { + tests := []struct{ in, want string }{ + {"1977-02-04", "1977"}, + {"1977", "1977"}, + {"", "Unknown"}, + {"77", "Unknown"}, + {"abcd-01-01", "Unknown"}, + {"19x7-01-01", "Unknown"}, + } + for _, tc := range tests { + if got := YearFromDate(tc.in); got != tc.want { + t.Errorf("YearFromDate(%q) = %q, want %q", tc.in, got, tc.want) + } } } diff --git a/internal/provider/beatport/client.go b/internal/provider/beatport/client.go index a258306..a56ac1c 100644 --- a/internal/provider/beatport/client.go +++ b/internal/provider/beatport/client.go @@ -11,6 +11,7 @@ import ( "net/url" "strconv" "strings" + "sync" "time" "streamrip-go/internal/config" @@ -18,6 +19,7 @@ import ( "streamrip-go/internal/netutil" "streamrip-go/internal/provider" "streamrip-go/internal/ratelimit" + "streamrip-go/internal/verbose" ) const ( @@ -38,8 +40,13 @@ type Client struct { http *http.Client limiter *ratelimit.Limiter baseURL string - loggedIn bool sessionID string + + // authMu serializes the token refresh / re-login path and guards loggedIn. + // ensureToken is reached from apiRequest, which concurrent download workers + // call, so without this several workers would refresh at once. + authMu sync.Mutex + loggedIn bool } func New(cfg *config.Config) *Client { @@ -55,9 +62,15 @@ func New(cfg *config.Config) *Client { func (c *Client) Source() string { return "beatport" } -func (c *Client) LoggedIn() bool { return c.loggedIn } +func (c *Client) LoggedIn() bool { + c.authMu.Lock() + defer c.authMu.Unlock() + return c.loggedIn +} func (c *Client) Login(ctx context.Context) error { + c.authMu.Lock() + defer c.authMu.Unlock() b := &c.cfg.Session.Beatport b.Username = strings.TrimSpace(b.Username) b.Password = strings.TrimSpace(b.Password) @@ -82,7 +95,7 @@ func (c *Client) Login(ctx context.Context) error { } func (c *Client) GetMetadata(ctx context.Context, item, mediaType string) (map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errNotLoggedIn } switch mediaType { @@ -129,7 +142,7 @@ func (c *Client) GetMetadata(ctx context.Context, item, mediaType string) (map[s } func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) ([]map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errNotLoggedIn } if limit <= 0 { @@ -162,7 +175,7 @@ func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) } func (c *Client) GetDownloadable(ctx context.Context, item string, quality int) (*provider.Downloadable, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errNotLoggedIn } q := beatportQuality(quality) @@ -296,11 +309,8 @@ func (c *Client) apiRequest(ctx context.Context, method, endpoint string, params return nil, err } if status == http.StatusUnauthorized { - c.cfg.Session.Beatport.TokenExpiry = 0 - if err = c.refresh(ctx); err != nil { - if loginErr := c.loginPasswordFlow(ctx); loginErr != nil { - return nil, err - } + if err = c.reauthorize(ctx); err != nil { + return nil, err } resp, status, err = c.rawRequest(ctx, method, endpoint, params, payload, contentType, true) if err != nil { @@ -314,6 +324,13 @@ func (c *Client) apiRequest(ctx context.Context, method, endpoint string, params } func (c *Client) ensureToken(ctx context.Context) error { + c.authMu.Lock() + defer c.authMu.Unlock() + return c.ensureTokenLocked(ctx) +} + +// ensureTokenLocked requires c.authMu to be held. +func (c *Client) ensureTokenLocked(ctx context.Context) error { if strings.TrimSpace(c.cfg.Session.Beatport.AccessToken) != "" && time.Now().Unix()+300 < c.cfg.Session.Beatport.TokenExpiry { return nil } @@ -325,6 +342,21 @@ func (c *Client) ensureToken(ctx context.Context) error { return c.loginPasswordFlow(ctx) } +// reauthorize forces a token refresh after a 401. Holding authMu means that +// when several workers get a 401 at once only the first re-authenticates and +// the rest reuse the resulting token. +func (c *Client) reauthorize(ctx context.Context) error { + c.authMu.Lock() + defer c.authMu.Unlock() + c.cfg.Session.Beatport.TokenExpiry = 0 + if err := c.refresh(ctx); err != nil { + if loginErr := c.loginPasswordFlow(ctx); loginErr != nil { + return err + } + } + return nil +} + func (c *Client) loginPasswordFlow(ctx context.Context) error { b := &c.cfg.Session.Beatport if strings.TrimSpace(b.Username) == "" || strings.TrimSpace(b.Password) == "" { @@ -385,15 +417,19 @@ func (c *Client) issueToken(ctx context.Context, payload map[string]string) erro if expiresIn <= 0 { expiresIn = 3600 } - c.cfg.Session.Beatport.AccessToken = access - if refresh != "" { - c.cfg.Session.Beatport.RefreshToken = refresh + expiry := time.Now().Unix() + expiresIn + if err := c.cfg.UpdateTokens(func(file, session *config.ConfigData) { + session.Beatport.AccessToken = access + if refresh != "" { + session.Beatport.RefreshToken = refresh + } + session.Beatport.TokenExpiry = expiry + file.Beatport.AccessToken = session.Beatport.AccessToken + file.Beatport.RefreshToken = session.Beatport.RefreshToken + file.Beatport.TokenExpiry = session.Beatport.TokenExpiry + }); err != nil { + verbose.Printf(verbose.V, "beatport: could not persist refreshed token: %v\n", err) } - c.cfg.Session.Beatport.TokenExpiry = time.Now().Unix() + expiresIn - c.cfg.File.Beatport.AccessToken = c.cfg.Session.Beatport.AccessToken - c.cfg.File.Beatport.RefreshToken = c.cfg.Session.Beatport.RefreshToken - c.cfg.File.Beatport.TokenExpiry = c.cfg.Session.Beatport.TokenExpiry - _ = c.cfg.SaveFile() return nil } diff --git a/internal/provider/deezer/client.go b/internal/provider/deezer/client.go index 5d0190b..50fd5d0 100644 --- a/internal/provider/deezer/client.go +++ b/internal/provider/deezer/client.go @@ -15,6 +15,7 @@ import ( "net/url" "strconv" "strings" + "sync" "time" "streamrip-go/internal/config" @@ -22,6 +23,7 @@ import ( "streamrip-go/internal/netutil" "streamrip-go/internal/provider" "streamrip-go/internal/ratelimit" + "streamrip-go/internal/verbose" ) var ( @@ -47,9 +49,12 @@ var ( ) type Client struct { - cfg *config.Config - http *http.Client - limiter *ratelimit.Limiter + cfg *config.Config + http *http.Client + limiter *ratelimit.Limiter + // authMu guards loggedIn and the login/refresh path, which download + // workers can reach concurrently. + authMu sync.Mutex loggedIn bool ua string deviceID string @@ -82,6 +87,8 @@ func (c *Client) Source() string { } func (c *Client) Login(ctx context.Context) error { + c.authMu.Lock() + defer c.authMu.Unlock() c.arl = strings.TrimSpace(c.cfg.Session.Deezer.ARL) c.sid = "" c.jwt = "" @@ -114,6 +121,8 @@ func (c *Client) ensureLaunchSession(ctx context.Context) error { } func (c *Client) LoggedIn() bool { + c.authMu.Lock() + defer c.authMu.Unlock() return c.loggedIn } @@ -122,7 +131,7 @@ func (c *Client) Close() error { } func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) ([]map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("deezer client not logged in") } if limit <= 0 { @@ -151,7 +160,7 @@ func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) } func (c *Client) GetMetadata(ctx context.Context, item, mediaType string) (map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("deezer client not logged in") } @@ -471,10 +480,11 @@ func (c *Client) persistRefreshToken() { if rt == "" { return } - c.cfg.Session.Deezer.RefreshToken = rt - c.cfg.File.Deezer.RefreshToken = rt - if strings.TrimSpace(c.cfg.Path) != "" { - _ = c.cfg.SaveFile() + if err := c.cfg.UpdateTokens(func(file, session *config.ConfigData) { + file.Deezer.RefreshToken = rt + session.Deezer.RefreshToken = rt + }); err != nil { + verbose.Printf(verbose.V, "deezer: could not persist refreshed token: %v\n", err) } } diff --git a/internal/provider/qobuz/client.go b/internal/provider/qobuz/client.go index 71fd070..185963c 100644 --- a/internal/provider/qobuz/client.go +++ b/internal/provider/qobuz/client.go @@ -30,6 +30,7 @@ import ( "streamrip-go/internal/netutil" "streamrip-go/internal/provider" "streamrip-go/internal/ratelimit" + "streamrip-go/internal/verbose" "github.com/vbauerster/mpb/v8" "github.com/vbauerster/mpb/v8/decor" @@ -63,6 +64,9 @@ type Client struct { limiter *ratelimit.Limiter baseURL string fetchCfg func(ctx context.Context) (string, []string, error) + // authMu guards loggedIn and the login/refresh path, which download + // workers can reach concurrently. + authMu sync.Mutex loggedIn bool secret string uat string @@ -100,10 +104,14 @@ func (c *Client) Source() string { } func (c *Client) LoggedIn() bool { + c.authMu.Lock() + defer c.authMu.Unlock() return c.loggedIn } func (c *Client) Login(ctx context.Context) error { + c.authMu.Lock() + defer c.authMu.Unlock() q := &c.cfg.Session.Qobuz q.EmailOrUserID = strings.TrimSpace(q.EmailOrUserID) q.PasswordOrToken = strings.TrimSpace(q.PasswordOrToken) @@ -203,14 +211,18 @@ func (c *Client) refreshAppCredentials(ctx context.Context, q *config.QobuzConfi return errors.New("qobuz app credential refresh returned no secrets") } q.Secrets = append([]string(nil), clean...) - c.cfg.File.Qobuz.AppID = q.AppID - c.cfg.File.Qobuz.Secrets = append([]string(nil), clean...) - _ = c.cfg.SaveFile() + refreshedAppID := q.AppID + if err := c.cfg.UpdateTokens(func(file, session *config.ConfigData) { + file.Qobuz.AppID = refreshedAppID + file.Qobuz.Secrets = append([]string(nil), clean...) + }); err != nil { + verbose.Printf(verbose.V, "qobuz: could not persist refreshed credentials: %v\n", err) + } return nil } func (c *Client) GetMetadata(ctx context.Context, item, mediaType string) (map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errNotLoggedIn } if mediaType == "playlist" { @@ -260,7 +272,7 @@ func (c *Client) GetTrackMetadata(ctx context.Context, id string) (*TrackMetadat } func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) ([]map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errNotLoggedIn } if limit <= 0 { @@ -283,7 +295,7 @@ func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) } func (c *Client) GetDownloadable(ctx context.Context, item string, quality int) (*provider.Downloadable, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errNotLoggedIn } if quality < 1 || quality > 4 { diff --git a/internal/provider/soundcloud/client.go b/internal/provider/soundcloud/client.go index 541d208..81890da 100644 --- a/internal/provider/soundcloud/client.go +++ b/internal/provider/soundcloud/client.go @@ -26,13 +26,17 @@ var soundcloudSearchBaseURL = "https://soundcloud.com" type commandRunner func(ctx context.Context, name string, args ...string) ([]byte, error) type Client struct { - cfg *config.Config + cfg *config.Config + bin string + run commandRunner + http *http.Client + mu sync.Mutex + cache map[string]map[string]any + + // authMu guards loggedIn. Kept separate from mu, which guards cache, so a + // cache operation can never block a login check. + authMu sync.Mutex loggedIn bool - bin string - run commandRunner - http *http.Client - mu sync.Mutex - cache map[string]map[string]any } func New(cfg *config.Config) *Client { @@ -50,6 +54,8 @@ func (c *Client) Source() string { } func (c *Client) LoggedIn() bool { + c.authMu.Lock() + defer c.authMu.Unlock() return c.loggedIn } @@ -57,12 +63,14 @@ func (c *Client) Login(context.Context) error { if _, err := exec.LookPath(c.bin); err != nil { return fmt.Errorf("yt-dlp is required for soundcloud downloads/search. install it and ensure it is in $PATH (e.g. pipx install yt-dlp): %w", err) } + c.authMu.Lock() + defer c.authMu.Unlock() c.loggedIn = true return nil } func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) ([]map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("soundcloud client not logged in") } if limit <= 0 { @@ -201,7 +209,7 @@ func (c *Client) searchPlaylists(ctx context.Context, query string, limit int) ( } func (c *Client) GetMetadata(ctx context.Context, item, mediaType string) (map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("soundcloud client not logged in") } @@ -273,7 +281,7 @@ func (c *Client) GetMetadata(ctx context.Context, item, mediaType string) (map[s } func (c *Client) GetDownloadable(ctx context.Context, item string, _ int) (*provider.Downloadable, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("soundcloud client not logged in") } info, err := c.trackInfo(ctx, item) diff --git a/internal/provider/tidal/client.go b/internal/provider/tidal/client.go index eda8626..e4eb89c 100644 --- a/internal/provider/tidal/client.go +++ b/internal/provider/tidal/client.go @@ -12,6 +12,7 @@ import ( "net/url" "strconv" "strings" + "sync" "time" "streamrip-go/internal/config" @@ -19,6 +20,7 @@ import ( "streamrip-go/internal/netutil" "streamrip-go/internal/provider" "streamrip-go/internal/ratelimit" + "streamrip-go/internal/verbose" ) const ( @@ -58,7 +60,10 @@ type Client struct { baseURL string lyricsAPI string openAPI string - loggedIn bool + // authMu guards loggedIn and the login/refresh path, which download + // workers can reach concurrently. + authMu sync.Mutex + loggedIn bool } func New(cfg *config.Config) *Client { @@ -77,10 +82,14 @@ func (c *Client) Source() string { } func (c *Client) LoggedIn() bool { + c.authMu.Lock() + defer c.authMu.Unlock() return c.loggedIn } func (c *Client) Login(ctx context.Context) error { + c.authMu.Lock() + defer c.authMu.Unlock() if strings.TrimSpace(c.cfg.Session.Tidal.AccessToken) == "" { return ErrMissingTidalToken } @@ -161,22 +170,25 @@ func (c *Client) refreshAccessToken(ctx context.Context) error { expiresIn = 7 * 24 * 3600 } - c.cfg.Session.Tidal.AccessToken = newToken - c.cfg.File.Tidal.AccessToken = newToken - if newRefresh != "" { - c.cfg.Session.Tidal.RefreshToken = newRefresh - c.cfg.File.Tidal.RefreshToken = newRefresh - } expiry := time.Now().Unix() + expiresIn - c.cfg.Session.Tidal.TokenExpiry = expiry - c.cfg.File.Tidal.TokenExpiry = expiry - _ = c.cfg.SaveFile() + if err := c.cfg.UpdateTokens(func(file, session *config.ConfigData) { + file.Tidal.AccessToken = newToken + session.Tidal.AccessToken = newToken + if newRefresh != "" { + file.Tidal.RefreshToken = newRefresh + session.Tidal.RefreshToken = newRefresh + } + file.Tidal.TokenExpiry = expiry + session.Tidal.TokenExpiry = expiry + }); err != nil { + verbose.Printf(verbose.V, "tidal: could not persist refreshed token: %v\n", err) + } return nil } func (c *Client) GetMetadata(ctx context.Context, item, mediaType string) (map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("tidal client not logged in") } @@ -257,7 +269,7 @@ func (c *Client) fetchTrackLyrics(ctx context.Context, trackID string) (string, } func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) ([]map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("tidal client not logged in") } if limit <= 0 { @@ -284,7 +296,7 @@ func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) } func (c *Client) GetDownloadable(ctx context.Context, trackID string, quality int) (*provider.Downloadable, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("tidal client not logged in") } if quality < 0 || quality > 4 { @@ -601,7 +613,7 @@ func formatsForQuality(quality int, preferAtmos bool) []string { } func (c *Client) GetVideoDownloadable(ctx context.Context, videoID string) (*provider.Downloadable, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("tidal client not logged in") } diff --git a/internal/provider/yandex/client.go b/internal/provider/yandex/client.go index 789336c..270e8fb 100644 --- a/internal/provider/yandex/client.go +++ b/internal/provider/yandex/client.go @@ -18,6 +18,7 @@ import ( "sort" "strconv" "strings" + "sync" "time" "streamrip-go/internal/config" @@ -25,6 +26,7 @@ import ( "streamrip-go/internal/netutil" "streamrip-go/internal/provider" "streamrip-go/internal/ratelimit" + "streamrip-go/internal/verbose" ) const ( @@ -40,10 +42,13 @@ const ( var ErrMissingYandexToken = errors.New("missing yandex access_token") type Client struct { - cfg *config.Config - http *http.Client - limiter *ratelimit.Limiter - baseURL string + cfg *config.Config + http *http.Client + limiter *ratelimit.Limiter + baseURL string + // authMu guards loggedIn and the login/refresh path, which download + // workers can reach concurrently. + authMu sync.Mutex loggedIn bool userID string } @@ -63,10 +68,14 @@ func (c *Client) Source() string { } func (c *Client) LoggedIn() bool { + c.authMu.Lock() + defer c.authMu.Unlock() return c.loggedIn } func (c *Client) Login(ctx context.Context) error { + c.authMu.Lock() + defer c.authMu.Unlock() if strings.TrimSpace(c.cfg.Session.Yandex.AccessToken) == "" { return ErrMissingYandexToken } @@ -80,16 +89,19 @@ func (c *Client) Login(ctx context.Context) error { result, _ := resp["result"].(map[string]any) if uid := strings.TrimSpace(jsonutil.StringFromAny(result["uid"])); uid != "" { c.userID = uid - c.cfg.Session.Yandex.UserID = uid - c.cfg.File.Yandex.UserID = uid - _ = c.cfg.SaveFile() + if err := c.cfg.UpdateTokens(func(file, session *config.ConfigData) { + file.Yandex.UserID = uid + session.Yandex.UserID = uid + }); err != nil { + verbose.Printf(verbose.V, "yandex: could not persist user id: %v\n", err) + } } c.loggedIn = true return nil } func (c *Client) GetMetadata(ctx context.Context, item, mediaType string) (map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("yandex client not logged in") } switch mediaType { @@ -107,7 +119,7 @@ func (c *Client) GetMetadata(ctx context.Context, item, mediaType string) (map[s } func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) ([]map[string]any, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("yandex client not logged in") } if limit <= 0 { @@ -137,7 +149,7 @@ func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) } func (c *Client) GetDownloadable(ctx context.Context, item string, quality int) (*provider.Downloadable, error) { - if !c.loggedIn { + if !c.LoggedIn() { return nil, errors.New("yandex client not logged in") } trackID, _ := splitTrackRef(item)