From e2fbbf91cae90abe857944cbfae942ae1d965e82 Mon Sep 17 00:00:00 2001 From: Elliot Shine Date: Tue, 20 Oct 2026 15:12:30 +1030 Subject: [PATCH] oceanmedia: fix segment endpoint redirecting to old segments This change fixes a bug where the segment endpoint redirected to a different segment than the one that was requested due to the cache map using the query string directly from fiber as the key. This is in a buffer that is constantly rewritten on different requests so the key kept on changing to the new value. --- cmd/oceanmedia/playlist_test.go | 20 ++++++++++++++++++++ cmd/oceanmedia/server.go | 6 ++++++ 2 files changed, 26 insertions(+) diff --git a/cmd/oceanmedia/playlist_test.go b/cmd/oceanmedia/playlist_test.go index 2c2b1626..0a8bf443 100644 --- a/cmd/oceanmedia/playlist_test.go +++ b/cmd/oceanmedia/playlist_test.go @@ -39,6 +39,7 @@ import ( "github.com/ausocean/cloud/gauth" "github.com/ausocean/cloud/model" "github.com/gofiber/fiber/v2" + "github.com/gofiber/fiber/v2/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -221,6 +222,25 @@ func TestServerSignURLCache(t *testing.T) { assert.Equal(t, 2, cs.callCount()) } +// TestServerSignURLCacheDoesNotAliasUnsafeKeys repeatedly passes a zero-copy +// string to the cache while mutating its backing buffer, as Fiber does when it +// reuses a request buffer. Every URI is distinct, so every call must sign a new +// URL rather than hitting an entry whose key was mutated to look like it. +func TestServerSignURLCacheDoesNotAliasUnsafeKeys(t *testing.T) { + cs := &countingSigner{} + s := newServer(nil, cs, testJWTSecret, time.Hour) + + const n = 4096 + buf := make([]byte, len("https://example.com/bucket/00000000.ts")) + for i := 0; i < n; i++ { + copy(buf, fmt.Sprintf("https://example.com/bucket/%08x.ts", i)) + _, err := s.signURL(context.Background(), utils.UnsafeString(buf)) + require.NoError(t, err) + } + + assert.Equal(t, n, cs.callCount()) +} + func newTestApp(s *server) *fiber.App { app := fiber.New() app.Get("/segment", s.handleSegment) diff --git a/cmd/oceanmedia/server.go b/cmd/oceanmedia/server.go index b59df8c9..e65b44d4 100644 --- a/cmd/oceanmedia/server.go +++ b/cmd/oceanmedia/server.go @@ -27,6 +27,7 @@ package main import ( "context" "log" + "strings" "sync" "time" @@ -109,6 +110,11 @@ func newServer(store datastore.Store, signer urlSigner, jwtSecret []byte, signed // signURL returns a pre-signed URL for the given storage URI, caching the // result until half the signed URL time to live has elapsed. func (s *server) signURL(ctx context.Context, rawURI string) (string, error) { + // rawURI may be a zero-copy string owned by the request context (for + // example fiber.Ctx.Query). It is used as a map key, which outlives the + // request, so copy it first. Otherwise the request buffer can be reused + // and mutate the key, causing every lookup to hit the same cached URL. + rawURI = strings.Clone(rawURI) now := s.now() s.signMu.Lock()