From b2b28e1fe595c3779bc8ce99357106779988137a Mon Sep 17 00:00:00 2001 From: Snider Date: Fri, 17 Apr 2026 20:29:26 +0100 Subject: [PATCH] Harden manifest cache access --- pkg/display/manifest.go | 21 ++++++++++-- pkg/display/manifest_test.go | 63 ++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/pkg/display/manifest.go b/pkg/display/manifest.go index cdb5f343..05f09672 100644 --- a/pkg/display/manifest.go +++ b/pkg/display/manifest.go @@ -50,12 +50,16 @@ type loadedManifest struct { } func (s *Service) loadManifestForOrigin(pageURL string) (*loadedManifest, error) { + s.manifestMu.Lock() if s.manifestCache == nil { s.manifestCache = make(map[string]*loadedManifest) } if cached, ok := s.manifestCache[pageURL]; ok { + s.manifestMu.Unlock() return cached, nil } + s.manifestMu.Unlock() + path, err := discoverManifestPath(pageURL) if err != nil { return nil, err @@ -81,7 +85,13 @@ func (s *Service) loadManifestForOrigin(pageURL string) (*loadedManifest, error) BaseDir: manifestBaseDir(path), Manifest: manifest, } + + s.manifestMu.Lock() + if s.manifestCache == nil { + s.manifestCache = make(map[string]*loadedManifest) + } s.manifestCache[pageURL] = loaded + s.manifestMu.Unlock() return loaded, nil } @@ -183,13 +193,18 @@ func discoverManifestPath(pageURL string) (string, error) { } func (s *Service) manifestWindowConfig(pageURL string) map[string]ManifestWindow { - s.manifestMu.Lock() - defer s.manifestMu.Unlock() loaded, err := s.loadManifestForOrigin(pageURL) if err != nil || loaded == nil { return nil } - return loaded.Manifest.Windows + if len(loaded.Manifest.Windows) == 0 { + return nil + } + windows := make(map[string]ManifestWindow, len(loaded.Manifest.Windows)) + for name, cfg := range loaded.Manifest.Windows { + windows[name] = cfg + } + return windows } func (s *Service) readManifestPreload(baseDir, preloadPath string) ([]byte, error) { diff --git a/pkg/display/manifest_test.go b/pkg/display/manifest_test.go index 52a7cf86..e20fcd27 100644 --- a/pkg/display/manifest_test.go +++ b/pkg/display/manifest_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -157,6 +158,30 @@ func TestManifest_ManifestWindowConfig_Ugly(t *testing.T) { assert.Nil(t, got) } +func TestManifest_ManifestWindowConfig_ReturnsCopy(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(root, ".core"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "index.html"), []byte(""), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(root, ".core", "view.yaml"), []byte(strings.Join([]string{ + "windows:", + " main:", + " title: Core GUI", + " width: 1280", + " height: 720", + }, "\n")), 0o644)) + + svc, err := New() + require.NoError(t, err) + + first := svc.manifestWindowConfig(filepath.Join(root, "index.html")) + require.NotNil(t, first) + first["main"] = ManifestWindow{Title: "mutated"} + + second := svc.manifestWindowConfig(filepath.Join(root, "index.html")) + require.NotNil(t, second) + assert.Equal(t, "Core GUI", second["main"].Title) +} + func TestManifest_LoadManifestForOrigin_RejectsOversizedFile(t *testing.T) { root := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(root, ".core"), 0o755)) @@ -171,6 +196,44 @@ func TestManifest_LoadManifestForOrigin_RejectsOversizedFile(t *testing.T) { assert.Contains(t, err.Error(), "exceeds") } +func TestManifest_LoadManifestForOrigin_Concurrent(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(root, ".core"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "index.html"), []byte(""), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(root, ".core", "view.yaml"), []byte(strings.Join([]string{ + "name: demo", + "windows:", + " main:", + " title: Core GUI", + }, "\n")), 0o644)) + + svc, err := New() + require.NoError(t, err) + + var wg sync.WaitGroup + errs := make(chan error, 16) + for i := 0; i < 16; i++ { + wg.Add(1) + go func() { + defer wg.Done() + loaded, loadErr := svc.loadManifestForOrigin(filepath.Join(root, "index.html")) + if loadErr != nil { + errs <- loadErr + return + } + if loaded == nil || loaded.Manifest.Name != "demo" { + errs <- assert.AnError + } + }() + } + wg.Wait() + close(errs) + + for err := range errs { + require.NoError(t, err) + } +} + func TestManifest_ManifestBaseDir_Good(t *testing.T) { assert.Equal(t, "/tmp/app", manifestBaseDir("/tmp/app/.core/view.yaml")) assert.Equal(t, "/tmp/app/assets", manifestBaseDir("/tmp/app/assets/view.yaml"))