diff --git a/CHANGELOG.md b/CHANGELOG.md index 69d3b4c..7c6585a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,8 @@ ## Unreleased - Honor long Spotify cooldowns without premature retries, retain valid tokens on throttling, and keep cookie-token requests from changing shared HTTP clients - +- Preserve literal CLI arguments and pass playback URIs to AppleScript as data; accept locale-prefixed and embedded Spotify URLs +- Compatibility: reject malformed Spotify URIs, missing identifiers, and unrelated URLs instead of silently truncating or misidentifying them - Let Windows config updates finish after concurrent readers close, including when replacement reports access denied - Refresh the SQLite runtime dependency and preferred Go toolchain to 1.27.1 while retaining Go 1.26.7 support; validate macOS, Windows, the Go floor, race coverage, and docs in CI - Correct command, output, cookie-cache, and automation documentation to match the CLI, including working playlist pipelines and release asset names diff --git a/cmd/spogo/arguments_test.go b/cmd/spogo/arguments_test.go new file mode 100644 index 0000000..d2cdf74 --- /dev/null +++ b/cmd/spogo/arguments_test.go @@ -0,0 +1,64 @@ +package main + +import ( + "bytes" + "io" + "net/http" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/steipete/spogo/internal/config" + "github.com/steipete/spogo/internal/spotify" +) + +type argumentTransport func(*http.Request) (*http.Response, error) + +func (f argumentTransport) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func TestRunSearchPreservesLiteralArguments(t *testing.T) { + for _, tc := range []struct { + name string + args []string + query string + }{ + {"literal", []string{"search", "track", "--", "--no-input"}, "--no-input"}, + {"flag before", []string{"--no-input", "search", "track", "query"}, "query"}, + {"flag after", []string{"search", "track", "query", "--no-input"}, "query"}, + } { + t.Run(tc.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.toml") + cfg := config.Default() + cfg.SetProfile("default", config.Profile{Engine: "web", Auth: "oauth", SpotifyClientID: "synthetic-client"}) + if err := config.Save(configPath, cfg); err != nil { + t.Fatal(err) + } + if err := spotify.SaveOAuthToken(config.OAuthTokenPath(configPath, "default"), spotify.OAuthToken{AccessToken: "synthetic-access", RefreshToken: "synthetic-refresh", ClientID: "synthetic-client", ExpiresAt: time.Now().Add(time.Hour)}); err != nil { + t.Fatal(err) + } + previous := http.DefaultTransport + t.Cleanup(func() { http.DefaultTransport = previous }) + calls := 0 + http.DefaultTransport = argumentTransport(func(r *http.Request) (*http.Response, error) { + calls++ + if r.URL.Path != "/v1/search" || r.URL.Query().Get("q") != tc.query { + t.Errorf("unexpected request %s", r.URL) + } + body := `{"tracks":{"items":[{"id":"t1","name":"Synthetic Track"}],"total":1,"limit":1,"offset":0}}` + return &http.Response{StatusCode: 200, Header: make(http.Header), Body: io.NopCloser(strings.NewReader(body)), ContentLength: int64(len(body)), Request: r}, nil + }) + out, errOut := new(bytes.Buffer), new(bytes.Buffer) + args := append([]string{"--config", configPath, "--json"}, tc.args...) + if code := run(args, out, errOut); code != 0 { + t.Fatalf("exit %d: %s", code, errOut) + } + if calls != 1 { + t.Fatalf("requests = %d, want 1", calls) + } + if !strings.Contains(out.String(), "Synthetic Track") { + t.Fatalf("command output bypassed supplied writer: %q", out) + } + }) + } +} diff --git a/cmd/spogo/main.go b/cmd/spogo/main.go index b82bec3..cf14349 100644 --- a/cmd/spogo/main.go +++ b/cmd/spogo/main.go @@ -1,7 +1,6 @@ package main import ( - "context" "fmt" "io" "os" @@ -39,7 +38,6 @@ func run(args []string, out io.Writer, errOut io.Writer) int { if exitCode >= 0 { return exitCode } - args = normalizeArgs(args) kctx, err := parser.Parse(args) if exitCode >= 0 { return exitCode @@ -65,7 +63,8 @@ func run(args []string, out io.Writer, errOut io.Writer) int { _, _ = fmt.Fprintln(errOut, err) return 1 } - ctx.SetCommandContext(context.Background()) + ctx.Output.Out = out + ctx.Output.Err = errOut if err := ctx.ValidateProfile(); err != nil { _, _ = fmt.Fprintln(errOut, err) return 2 @@ -76,25 +75,3 @@ func run(args []string, out io.Writer, errOut io.Writer) int { } return 0 } - -func normalizeArgs(args []string) []string { - if len(args) == 0 { - return args - } - front := make([]string, 0, 1) - rest := make([]string, 0, len(args)) - for _, arg := range args { - if arg == "--no-input" { - front = append(front, arg) - continue - } - rest = append(rest, arg) - } - if len(front) == 0 { - return args - } - normalized := make([]string, 0, len(args)) - normalized = append(normalized, front...) - normalized = append(normalized, rest...) - return normalized -} diff --git a/cmd/spogo/main_test.go b/cmd/spogo/main_test.go index 39368eb..97c8baa 100644 --- a/cmd/spogo/main_test.go +++ b/cmd/spogo/main_test.go @@ -229,14 +229,6 @@ func TestRunOAuthStatusMismatchReturnsAuthExitCode(t *testing.T) { } } -func TestNormalizeArgsMovesNoInput(t *testing.T) { - got := normalizeArgs([]string{"auth", "paste", "--no-input", "--cookie-path", "cookies.json"}) - want := []string{"--no-input", "auth", "paste", "--cookie-path", "cookies.json"} - if fmt.Sprint(got) != fmt.Sprint(want) { - t.Fatalf("got %v, want %v", got, want) - } -} - func TestMain(t *testing.T) { origArgs := os.Args origExit := exitFunc diff --git a/docs/commands.md b/docs/commands.md index 9afe98c..28e84ee 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -84,6 +84,8 @@ Cookie and official Spotify OAuth management. See [Auth](auth.md). ## search +Use `--` before positional text that begins with a dash, for example `spogo search track -- --no-input`. Global flags remain valid before or after the command. + Browse the catalog. Each subcommand takes a query plus `--limit N` and `--offset N`. | Command | Returns | diff --git a/docs/playback.md b/docs/playback.md index 2a34fe4..0d47bad 100644 --- a/docs/playback.md +++ b/docs/playback.md @@ -16,7 +16,7 @@ spogo play [] [--type ] [--shuffle] Accepts: - A Spotify URI: `spotify:track:7hQJA50XrCWABAu5v6QZ4i`, `spotify:album:...`, `spotify:playlist:...`, `spotify:show:...`, `spotify:episode:...`, `spotify:artist:...`. -- A web URL: `https://open.spotify.com/track/7hQJA50XrCWABAu5v6QZ4i`. +- A web URL: `https://open.spotify.com/track/7hQJA50XrCWABAu5v6QZ4i`. Locale-prefixed (`/intl-de/track/...`) and embedded (`/embed/track/...`) Spotify URLs work too. - A bare ID — combine with `--type` to disambiguate. - No argument — resumes the current item. diff --git a/internal/spotify/applescript.go b/internal/spotify/applescript.go index d11d692..ab92afb 100644 --- a/internal/spotify/applescript.go +++ b/internal/spotify/applescript.go @@ -25,8 +25,13 @@ func NewAppleScriptClient(opts AppleScriptOptions) (API, error) { }, nil } -func (c *AppleScriptClient) runScript(ctx context.Context, script string) (string, error) { - cmd := exec.CommandContext(ctx, "osascript", "-e", script) +func (c *AppleScriptClient) runScript(ctx context.Context, script string, args ...string) (string, error) { + commandArgs := []string{"-e", script} + if len(args) > 0 { + commandArgs = append(commandArgs, "--") + commandArgs = append(commandArgs, args...) + } + cmd := exec.CommandContext(ctx, "osascript", commandArgs...) out, err := cmd.CombinedOutput() if err != nil { msg := strings.TrimSpace(string(out)) @@ -39,13 +44,13 @@ func (c *AppleScriptClient) runScript(ctx context.Context, script string) (strin } func (c *AppleScriptClient) Play(ctx context.Context, uri string) error { - var script string if uri == "" { - script = `tell application "Spotify" to play` - } else { - script = fmt.Sprintf(`tell application "Spotify" to play track "%s"`, uri) + _, err := c.runScript(ctx, `tell application "Spotify" to play`) + return err } - _, err := c.runScript(ctx, script) + _, err := c.runScript(ctx, `on run argv + tell application "Spotify" to play track (item 1 of argv) +end run`, uri) return err } diff --git a/internal/spotify/applescript_test.go b/internal/spotify/applescript_test.go index f91cb88..bb52ada 100644 --- a/internal/spotify/applescript_test.go +++ b/internal/spotify/applescript_test.go @@ -56,7 +56,8 @@ func TestAppleScriptClientLocalCommands(t *testing.T) { log := string(logData) for _, want := range []string{ "to play", - `play track "spotify:track:1"`, + "play track (item 1 of argv)", + "spotify:track:1", "to pause", "to next track", "to previous track", @@ -216,7 +217,7 @@ func installFakeOsaScript(t *testing.T) string { logPath := filepath.Join(dir, "osascript.log") scriptPath := filepath.Join(dir, "osascript") script := `#!/bin/sh -printf '%s\n' "$2" >> "$SPOGO_OSASCRIPT_LOG" +printf '%s\0' "$@" >> "$SPOGO_OSASCRIPT_LOG" if [ -n "$SPOGO_OSASCRIPT_ERROR" ]; then printf '%s\n' "$SPOGO_OSASCRIPT_ERROR" exit 1 @@ -230,3 +231,22 @@ printf '%s\n' "$SPOGO_OSASCRIPT_OUTPUT" t.Setenv("SPOGO_OSASCRIPT_LOG", logPath) return logPath } + +func TestAppleScriptPlayKeepsURIOutOfSource(t *testing.T) { + logPath := installFakeOsaScript(t) + uri := "spotify:track:quoted\"\\track\nliteral" + if err := (&AppleScriptClient{}).Play(context.Background(), uri); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(logPath) + if err != nil { + t.Fatal(err) + } + args := strings.Split(strings.TrimSuffix(string(data), "\x00"), "\x00") + if len(args) != 4 || args[0] != "-e" || args[2] != "--" || args[3] != uri { + t.Fatalf("unexpected osascript arguments: %q", args) + } + if strings.Contains(args[1], uri) || !strings.Contains(args[1], "item 1 of argv") { + t.Fatalf("URI must be passed as data, script: %q", args[1]) + } +} diff --git a/internal/spotify/parse.go b/internal/spotify/parse.go index 9e3e6c0..e52c9a5 100644 --- a/internal/spotify/parse.go +++ b/internal/spotify/parse.go @@ -3,7 +3,6 @@ package spotify import ( "errors" "net/url" - "path" "strings" ) @@ -29,40 +28,47 @@ func ParseResource(input string) (Resource, error) { if input == "" { return Resource{}, errors.New("empty input") } - if strings.HasPrefix(input, "spotify:") { + if strings.HasPrefix(strings.ToLower(input), "spotify:") { parts := strings.Split(input, ":") - if len(parts) < 3 { + if len(parts) != 3 { return Resource{}, errors.New("invalid spotify uri") } - kind := parts[1] - id := parts[2] - if !isSupportedType(kind) { - return Resource{}, ErrUnsupportedType - } - return Resource{Type: kind, ID: id, URI: "spotify:" + kind + ":" + id}, nil + return typedResource(parts[1], parts[2]) } - if strings.HasPrefix(input, "open.spotify.com/") { + if strings.HasPrefix(strings.ToLower(input), "open.spotify.com/") { input = "https://" + input } - if strings.Contains(input, "open.spotify.com/") { + if strings.Contains(input, "://") { parsed, err := url.Parse(input) if err != nil { return Resource{}, err } - segments := strings.Split(strings.Trim(path.Clean(parsed.Path), "/"), "/") - if len(segments) < 2 { + if (parsed.Scheme != "https" && parsed.Scheme != "http") || + !strings.EqualFold(parsed.Hostname(), "open.spotify.com") || parsed.User != nil { return Resource{}, errors.New("invalid spotify url") } - kind := segments[0] - id := segments[1] - if !isSupportedType(kind) { - return Resource{}, ErrUnsupportedType + segments := strings.Split(strings.Trim(parsed.Path, "/"), "/") + if len(segments) == 3 && (strings.HasPrefix(segments[0], "intl-") || segments[0] == "embed") { + segments = segments[1:] + } + if len(segments) != 2 { + return Resource{}, errors.New("invalid spotify url") } - return Resource{Type: kind, ID: id, URI: "spotify:" + kind + ":" + id}, nil + return typedResource(segments[0], segments[1]) } return Resource{ID: input}, nil } +func typedResource(kind, id string) (Resource, error) { + if !isSupportedType(kind) { + return Resource{}, ErrUnsupportedType + } + if strings.TrimSpace(id) == "" { + return Resource{}, errors.New("spotify id required") + } + return Resource{Type: kind, ID: id, URI: "spotify:" + kind + ":" + id}, nil +} + func ParseTypedID(input, expectedType string) (Resource, error) { res, err := ParseResource(input) if err != nil { diff --git a/internal/spotify/parse_more_test.go b/internal/spotify/parse_more_test.go index 569ab87..e7f1b92 100644 --- a/internal/spotify/parse_more_test.go +++ b/internal/spotify/parse_more_test.go @@ -26,3 +26,33 @@ func TestParseTypedIDNoExpectedType(t *testing.T) { t.Fatalf("unexpected: %#v", res) } } + +func TestParseResourceShareURLs(t *testing.T) { + for _, input := range []string{ + "https://open.spotify.com/intl-de/track/abc?si=share", + "open.spotify.com/intl-pt/track/abc", + "https://OPEN.SPOTIFY.COM/track/abc", + "HTTPS://open.spotify.com/track/abc", + "hTtP://open.spotify.com/track/abc", + "SPOTIFY:track:abc", + "https://open.spotify.com/embed/track/abc", + } { + res, err := ParseResource(input) + if err != nil || res.URI != "spotify:track:abc" { + t.Errorf("ParseResource(%q) = %+v, %v", input, res, err) + } + } +} + +func TestParseResourceRejectsMalformedResources(t *testing.T) { + for _, input := range []string{ + "spotify:track:", "spotify:track:abc:extra", + "https://open.spotify.com/track/", "https://open.spotify.com/track/abc/extra", + "https://example.test/track/abc?ref=open.spotify.com/", + "https://user@open.spotify.com/track/abc", + } { + if _, err := ParseResource(input); err == nil { + t.Errorf("accepted malformed resource %q", input) + } + } +}