diff --git a/core/transcode/transcode.go b/core/transcode/transcode.go index acee439bb..d9bc41018 100644 --- a/core/transcode/transcode.go +++ b/core/transcode/transcode.go @@ -29,7 +29,8 @@ type deciderService struct { func (s *deciderService) MakeDecision(ctx context.Context, mf *model.MediaFile, clientInfo *ClientInfo) (*Decision, error) { decision := &Decision{ - MediaID: mf.ID, + MediaID: mf.ID, + SourceUpdatedAt: mf.UpdatedAt, } sourceBitrate := mf.BitRate // kbps @@ -300,6 +301,7 @@ func (s *deciderService) CreateTranscodeParams(decision *Decision) (string, erro claims := map[string]any{ "mid": decision.MediaID, "dp": decision.CanDirectPlay, + "ua": decision.SourceUpdatedAt.Truncate(time.Second).Unix(), } if decision.CanTranscode && decision.TargetFormat != "" { claims["fmt"] = decision.TargetFormat @@ -355,5 +357,34 @@ func (s *deciderService) ParseTranscodeParams(token string) (*Params, error) { params.TargetBitDepth = int(bd) } + ua, ok := claims["ua"].(float64) + if !ok { + return nil, fmt.Errorf("invalid transcode token: missing source timestamp") + } + params.SourceUpdatedAt = time.Unix(int64(ua), 0) + return params, nil } + +func (s *deciderService) ValidateTranscodeParams(ctx context.Context, token string, mediaID string) (*Params, *model.MediaFile, error) { + params, err := s.ParseTranscodeParams(token) + if err != nil { + return nil, nil, errors.Join(ErrTokenInvalid, err) + } + if params.MediaID != mediaID { + return nil, nil, fmt.Errorf("%w: token mediaID %q does not match %q", ErrTokenInvalid, params.MediaID, mediaID) + } + mf, err := s.ds.MediaFile(ctx).Get(mediaID) + if err != nil { + if errors.Is(err, model.ErrNotFound) { + return nil, nil, ErrMediaNotFound + } + return nil, nil, err + } + if !mf.UpdatedAt.Truncate(time.Second).Equal(params.SourceUpdatedAt) { + log.Info(ctx, "Transcode token is stale", "mediaID", mediaID, + "tokenUpdatedAt", params.SourceUpdatedAt, "fileUpdatedAt", mf.UpdatedAt) + return nil, nil, ErrTokenStale + } + return params, mf, nil +} diff --git a/core/transcode/transcode_test.go b/core/transcode/transcode_test.go index e7a0e635e..b4f0f112e 100644 --- a/core/transcode/transcode_test.go +++ b/core/transcode/transcode_test.go @@ -2,6 +2,7 @@ package transcode import ( "context" + "time" "github.com/navidrome/navidrome/core/auth" "github.com/navidrome/navidrome/model" @@ -856,10 +857,17 @@ var _ = Describe("Decider", func() { }) Describe("Token round-trip", func() { + var sourceTime time.Time + + BeforeEach(func() { + sourceTime = time.Date(2025, 6, 15, 10, 30, 0, 0, time.UTC) + }) + It("creates and parses a direct play token", func() { decision := &Decision{ - MediaID: "media-123", - CanDirectPlay: true, + MediaID: "media-123", + CanDirectPlay: true, + SourceUpdatedAt: sourceTime, } token, err := svc.CreateTranscodeParams(decision) Expect(err).ToNot(HaveOccurred()) @@ -870,16 +878,18 @@ var _ = Describe("Decider", func() { Expect(params.MediaID).To(Equal("media-123")) Expect(params.DirectPlay).To(BeTrue()) Expect(params.TargetFormat).To(BeEmpty()) + Expect(params.SourceUpdatedAt.Unix()).To(Equal(sourceTime.Unix())) }) It("creates and parses a transcode token with kbps bitrate", func() { decision := &Decision{ - MediaID: "media-456", - CanDirectPlay: false, - CanTranscode: true, - TargetFormat: "mp3", - TargetBitrate: 256, // kbps - TargetChannels: 2, + MediaID: "media-456", + CanDirectPlay: false, + CanTranscode: true, + TargetFormat: "mp3", + TargetBitrate: 256, // kbps + TargetChannels: 2, + SourceUpdatedAt: sourceTime, } token, err := svc.CreateTranscodeParams(decision) Expect(err).ToNot(HaveOccurred()) @@ -891,6 +901,7 @@ var _ = Describe("Decider", func() { Expect(params.TargetFormat).To(Equal("mp3")) Expect(params.TargetBitrate).To(Equal(256)) // kbps Expect(params.TargetChannels).To(Equal(2)) + Expect(params.SourceUpdatedAt.Unix()).To(Equal(sourceTime.Unix())) }) It("creates and parses a transcode token with sample rate", func() { @@ -902,6 +913,7 @@ var _ = Describe("Decider", func() { TargetBitrate: 0, TargetChannels: 2, TargetSampleRate: 48000, + SourceUpdatedAt: sourceTime, } token, err := svc.CreateTranscodeParams(decision) Expect(err).ToNot(HaveOccurred()) @@ -917,13 +929,14 @@ var _ = Describe("Decider", func() { It("creates and parses a transcode token with bit depth", func() { decision := &Decision{ - MediaID: "media-bd", - CanDirectPlay: false, - CanTranscode: true, - TargetFormat: "flac", - TargetBitrate: 0, - TargetChannels: 2, - TargetBitDepth: 24, + MediaID: "media-bd", + CanDirectPlay: false, + CanTranscode: true, + TargetFormat: "flac", + TargetBitrate: 0, + TargetChannels: 2, + TargetBitDepth: 24, + SourceUpdatedAt: sourceTime, } token, err := svc.CreateTranscodeParams(decision) Expect(err).ToNot(HaveOccurred()) @@ -936,12 +949,13 @@ var _ = Describe("Decider", func() { It("omits bit depth from token when 0", func() { decision := &Decision{ - MediaID: "media-nobd", - CanDirectPlay: false, - CanTranscode: true, - TargetFormat: "mp3", - TargetBitrate: 256, - TargetBitDepth: 0, + MediaID: "media-nobd", + CanDirectPlay: false, + CanTranscode: true, + TargetFormat: "mp3", + TargetBitrate: 256, + TargetBitDepth: 0, + SourceUpdatedAt: sourceTime, } token, err := svc.CreateTranscodeParams(decision) Expect(err).ToNot(HaveOccurred()) @@ -959,6 +973,7 @@ var _ = Describe("Decider", func() { TargetFormat: "mp3", TargetBitrate: 256, TargetSampleRate: 0, + SourceUpdatedAt: sourceTime, } token, err := svc.CreateTranscodeParams(decision) Expect(err).ToNot(HaveOccurred()) @@ -968,9 +983,91 @@ var _ = Describe("Decider", func() { Expect(params.TargetSampleRate).To(Equal(0)) }) + It("truncates SourceUpdatedAt to seconds", func() { + timeWithNanos := time.Date(2025, 6, 15, 10, 30, 0, 123456789, time.UTC) + decision := &Decision{ + MediaID: "media-trunc", + CanDirectPlay: true, + SourceUpdatedAt: timeWithNanos, + } + token, err := svc.CreateTranscodeParams(decision) + Expect(err).ToNot(HaveOccurred()) + + params, err := svc.ParseTranscodeParams(token) + Expect(err).ToNot(HaveOccurred()) + Expect(params.SourceUpdatedAt.Unix()).To(Equal(timeWithNanos.Truncate(time.Second).Unix())) + }) + It("rejects an invalid token", func() { _, err := svc.ParseTranscodeParams("invalid-token") Expect(err).To(HaveOccurred()) }) }) + + Describe("ValidateTranscodeParams", func() { + var ( + mockMFRepo *tests.MockMediaFileRepo + sourceTime time.Time + ) + + BeforeEach(func() { + sourceTime = time.Date(2025, 6, 15, 10, 30, 0, 0, time.UTC) + mockMFRepo = &tests.MockMediaFileRepo{} + ds.MockedMediaFile = mockMFRepo + }) + + createTokenForMedia := func(mediaID string, updatedAt time.Time) string { + decision := &Decision{ + MediaID: mediaID, + CanDirectPlay: true, + SourceUpdatedAt: updatedAt, + } + token, err := svc.CreateTranscodeParams(decision) + Expect(err).ToNot(HaveOccurred()) + return token + } + + It("returns params and media file for valid token", func() { + mockMFRepo.SetData(model.MediaFiles{ + {ID: "song-1", UpdatedAt: sourceTime}, + }) + token := createTokenForMedia("song-1", sourceTime) + + params, mf, err := svc.ValidateTranscodeParams(ctx, token, "song-1") + Expect(err).ToNot(HaveOccurred()) + Expect(params.MediaID).To(Equal("song-1")) + Expect(params.DirectPlay).To(BeTrue()) + Expect(mf.ID).To(Equal("song-1")) + }) + + It("returns ErrTokenInvalid for invalid token", func() { + _, _, err := svc.ValidateTranscodeParams(ctx, "bad-token", "song-1") + Expect(err).To(MatchError(ContainSubstring(ErrTokenInvalid.Error()))) + }) + + It("returns ErrTokenInvalid when mediaID does not match token", func() { + token := createTokenForMedia("song-1", sourceTime) + + _, _, err := svc.ValidateTranscodeParams(ctx, token, "song-2") + Expect(err).To(MatchError(ContainSubstring(ErrTokenInvalid.Error()))) + }) + + It("returns ErrMediaNotFound when media file does not exist", func() { + token := createTokenForMedia("gone-id", sourceTime) + + _, _, err := svc.ValidateTranscodeParams(ctx, token, "gone-id") + Expect(err).To(MatchError(ErrMediaNotFound)) + }) + + It("returns ErrTokenStale when media file has changed", func() { + newTime := sourceTime.Add(1 * time.Hour) + mockMFRepo.SetData(model.MediaFiles{ + {ID: "song-1", UpdatedAt: newTime}, + }) + token := createTokenForMedia("song-1", sourceTime) + + _, _, err := svc.ValidateTranscodeParams(ctx, token, "song-1") + Expect(err).To(MatchError(ErrTokenStale)) + }) + }) }) diff --git a/core/transcode/types.go b/core/transcode/types.go index 13344740c..f7f6be0f1 100644 --- a/core/transcode/types.go +++ b/core/transcode/types.go @@ -2,15 +2,24 @@ package transcode import ( "context" + "errors" + "time" "github.com/navidrome/navidrome/model" ) +var ( + ErrTokenInvalid = errors.New("invalid or expired transcode token") + ErrMediaNotFound = errors.New("media file not found") + ErrTokenStale = errors.New("transcode token is stale: media file has changed") +) + // Decider is the core service interface for making transcoding decisions type Decider interface { MakeDecision(ctx context.Context, mf *model.MediaFile, clientInfo *ClientInfo) (*Decision, error) CreateTranscodeParams(decision *Decision) (string, error) ParseTranscodeParams(token string) (*Params, error) + ValidateTranscodeParams(ctx context.Context, token string, mediaID string) (*Params, *model.MediaFile, error) } // ClientInfo represents client playback capabilities. @@ -98,6 +107,7 @@ type Decision struct { TargetSampleRate int TargetBitDepth int SourceStream StreamDetails + SourceUpdatedAt time.Time TranscodeStream *StreamDetails } @@ -126,4 +136,5 @@ type Params struct { TargetChannels int TargetSampleRate int TargetBitDepth int + SourceUpdatedAt time.Time } diff --git a/server/e2e/e2e_suite_test.go b/server/e2e/e2e_suite_test.go index 01efb4e6a..8447cbca3 100644 --- a/server/e2e/e2e_suite_test.go +++ b/server/e2e/e2e_suite_test.go @@ -213,6 +213,10 @@ func (n noopDecider) ParseTranscodeParams(string) (*transcode.Params, error) { return nil, nil } +func (n noopDecider) ValidateTranscodeParams(context.Context, string, string) (*transcode.Params, *model.MediaFile, error) { + return nil, nil, nil +} + // noopArchiver implements core.Archiver type noopArchiver struct{} diff --git a/server/subsonic/transcode.go b/server/subsonic/transcode.go index 020406c45..f7f251449 100644 --- a/server/subsonic/transcode.go +++ b/server/subsonic/transcode.go @@ -306,40 +306,48 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request) // GetTranscodeStream handles the OpenSubsonic getTranscodeStream endpoint. // It streams media using the decision encoded in the transcodeParams JWT token. +// All errors are returned as proper HTTP status codes (not Subsonic error responses). func (api *Router) GetTranscodeStream(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() p := req.Params(r) mediaID, err := p.String("mediaId") if err != nil { - return nil, newError(responses.ErrorMissingParameter, "missing required parameter: mediaId") + http.Error(w, "Bad Request", http.StatusBadRequest) + return nil, nil } mediaType, err := p.String("mediaType") if err != nil { - return nil, newError(responses.ErrorMissingParameter, "missing required parameter: mediaType") + http.Error(w, "Bad Request", http.StatusBadRequest) + return nil, nil } - transcodeParams, err := p.String("transcodeParams") + transcodeParamsToken, err := p.String("transcodeParams") if err != nil { - return nil, newError(responses.ErrorMissingParameter, "missing required parameter: transcodeParams") + http.Error(w, "Bad Request", http.StatusBadRequest) + return nil, nil } // Only support songs for now if mediaType != "song" { - return nil, newError(responses.ErrorGeneric, "mediaType '%s' is not yet supported", mediaType) + http.Error(w, "Bad Request", http.StatusBadRequest) + return nil, nil } - // Parse and validate the token - params, err := api.transcodeDecision.ParseTranscodeParams(transcodeParams) + // Validate the token, mediaID match, file existence, and freshness + params, mf, err := api.transcodeDecision.ValidateTranscodeParams(ctx, transcodeParamsToken, mediaID) if err != nil { - log.Warn(ctx, "Failed to parse transcode token", err) - return nil, newError(responses.ErrorDataNotFound, "invalid or expired transcodeParams token") - } - - // Verify mediaId matches token - if params.MediaID != mediaID { - return nil, newError(responses.ErrorDataNotFound, "mediaId does not match token") + switch { + case errors.Is(err, transcode.ErrMediaNotFound): + http.Error(w, "Not Found", http.StatusNotFound) + case errors.Is(err, transcode.ErrTokenInvalid), errors.Is(err, transcode.ErrTokenStale): + http.Error(w, "Gone", http.StatusGone) + default: + log.Error(ctx, "Error validating transcode params", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + } + return nil, nil } // Build streaming parameters from the token @@ -352,10 +360,12 @@ func (api *Router) GetTranscodeStream(w http.ResponseWriter, r *http.Request) (* streamReq.Channels = params.TargetChannels } - // Create stream - stream, err := api.streamer.NewStream(ctx, streamReq) + // Create stream (use DoStream to avoid duplicate DB fetch) + stream, err := api.streamer.DoStream(ctx, mf, streamReq) if err != nil { - return nil, err + log.Error(ctx, "Error creating stream", "mediaID", mediaID, err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return nil, nil } // Make sure the stream will be closed at the end diff --git a/server/subsonic/transcode_test.go b/server/subsonic/transcode_test.go index ede6d92fe..57c66e762 100644 --- a/server/subsonic/transcode_test.go +++ b/server/subsonic/transcode_test.go @@ -206,37 +206,63 @@ var _ = Describe("Transcode endpoints", func() { }) Describe("GetTranscodeStream", func() { - It("returns error when mediaId is missing", func() { + It("returns 400 when mediaId is missing", func() { r := newGetRequest("mediaType=song", "transcodeParams=abc") - _, err := router.GetTranscodeStream(w, r) - Expect(err).To(HaveOccurred()) + resp, err := router.GetTranscodeStream(w, r) + Expect(err).ToNot(HaveOccurred()) + Expect(resp).To(BeNil()) + Expect(w.Code).To(Equal(http.StatusBadRequest)) }) - It("returns error when transcodeParams is missing", func() { + It("returns 400 when transcodeParams is missing", func() { r := newGetRequest("mediaId=123", "mediaType=song") - _, err := router.GetTranscodeStream(w, r) - Expect(err).To(HaveOccurred()) + resp, err := router.GetTranscodeStream(w, r) + Expect(err).ToNot(HaveOccurred()) + Expect(resp).To(BeNil()) + Expect(w.Code).To(Equal(http.StatusBadRequest)) }) - It("returns error for invalid token", func() { - mockTD.parseErr = model.ErrNotFound + It("returns 410 for invalid token", func() { + mockTD.validateErr = transcode.ErrTokenInvalid r := newGetRequest("mediaId=123", "mediaType=song", "transcodeParams=bad-token") - _, err := router.GetTranscodeStream(w, r) - Expect(err).To(HaveOccurred()) + resp, err := router.GetTranscodeStream(w, r) + Expect(err).ToNot(HaveOccurred()) + Expect(resp).To(BeNil()) + Expect(w.Code).To(Equal(http.StatusGone)) }) - It("returns error when mediaId doesn't match token", func() { - mockTD.params = &transcode.Params{MediaID: "other-id", DirectPlay: true} + It("returns 410 when mediaId doesn't match token", func() { + mockTD.validateErr = transcode.ErrTokenInvalid r := newGetRequest("mediaId=wrong-id", "mediaType=song", "transcodeParams=valid-token") - _, err := router.GetTranscodeStream(w, r) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("does not match")) + resp, err := router.GetTranscodeStream(w, r) + Expect(err).ToNot(HaveOccurred()) + Expect(resp).To(BeNil()) + Expect(w.Code).To(Equal(http.StatusGone)) + }) + + It("returns 404 when media file not found", func() { + mockTD.validateErr = transcode.ErrMediaNotFound + r := newGetRequest("mediaId=gone-id", "mediaType=song", "transcodeParams=valid-token") + resp, err := router.GetTranscodeStream(w, r) + Expect(err).ToNot(HaveOccurred()) + Expect(resp).To(BeNil()) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) + + It("returns 410 when media file has changed (stale token)", func() { + mockTD.validateErr = transcode.ErrTokenStale + r := newGetRequest("mediaId=song-1", "mediaType=song", "transcodeParams=stale-token") + resp, err := router.GetTranscodeStream(w, r) + Expect(err).ToNot(HaveOccurred()) + Expect(resp).To(BeNil()) + Expect(w.Code).To(Equal(http.StatusGone)) }) It("builds correct StreamRequest for direct play", func() { fakeStreamer := &fakeMediaStreamer{} router = New(ds, nil, fakeStreamer, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, mockTD) - mockTD.params = &transcode.Params{MediaID: "song-1", DirectPlay: true} + mockTD.validateParams = &transcode.Params{MediaID: "song-1", DirectPlay: true} + mockTD.validateMF = &model.MediaFile{ID: "song-1"} r := newGetRequest("mediaId=song-1", "mediaType=song", "transcodeParams=valid-token") _, _ = router.GetTranscodeStream(w, r) @@ -253,7 +279,7 @@ var _ = Describe("Transcode endpoints", func() { It("builds correct StreamRequest for transcoding", func() { fakeStreamer := &fakeMediaStreamer{} router = New(ds, nil, fakeStreamer, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, mockTD) - mockTD.params = &transcode.Params{ + mockTD.validateParams = &transcode.Params{ MediaID: "song-2", DirectPlay: false, TargetFormat: "mp3", @@ -262,6 +288,7 @@ var _ = Describe("Transcode endpoints", func() { TargetBitDepth: 16, TargetChannels: 2, } + mockTD.validateMF = &model.MediaFile{ID: "song-2"} r := newGetRequest("mediaId=song-2", "mediaType=song", "transcodeParams=valid-token", "offset=10") _, _ = router.GetTranscodeStream(w, r) @@ -323,13 +350,16 @@ func newJSONPostRequest(queryParams string, jsonBody string) *http.Request { return r } -// mockTranscodeDecision is a test double for core.TranscodeDecision +// mockTranscodeDecision is a test double for transcode.Decider type mockTranscodeDecision struct { - decision *transcode.Decision - token string - tokenErr error - params *transcode.Params - parseErr error + decision *transcode.Decision + token string + tokenErr error + params *transcode.Params + parseErr error + validateParams *transcode.Params + validateMF *model.MediaFile + validateErr error } func (m *mockTranscodeDecision) MakeDecision(_ context.Context, _ *model.MediaFile, _ *transcode.ClientInfo) (*transcode.Decision, error) { @@ -350,6 +380,13 @@ func (m *mockTranscodeDecision) ParseTranscodeParams(_ string) (*transcode.Param return m.params, nil } +func (m *mockTranscodeDecision) ValidateTranscodeParams(_ context.Context, _ string, _ string) (*transcode.Params, *model.MediaFile, error) { + if m.validateErr != nil { + return nil, nil, m.validateErr + } + return m.validateParams, m.validateMF, nil +} + // fakeMediaStreamer captures the StreamRequest and returns a sentinel error, // allowing tests to verify parameter passing without constructing a real Stream. var errStreamCaptured = errors.New("stream request captured")