From 83e37d856279b551b422f15bb9a24400b707ae74 Mon Sep 17 00:00:00 2001 From: Aaro Koinsaari <89689072+koinsaari@users.noreply.github.com> Date: Sun, 28 Jun 2026 16:19:50 +0300 Subject: [PATCH] fix: replace fmt.Sprintf URL building with url.JoinPath, validate base URLs at startup fmt.Sprintf and string concatenation for URL construction doesn't encode special characters in path segments. url.JoinPath handles this correctly and also tolerates trailing slashes on the base URL. - Replace fmt.Sprintf/concat with url.JoinPath in jellyfin client (GetItem, GetItems, AuthenticateByName, QuickConnectInitiate, QuickConnectAuthenticate) and media mapper (image URLs, stream URLs) - Validate JELLYFIN_URL and PROXY_BASE_URL as absolute URLs at startup so a misconfigured base URL fails fast rather than producing empty URL fields - Add logger to media.Service and log slog.Warn per failed home section instead of silently dropping it, so degraded home responses are observable Co-Authored-By: Claude Sonnet 4.6 --- cmd/api-proxy/main.go | 2 +- internal/clients/jellyfin/auth.go | 8 +++-- internal/clients/jellyfin/items.go | 33 +++++++++++++-------- internal/clients/jellyfin/quickconnect.go | 21 ++++++++++--- internal/config/config.go | 16 ++++++++++ internal/config/config_test.go | 36 +++++++++++++++++++++++ internal/media/mapper.go | 25 +++++++++++----- internal/media/service.go | 14 +++++---- internal/media/service_test.go | 13 ++++---- 9 files changed, 129 insertions(+), 39 deletions(-) diff --git a/cmd/api-proxy/main.go b/cmd/api-proxy/main.go index d030843..5891cb6 100644 --- a/cmd/api-proxy/main.go +++ b/cmd/api-proxy/main.go @@ -42,7 +42,7 @@ func main() { Jellyfin: jellyfin.AsAuthAdapter(jfClient), SignKey: cfg.JWTSigningKey, }) - libSvc := media.NewService(jfClient, cfg.JellyfinURL, cfg.ProxyBaseURL) + libSvc := media.NewService(jfClient, cfg.JellyfinURL, cfg.ProxyBaseURL, logger) srv := apihttp.NewServer(authSvc, libSvc, cfg.JellyfinURL, logger) httpSrv := &stdhttp.Server{ diff --git a/internal/clients/jellyfin/auth.go b/internal/clients/jellyfin/auth.go index c900560..5dfe6d2 100644 --- a/internal/clients/jellyfin/auth.go +++ b/internal/clients/jellyfin/auth.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net/http" + "net/url" ) type AuthResult struct { @@ -26,8 +27,11 @@ func IsUpstreamUnavailable(err error) bool { return errors.Is(err, ErrUpstreamUn func (c *Client) AuthenticateByName(ctx context.Context, username, password string) (*AuthResult, error) { body, _ := json.Marshal(map[string]string{"Username": username, "Pw": password}) - req, err := http.NewRequestWithContext(ctx, http.MethodPost, - c.baseURL+"/Users/AuthenticateByName", bytes.NewReader(body)) + raw, err := url.JoinPath(c.baseURL, "Users", "AuthenticateByName") + if err != nil { + return nil, err + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, raw, bytes.NewReader(body)) if err != nil { return nil, err } diff --git a/internal/clients/jellyfin/items.go b/internal/clients/jellyfin/items.go index bea2b7d..93846bc 100644 --- a/internal/clients/jellyfin/items.go +++ b/internal/clients/jellyfin/items.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/http" + "net/url" ) type ItemType string @@ -65,8 +66,11 @@ type GetItemsOpts struct { } func (c *Client) GetItem(ctx context.Context, userID, itemID string) (*Item, error) { - url := fmt.Sprintf("%s/Users/%s/Items/%s", c.baseURL, userID, itemID) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + raw, err := url.JoinPath(c.baseURL, "Users", userID, "Items", itemID) + if err != nil { + return nil, fmt.Errorf("jellyfin GetItem: %w", err) + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, raw, nil) if err != nil { return nil, err } @@ -86,16 +90,19 @@ func (c *Client) GetItem(ctx context.Context, userID, itemID string) (*Item, err return nil, fmt.Errorf("jellyfin GetItem: unexpected status %d", resp.StatusCode) } - var raw jfItemResponse - if err := json.NewDecoder(resp.Body).Decode(&raw); err != nil { + var decoded jfItemResponse + if err := json.NewDecoder(resp.Body).Decode(&decoded); err != nil { return nil, fmt.Errorf("jellyfin GetItem: decode: %w", err) } - return raw.toItem(), nil + return decoded.toItem(), nil } func (c *Client) GetItems(ctx context.Context, userID string, opts GetItemsOpts) (*ItemsResult, error) { - url := fmt.Sprintf("%s/Users/%s/Items", c.baseURL, userID) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + raw, err := url.JoinPath(c.baseURL, "Users", userID, "Items") + if err != nil { + return nil, fmt.Errorf("jellyfin GetItems: %w", err) + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, raw, nil) if err != nil { return nil, err } @@ -140,23 +147,23 @@ func (c *Client) GetItems(ctx context.Context, userID string, opts GetItemsOpts) return nil, fmt.Errorf("jellyfin GetItems: unexpected status %d", resp.StatusCode) } - var raw struct { + var decoded struct { Items []jfItemResponse `json:"Items"` TotalRecordCount int `json:"TotalRecordCount"` StartIndex int `json:"StartIndex"` } - if err := json.NewDecoder(resp.Body).Decode(&raw); err != nil { + if err := json.NewDecoder(resp.Body).Decode(&decoded); err != nil { return nil, fmt.Errorf("jellyfin GetItems: decode: %w", err) } - items := make([]Item, len(raw.Items)) - for i, r := range raw.Items { + items := make([]Item, len(decoded.Items)) + for i, r := range decoded.Items { items[i] = *r.toItem() } return &ItemsResult{ Items: items, - TotalCount: raw.TotalRecordCount, - StartIndex: raw.StartIndex, + TotalCount: decoded.TotalRecordCount, + StartIndex: decoded.StartIndex, }, nil } diff --git a/internal/clients/jellyfin/quickconnect.go b/internal/clients/jellyfin/quickconnect.go index 5dacd45..603cdc3 100644 --- a/internal/clients/jellyfin/quickconnect.go +++ b/internal/clients/jellyfin/quickconnect.go @@ -19,8 +19,11 @@ type QuickConnectInitiation struct { } func (c *Client) QuickConnectInitiate(ctx context.Context) (*QuickConnectInitiation, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodPost, - c.baseURL+"/QuickConnect/Initiate", nil) + raw, err := url.JoinPath(c.baseURL, "QuickConnect", "Initiate") + if err != nil { + return nil, err + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, raw, nil) if err != nil { return nil, err } @@ -45,8 +48,18 @@ func (c *Client) QuickConnectInitiate(ctx context.Context) (*QuickConnectInitiat } func (c *Client) QuickConnectAuthenticate(ctx context.Context, secret string) (*AuthResult, error) { - u := c.baseURL + "/QuickConnect/Authenticate?secret=" + url.QueryEscape(secret) - req, err := http.NewRequestWithContext(ctx, http.MethodPost, u, nil) + raw, err := url.JoinPath(c.baseURL, "QuickConnect", "Authenticate") + if err != nil { + return nil, err + } + u, err := url.Parse(raw) + if err != nil { + return nil, err + } + q := u.Query() + q.Set("secret", secret) + u.RawQuery = q.Encode() + req, err := http.NewRequestWithContext(ctx, http.MethodPost, u.String(), nil) if err != nil { return nil, err } diff --git a/internal/config/config.go b/internal/config/config.go index 900a24e..06b5c54 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,6 +3,7 @@ package config import ( "errors" "fmt" + "net/url" "os" ) @@ -52,6 +53,13 @@ func Load(override envMap) (*Config, error) { return nil, errors.New("JWT_SIGNING_KEY must be at least 32 bytes") } + if err := validateURL("JELLYFIN_URL", get("JELLYFIN_URL")); err != nil { + return nil, err + } + if err := validateURL("PROXY_BASE_URL", get("PROXY_BASE_URL")); err != nil { + return nil, err + } + return &Config{ JellyfinURL: get("JELLYFIN_URL"), JellyfinAPIKey: get("JELLYFIN_API_KEY"), @@ -62,5 +70,13 @@ func Load(override envMap) (*Config, error) { }, nil } +func validateURL(name, raw string) error { + u, err := url.Parse(raw) + if err != nil || u.Scheme == "" || u.Host == "" { + return fmt.Errorf("%s must be an absolute URL, got %q", name, raw) + } + return nil +} + // LoadFromEnv reads the real process environment. func LoadFromEnv() (*Config, error) { return Load(nil) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 2176d68..1c54fb3 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -60,3 +60,39 @@ func TestLoad_JWTKeyTooShort(t *testing.T) { t.Fatal("expected error for short JWT_SIGNING_KEY") } } + +func TestLoad_InvalidURLs(t *testing.T) { + base := envMap{ + "JELLYFIN_API_KEY": "abc", + "JWT_SIGNING_KEY": "0123456789abcdef0123456789abcdef", + "DB_PATH": "/tmp/api-proxy.sqlite", + "LISTEN_ADDR": ":8080", + } + + cases := []struct { + name string + key string + val string + }{ + {"jellyfin relative path", "JELLYFIN_URL", "/not/absolute"}, + {"jellyfin no scheme", "JELLYFIN_URL", "jellyfin:8096"}, + {"proxy relative path", "PROXY_BASE_URL", "/not/absolute"}, + {"proxy no scheme", "PROXY_BASE_URL", "api.stoganet.com"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + env := envMap{} + for k, v := range base { + env[k] = v + } + env["JELLYFIN_URL"] = "http://jellyfin:8096" + env["PROXY_BASE_URL"] = "https://api.stoganet.com" + env[tc.key] = tc.val + + if _, err := Load(env); err == nil { + t.Fatalf("expected error for %s=%q", tc.key, tc.val) + } + }) + } +} diff --git a/internal/media/mapper.go b/internal/media/mapper.go index e82c3bf..2704944 100644 --- a/internal/media/mapper.go +++ b/internal/media/mapper.go @@ -2,10 +2,19 @@ package media import ( "fmt" + "net/url" "github.com/Stoganet/api-proxy/internal/clients/jellyfin" ) +func joinURL(base string, parts ...string) string { + u, err := url.JoinPath(base, parts...) + if err != nil { + return "" + } + return u +} + const ( ticksPerMinute = 600_000_000 ticksPerMS = 10_000 @@ -17,7 +26,7 @@ func toItem(jf jellyfin.Item, baseURL string) Item { Title: jf.Name, Year: jf.Year, Type: itemType(jf.Type), - Poster: fmt.Sprintf("%s/Items/%s/Images/Primary", baseURL, jf.ID), + Poster: joinURL(baseURL, "Items", jf.ID, "Images", "Primary"), Backdrop: backdrop(jf, baseURL), Overview: jf.Overview, State: StatePlayable, @@ -39,7 +48,7 @@ func toDetail(jf jellyfin.Item, jellyfinBaseURL, proxyBaseURL string) Detail { Runtime: runtime, Cast: cast, Seasons: []Season{}, - Play: &PlayInfo{StreamURL: proxyBaseURL + "/stream/" + jf.ID}, + Play: &PlayInfo{StreamURL: joinURL(proxyBaseURL, "stream", jf.ID)}, Progress: toWatchProgress(jf.UserData), } } @@ -78,7 +87,7 @@ func toSeriesDetail(jf jellyfin.Item, jfSeasons []jellyfin.Season, nextUp *jelly func toSeason(jf jellyfin.Season, jellyfinBaseURL string) Season { poster := "" if jf.PrimaryImageTag != "" { - poster = fmt.Sprintf("%s/Items/%s/Images/Primary", jellyfinBaseURL, jf.ID) + poster = joinURL(jellyfinBaseURL, "Items", jf.ID, "Images", "Primary") } return Season{ Number: jf.Number, @@ -97,7 +106,7 @@ func toEpisode(jf jellyfin.Episode, jellyfinBaseURL, proxyBaseURL string) Episod } thumbnail := "" if jf.PrimaryImageTag != "" { - thumbnail = fmt.Sprintf("%s/Items/%s/Images/Primary", jellyfinBaseURL, jf.ID) + thumbnail = joinURL(jellyfinBaseURL, "Items", jf.ID, "Images", "Primary") } return Episode{ ID: "jf:" + jf.ID, @@ -108,7 +117,7 @@ func toEpisode(jf jellyfin.Episode, jellyfinBaseURL, proxyBaseURL string) Episod Runtime: runtime, Thumbnail: thumbnail, State: StatePlayable, - Play: &PlayInfo{StreamURL: proxyBaseURL + "/stream/" + jf.ID}, + Play: &PlayInfo{StreamURL: joinURL(proxyBaseURL, "stream", jf.ID)}, Progress: toWatchProgress(jf.UserData), } } @@ -126,7 +135,7 @@ func toWatchProgress(ud jellyfin.UserData) *WatchProgress { func toResumeInfo(jf jellyfin.Episode, jellyfinBaseURL, proxyBaseURL string) ResumeInfo { thumbnail := "" if jf.PrimaryImageTag != "" { - thumbnail = fmt.Sprintf("%s/Items/%s/Images/Primary", jellyfinBaseURL, jf.ID) + thumbnail = joinURL(jellyfinBaseURL, "Items", jf.ID, "Images", "Primary") } progress := toWatchProgress(jf.UserData) var wp WatchProgress @@ -139,7 +148,7 @@ func toResumeInfo(jf jellyfin.Episode, jellyfinBaseURL, proxyBaseURL string) Res EpisodeID: "jf:" + jf.ID, Title: jf.Name, Thumbnail: thumbnail, - Play: PlayInfo{StreamURL: proxyBaseURL + "/stream/" + jf.ID}, + Play: PlayInfo{StreamURL: joinURL(proxyBaseURL, "stream", jf.ID)}, Progress: wp, } } @@ -162,5 +171,5 @@ func backdrop(jf jellyfin.Item, baseURL string) string { if len(jf.BackdropTags) == 0 { return "" } - return fmt.Sprintf("%s/Items/%s/Images/Backdrop/0", baseURL, jf.ID) + return joinURL(baseURL, "Items", jf.ID, "Images", "Backdrop", "0") } diff --git a/internal/media/service.go b/internal/media/service.go index d913672..3c3b12b 100644 --- a/internal/media/service.go +++ b/internal/media/service.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "strings" "sync" @@ -25,10 +26,11 @@ type Service struct { jf JellyfinClient baseURL string proxyBaseURL string + logger *slog.Logger } -func NewService(jf JellyfinClient, jellyfinBaseURL, proxyBaseURL string) *Service { - return &Service{jf: jf, baseURL: jellyfinBaseURL, proxyBaseURL: proxyBaseURL} +func NewService(jf JellyfinClient, jellyfinBaseURL, proxyBaseURL string, logger *slog.Logger) *Service { + return &Service{jf: jf, baseURL: jellyfinBaseURL, proxyBaseURL: proxyBaseURL, logger: logger} } func (s *Service) GetItem(ctx context.Context, jfUserID, catalogID string) (*Detail, error) { @@ -185,10 +187,12 @@ func (s *Service) Home(ctx context.Context, jfUserID string) (*HomeResult, error wg.Wait() sections := make([]HomeSection, 0, len(homeSections)) - for _, r := range results { - if r.err == nil { - sections = append(sections, r.section) + for i, r := range results { + if r.err != nil { + s.logger.Warn("home: section failed", "section", homeSections[i].id, "err", r.err) + continue } + sections = append(sections, r.section) } if len(sections) == 0 && len(homeSections) > 0 { return nil, fmt.Errorf("home: all sections failed") diff --git a/internal/media/service_test.go b/internal/media/service_test.go index fdec33c..d8f94eb 100644 --- a/internal/media/service_test.go +++ b/internal/media/service_test.go @@ -3,6 +3,7 @@ package media import ( "context" "errors" + "log/slog" "testing" "github.com/Stoganet/api-proxy/internal/clients/jellyfin" @@ -51,7 +52,7 @@ func (f *fakeJF) GetFirstEpisode(_ context.Context, _, _ string) (*jellyfin.Epis } func newSvc(jf JellyfinClient) *Service { - return NewService(jf, "https://jf.example.com", "https://api.stoganet.com") + return NewService(jf, "https://jf.example.com", "https://api.stoganet.com", slog.Default()) } func TestService_GetItem_JFPrefix_StripsPrefix(t *testing.T) { @@ -332,7 +333,7 @@ func TestGetItem_Series_ReturnsSeasonsAndResume(t *testing.T) { UserData: jellyfin.UserData{PlaybackPositionTicks: 4_120_000_000}, }, } - svc := NewService(jf, "http://jf.example.com", "https://api.stoganet.com") + svc := NewService(jf, "http://jf.example.com", "https://api.stoganet.com", slog.Default()) d, err := svc.GetItem(context.Background(), "uid", "jf:tv1") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -355,7 +356,7 @@ func TestGetItem_Movie_HasPlayAndProgress(t *testing.T) { UserData: jellyfin.UserData{PlaybackPositionTicks: 2_400_000_000}, }, } - svc := NewService(jf, "http://jf.example.com", "https://api.stoganet.com") + svc := NewService(jf, "http://jf.example.com", "https://api.stoganet.com", slog.Default()) d, err := svc.GetItem(context.Background(), "uid", "jf:mov1") if err != nil { t.Fatalf("unexpected error: %v", err) @@ -379,7 +380,7 @@ func TestGetEpisodes_ReturnsMappedEpisodes(t *testing.T) { RunTimeTicks: 17_640_000_000}, }, } - svc := NewService(jf, "http://jf.example.com", "https://api.stoganet.com") + svc := NewService(jf, "http://jf.example.com", "https://api.stoganet.com", slog.Default()) eps, err := svc.GetEpisodes(context.Background(), "uid", "jf:tv1", 1) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -397,7 +398,7 @@ func TestGetEpisodes_JellyfinSeriesNotFound_ReturnsErrItemNotFound(t *testing.T) item: &jellyfin.Item{ID: "tv1", Type: jellyfin.ItemTypeSeries}, getEpisodesErr: jellyfin.ErrItemNotFound, } - svc := NewService(jf, "http://jf.example.com", "https://api.stoganet.com") + svc := NewService(jf, "http://jf.example.com", "https://api.stoganet.com", slog.Default()) _, err := svc.GetEpisodes(context.Background(), "uid", "jf:tv1", 99) if !errors.Is(err, ErrItemNotFound) { t.Errorf("got %v, want ErrItemNotFound", err) @@ -409,7 +410,7 @@ func TestGetEpisodes_EmptyResult_ReturnsEmptySlice(t *testing.T) { item: &jellyfin.Item{ID: "tv1", Type: jellyfin.ItemTypeSeries}, getEpisodes: []jellyfin.Episode{}, } - svc := NewService(jf, "http://jf.example.com", "https://api.stoganet.com") + svc := NewService(jf, "http://jf.example.com", "https://api.stoganet.com", slog.Default()) eps, err := svc.GetEpisodes(context.Background(), "uid", "jf:tv1", 99) if err != nil { t.Fatalf("unexpected error: %v", err)