fix: resolve bugs found in pre-release review

cmd/login.go

@@ -2,6 +2,8 @@ package cmd
 
 import (
 	"bufio"
+	"context"
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -21,24 +23,61 @@ func newLoginCmd() *cobra.Command {
 				return err
 			}
 
-			email, password, err := promptCredentials()
+			err := runLogin(cmd.Context())
-			if err != nil {
+			if errors.Is(err, context.Canceled) {
-				return err
+				return nil
 			}
 
-			_, username, err := deezer.Login(cmd.Context(), email, password)
+			return err
-			if err != nil {
-				return err
-			}
-
-			fmt.Printf("Successfully logged in as %s.\n", username)
-
-			return nil
 		},
 	}
 }
 
-func promptCredentials() (string, string, error) {
+func runLogin(ctx context.Context) error {
+	email, password, err := promptCredentials(ctx)
+	if err != nil {
+		return err
+	}
+
+	_, username, err := deezer.Login(ctx, email, password)
+	if err != nil {
+		return err
+	}
+
+	fmt.Printf("Successfully logged in as %s.\n", username)
+
+	return nil
+}
+
+func promptCredentials(ctx context.Context) (string, string, error) {
+	oldState, stateErr := term.GetState(int(os.Stdin.Fd()))
+
+	type credentials struct {
+		email    string
+		password string
+		err      error
+	}
+	resultChan := make(chan credentials, 1)
+	go func() {
+		var c credentials
+		c.email, c.password, c.err = readCredentials()
+		resultChan <- c
+	}()
+
+	select {
+	case c := <-resultChan:
+		return c.email, c.password, c.err
+	case <-ctx.Done():
+		if stateErr == nil {
+			term.Restore(int(os.Stdin.Fd()), oldState)
+		}
+		fmt.Println()
+
+		return "", "", ctx.Err()
+	}
+}
+
+func readCredentials() (string, string, error) {
 	fmt.Print("Email: ")
 	line, err := bufio.NewReader(os.Stdin).ReadString('\n')
 	if err != nil {

internal/config/migrate.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"io"
 	"os"
 	"path/filepath"
 )
@@ -11,15 +12,54 @@ func MigrateLegacy(outputDir string) {
 		return
 	}
 
-	oldDB := filepath.Join(homeDir, ".godeez", "tracks.db")
+	oldDir := filepath.Join(homeDir, ".godeez")
+	oldDB := filepath.Join(oldDir, "tracks.db")
 	newDB := filepath.Join(outputDir, ".tracks.db")
 	if _, err := os.Stat(oldDB); err == nil {
 		if _, err := os.Stat(newDB); os.IsNotExist(err) {
-			os.Rename(oldDB, newDB)
+			if err := os.Rename(oldDB, newDB); err != nil {
+				if err := copyFile(oldDB, newDB); err != nil {
+					return
+				}
+				os.Remove(oldDB)
+			}
 		}
 	}
 
-	oldDir := filepath.Join(homeDir, ".godeez")
-	os.Remove(filepath.Join(oldDir, "config.toml"))
 	os.Remove(oldDir)
 }
+
+func copyFile(src, dst string) error {
+	in, err := os.Open(src)
+	if err != nil {
+		return err
+	}
+	defer in.Close()
+
+	out, err := os.CreateTemp(filepath.Dir(dst), ".tracks.db-*.tmp")
+	if err != nil {
+		return err
+	}
+	tmp := out.Name()
+
+	if _, err := io.Copy(out, in); err != nil {
+		out.Close()
+		os.Remove(tmp)
+		return err
+	}
+	if err := out.Sync(); err != nil {
+		out.Close()
+		os.Remove(tmp)
+		return err
+	}
+	if err := out.Close(); err != nil {
+		os.Remove(tmp)
+		return err
+	}
+	if err := os.Rename(tmp, dst); err != nil {
+		os.Remove(tmp)
+		return err
+	}
+
+	return nil
+}

internal/deezer/client.go

@@ -213,6 +213,7 @@ func (c *Client) MediaStream(ctx context.Context, media *Media) (io.ReadCloser,
 	}
 
 	if resp.StatusCode != http.StatusOK {
+		resp.Body.Close()
 		return nil, fmt.Errorf("unexpected status code: %d", resp.StatusCode)
 	}
 

internal/deezer/gateway.go

@@ -120,6 +120,10 @@ func (m *mobileClient) authenticate(ctx context.Context) (string, string, string
 		return "", "", "", err
 	}
 
+	if len(decrypted) < 96 {
+		return "", "", "", fmt.Errorf("unexpected response from gateway")
+	}
+
 	token := string(decrypted[0:64])
 	tokenKey := string(decrypted[64:80])
 	userKey := string(decrypted[80:96])

internal/deezer/login.go

@@ -2,6 +2,7 @@ package deezer
 
 import (
 	"context"
+	"errors"
 	"fmt"
 )
 
@@ -12,9 +13,14 @@ func resolveARL(ctx context.Context, validate func(ctx context.Context, arl stri
 	}
 
 	if creds != nil && creds.ARL != "" {
-		if verr := validate(ctx, creds.ARL); verr == nil {
+		verr := validate(ctx, creds.ARL)
+		if verr == nil {
 			return creds.ARL, nil
 		}
+
+		if !errors.Is(verr, ErrInvalidARL) {
+			return "", fmt.Errorf("stored session could not be validated: %w", verr)
+		}
 	}
 
 	if creds != nil && creds.Email != "" && creds.Password != "" {

internal/deezer/session.go

@@ -3,6 +3,7 @@ package deezer
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -10,6 +11,8 @@ import (
 	"time"
 )
 
+var ErrInvalidARL = errors.New("invalid or expired ARL cookie")
+
 type Session struct {
 	APIToken     string
 	LicenseToken string
@@ -71,7 +74,7 @@ func authenticate(ctx context.Context, arlCookie string) (*Session, error) {
 	}
 
 	if res.Results.User.ID == 0 {
-		return nil, fmt.Errorf("invalid or expired ARL cookie")
+		return nil, ErrInvalidARL
 	}
 
 	opts := res.Results.User.Options

internal/deezer/track.go

@@ -62,7 +62,27 @@ func (t *Track) Filename(kind Kind, mediaFormat string) string {
 		}
 	}
 
-	fileName := fmt.Sprintf("%s%s - %s.%s", prefix, t.Artist, t.FullTitle(), ext)
+	base := fmt.Sprintf("%s%s - %s", prefix, t.Artist, t.FullTitle())
-	fileName, _ = filenamify.Filenamify(fileName, filenamify.Options{MaxLength: 255})
+	base, _ = filenamify.Filenamify(base, filenamify.Options{MaxLength: 255})
-	return fileName
+	base = truncateBytes(base, 255-len(ext)-1-len("-id3v2"))
+
+	return base + "." + ext
+}
+
+func truncateBytes(s string, max int) string {
+	if max <= 0 {
+		return ""
+	}
+	if len(s) <= max {
+		return s
+	}
+
+	last := 0
+	for i := range s {
+		if i > max {
+			return s[:last]
+		}
+		last = i
+	}
+	return s[:last]
 }

internal/download/bpm.go

@@ -93,7 +93,7 @@ func findTrackURL(ctx context.Context, httpClient *http.Client, artist, title, d
 
 		const toleranceSec = 2
 		foundDuration := minutes*60 + seconds
-		if foundDuration <= wantDuration-toleranceSec || foundDuration >= wantDuration+toleranceSec {
+		if foundDuration < wantDuration-toleranceSec || foundDuration > wantDuration+toleranceSec {
 			return true
 		}
 

internal/download/download.go

@@ -81,6 +81,7 @@ func (d *Downloader) prepareResource(ctx context.Context, id string, opts Option
 	if err := fsutil.EnsureDir(outputDir); err != nil {
 		return nil, "", fmt.Errorf("failed to create output directory: %w", err)
 	}
+	sweepPartFiles(outputDir)
 
 	return resource, outputDir, nil
 }

internal/download/genre.go

@@ -4,7 +4,10 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
+	"unicode"
+	"unicode/utf8"
 
 	"github.com/PuerkitoBio/goquery"
 )
@@ -31,7 +34,9 @@ func toLower(ss []string) []string {
 }
 
 func fetchGenre(ctx context.Context, httpClient *http.Client, artist, title string) (string, error) {
-	reqURL := fmt.Sprintf("https://www.last.fm/music/%s/%s/+tags", artist, title)
+	// Escape the path segments: names containing '/', '?', or '#' would
+	// otherwise change the URL structure and fetch the wrong page.
+	reqURL := fmt.Sprintf("https://www.last.fm/music/%s/%s/+tags", url.PathEscape(artist), url.PathEscape(title))
 
 	doc, err := fetchGenrePage(ctx, httpClient, reqURL)
 	if err != nil {
@@ -116,7 +121,8 @@ func formatTags(tags []string) string {
 		}
 		words := strings.Fields(tag)
 		for i, w := range words {
-			words[i] = strings.ToUpper(w[:1]) + strings.ToLower(w[1:])
+			r, size := utf8.DecodeRuneInString(w)
+			words[i] = string(unicode.ToUpper(r)) + strings.ToLower(w[size:])
 		}
 		formatted = append(formatted, strings.Join(words, " "))
 	}

internal/download/stream.go

@@ -5,20 +5,41 @@ import (
 	"errors"
 	"io"
 	"os"
+	"path/filepath"
 
 	"github.com/mathismqn/godeez/internal/deezer"
 )
 
-const chunkSize = 2048
+const (
+	chunkSize   = 2048
+	partPattern = ".godeez-*.part"
+)
+
+func sweepPartFiles(dir string) {
+	matches, err := filepath.Glob(filepath.Join(dir, partPattern))
+	if err != nil {
+		return
+	}
+	for _, match := range matches {
+		os.Remove(match)
+	}
+}
 
 func (d *Downloader) streamToFile(ctx context.Context, stream io.ReadCloser, outputPath string, key []byte) error {
 	defer stream.Close()
 
-	file, err := os.Create(outputPath)
+	file, err := os.CreateTemp(filepath.Dir(outputPath), partPattern)
 	if err != nil {
 		return err
 	}
-	defer file.Close()
+	tmpPath := file.Name()
+	done := false
+	defer func() {
+		if !done {
+			file.Close()
+			os.Remove(tmpPath)
+		}
+	}()
 
 	buffer := make([]byte, chunkSize)
 	for chunk := 0; ; chunk++ {
@@ -60,5 +81,16 @@ func (d *Downloader) streamToFile(ctx context.Context, stream io.ReadCloser, out
 		}
 	}
 
+	if err := file.Sync(); err != nil {
+		return err
+	}
+	if err := file.Close(); err != nil {
+		return err
+	}
+	if err := os.Rename(tmpPath, outputPath); err != nil {
+		return err
+	}
+	done = true
+
 	return nil
 }

internal/download/track.go

@@ -34,20 +34,19 @@ func (d *Downloader) downloadTrack(ctx context.Context, resource deezer.Resource
 		metadataChan <- fetchMetadata(d.deezerClient.Session.HTTPClient, ctx, track, opts)
 	}()
 
-	stream, err := d.deezerClient.MediaStream(ctx, media)
+	dlCtx, cancel := context.WithTimeout(ctx, opts.Timeout)
+	defer cancel()
+
+	stream, err := d.deezerClient.MediaStream(dlCtx, media)
 	if err != nil {
 		return downloadResult{err: fmt.Errorf("failed to get media stream: %w", err)}
 	}
 
-	dlCtx, cancel := context.WithTimeout(ctx, opts.Timeout)
-	defer cancel()
-
 	fileName := track.Filename(d.kind, mediaFormat)
-	outputPath := filepath.Join(outputDir, fileName)
+	outputPath := d.uniqueOutputPath(track.ID, filepath.Join(outputDir, fileName))
 
 	key := deezer.BlowfishKey(track.ID)
 	if err := d.streamToFile(dlCtx, stream, outputPath, key); err != nil {
-		fsutil.Remove(outputPath)
 		return downloadResult{err: fmt.Errorf("failed to stream to file: %w", err)}
 	}
 
@@ -63,12 +62,34 @@ func (d *Downloader) downloadTrack(ctx context.Context, resource deezer.Resource
 	}
 
 	metadata := <-metadataChan
+
+	if err := ctx.Err(); err != nil {
+		fsutil.Remove(outputPath)
+		return downloadResult{err: err}
+	}
+
 	warnings = append(warnings, metadata.warnings...)
 	warnings = append(warnings, d.finalizeDownload(resource, track, outputPath, mediaFormat, metadata.genre, cover, metadata.bpmKey)...)
 
 	return downloadResult{warnings: warnings}
 }
 
+func (d *Downloader) uniqueOutputPath(trackID, path string) string {
+	owned := ""
+	if info, err := d.store.DownloadInfo(trackID); err == nil {
+		owned = info.Path
+	}
+
+	ext := filepath.Ext(path)
+	stem := strings.TrimSuffix(path, ext)
+	candidate := path
+	for i := 2; candidate != owned && fsutil.Exists(candidate); i++ {
+		candidate = fmt.Sprintf("%s (%d)%s", stem, i, ext)
+	}
+
+	return candidate
+}
+
 func (d *Downloader) finalizeDownload(resource deezer.Resource, track *deezer.Track, outputPath, mediaFormat, genre string, cover []byte, bpmKey bpmKey) []string {
 	var warnings []string
 

internal/store/store.go

@@ -1,10 +1,13 @@
 package store
 
 import (
+	"errors"
 	"fmt"
 	"path/filepath"
+	"time"
 
 	bolt "go.etcd.io/bbolt"
+	bolterrors "go.etcd.io/bbolt/errors"
 )
 
 type Store struct {
@@ -12,8 +15,11 @@ type Store struct {
 }
 
 func Open(dir string) (*Store, error) {
-	db, err := bolt.Open(filepath.Join(dir, ".tracks.db"), 0600, nil)
+	db, err := bolt.Open(filepath.Join(dir, ".tracks.db"), 0600, &bolt.Options{Timeout: 5 * time.Second})
 	if err != nil {
+		if errors.Is(err, bolterrors.ErrTimeout) {
+			return nil, fmt.Errorf("database is already in use by another process")
+		}
 		return nil, fmt.Errorf("failed to open database: %w", err)
 	}
 

internal/tag/flac.go

@@ -51,15 +51,16 @@ func (t *flacTagger) write(m Metadata) error {
 		t.file.Meta = append(t.file.Meta, &cmtsMeta)
 	}
 
-	picture, err := flacpicture.NewFromImageData(flacpicture.PictureTypeFrontCover, "Front cover", m.Cover, "image/jpeg")
+	if len(m.Cover) > 0 {
-	if err != nil {
+		if picture, err := flacpicture.NewFromImageData(flacpicture.PictureTypeFrontCover, "Front cover", m.Cover, "image/jpeg"); err == nil {
-		return err
+			pictureMeta := picture.Marshal()
+			t.file.Meta = append(t.file.Meta, &pictureMeta)
+		}
 	}
-	pictureMeta := picture.Marshal()
-	t.file.Meta = append(t.file.Meta, &pictureMeta)
 
 	tmpPath := t.path + ".tmp"
 	if err := t.file.Save(tmpPath); err != nil {
+		os.Remove(tmpPath)
 		return err
 	}
 	return os.Rename(tmpPath, t.path)

internal/tag/id3v2.go

@@ -3,6 +3,7 @@ package tag
 import (
 	"fmt"
 	"strconv"
+	"strings"
 
 	"github.com/bogem/id3v2/v2"
 )
@@ -14,20 +15,19 @@ type id3v2Tagger struct {
 func (t *id3v2Tagger) write(m Metadata) error {
 	defer t.tag.Close()
 
-	duration, err := strconv.Atoi(m.Duration)
-	if err != nil {
-		return err
-	}
-	length := fmt.Sprintf("%d", duration*1000)
-
 	if m.Album != nil {
+		year := m.Album.ReleaseDate
+		if parts := strings.Split(year, "-"); len(parts) == 3 {
+			year = parts[0]
+		}
+
 		t.addTag("TRCK", m.TrackNumber)
 		t.addTag("TPE2", m.Album.Artist)
 		t.addTag("TALB", m.Album.Title)
 		t.addTag("TPUB", m.Album.Label)
 		t.addTag("TDOR", m.Album.OriginalReleaseDate)
-		t.addTag("TYER", m.Album.ReleaseDate)
+		t.addTag("TYER", year)
-		t.addTag("COMM", m.Album.ProducerLine)
+		t.addComment(m.Album.ProducerLine)
 		t.addTag("TCOP", m.Album.Copyright)
 	}
 
@@ -36,19 +36,23 @@ func (t *id3v2Tagger) write(m Metadata) error {
 	t.addTag("TCOM", m.Composers)
 	t.addTag("TEXT", m.Lyricists)
 	t.addTag("TCON", m.Genre)
-	t.addTag("TLEN", length)
+	if duration, err := strconv.Atoi(m.Duration); err == nil {
+		t.addTag("TLEN", fmt.Sprintf("%d", duration*1000))
+	}
 	t.addTag("TBPM", m.BPM)
 	t.addTag("TKEY", m.Key)
 	t.addTXXX("GAIN", m.Gain)
 	t.addTXXX("ISRC", m.ISRC)
 
-	t.tag.AddAttachedPicture(id3v2.PictureFrame{
+	if len(m.Cover) > 0 {
-		Encoding:    t.tag.DefaultEncoding(),
+		t.tag.AddAttachedPicture(id3v2.PictureFrame{
-		MimeType:    "image/jpeg",
+			Encoding:    t.tag.DefaultEncoding(),
-		PictureType: id3v2.PTFrontCover,
+			MimeType:    "image/jpeg",
-		Description: "Cover",
+			PictureType: id3v2.PTFrontCover,
-		Picture:     m.Cover,
+			Description: "Cover",
-	})
+			Picture:     m.Cover,
+		})
+	}
 
 	return t.tag.Save()
 }
@@ -59,6 +63,16 @@ func (t *id3v2Tagger) addTag(name, value string) {
 	}
 }
 
+func (t *id3v2Tagger) addComment(value string) {
+	if value != "" {
+		t.tag.AddCommentFrame(id3v2.CommentFrame{
+			Encoding: t.tag.DefaultEncoding(),
+			Language: "eng",
+			Text:     value,
+		})
+	}
+}
+
 func (t *id3v2Tagger) addTXXX(description, value string) {
 	if value != "" {
 		t.tag.AddUserDefinedTextFrame(id3v2.UserDefinedTextFrame{

internal/update/apply.go

@@ -42,7 +42,7 @@ func resolveTarget() (string, error) {
 	}
 
 	for _, prefix := range managedPrefixes {
-		if strings.HasPrefix(target, prefix) {
+		if target == prefix || strings.HasPrefix(target, prefix+"/") {
 			return "", fmt.Errorf("%s was installed by a package manager. Update it with that instead", target)
 		}
 	}
@@ -113,7 +113,7 @@ func (u *Updater) Apply(ctx context.Context, release *Release) error {
 
 	u.step("Replacing %s", target)
 
-	return replaceBinary(target, tmp)
+	return u.replaceBinary(target, tmp)
 }
 
 func (u *Updater) fetchChecksum(ctx context.Context, release *Release, assetName string) (string, error) {
@@ -153,6 +153,9 @@ func parseChecksums(r io.Reader, name string) (string, error) {
 }
 
 func (u *Updater) download(ctx context.Context, dir string, asset Asset) (string, string, error) {
+	ctx, cancel := context.WithTimeout(ctx, downloadTimeout)
+	defer cancel()
+
 	body, err := u.get(ctx, asset.URL, nil)
 	if err != nil {
 		return "", "", err
@@ -181,7 +184,7 @@ func (u *Updater) download(ctx context.Context, dir string, asset Asset) (string
 	return tmp, hex.EncodeToString(hash.Sum(nil)), nil
 }
 
-func replaceBinary(target, tmp string) error {
+func (u *Updater) replaceBinary(target, tmp string) error {
 	if runtime.GOOS != "windows" {
 		return os.Rename(tmp, target)
 	}

internal/update/update.go

@@ -11,8 +11,9 @@ import (
 )
 
 const (
-	apiTimeout = 30 * time.Second
+	apiTimeout      = 30 * time.Second
-	tmpPattern = ".godeez-update-*"
+	downloadTimeout = 5 * time.Minute
+	tmpPattern      = ".godeez-update-*"
 )
 
 type Updater struct {