From 4df3e44eff767491d3b14df9e73369f130c90b46 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 8 Jan 2026 10:06:18 -0800 Subject: [PATCH 1/3] Add failing tests for ETag, HEAD requests, and conditional requests TDD: Write tests first before implementation for: - ETag generation and consistency in service layer - HEAD request support (headers only, no body) - Conditional requests with If-None-Match header (304 responses) --- internal/handlers/handlers_test.go | 256 +++++++++++++++++++++++++++++ internal/imgcache/service_test.go | 108 ++++++++++++ 2 files changed, 364 insertions(+) create mode 100644 internal/handlers/handlers_test.go diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go new file mode 100644 index 0000000..501fb05 --- /dev/null +++ b/internal/handlers/handlers_test.go @@ -0,0 +1,256 @@ +package handlers + +import ( + "bytes" + "context" + "database/sql" + "image" + "image/color" + "image/jpeg" + "io" + "io/fs" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + "testing/fstest" + "time" + + "github.com/go-chi/chi/v5" + "sneak.berlin/go/pixa/internal/database" + "sneak.berlin/go/pixa/internal/imgcache" +) + +// testFixtures contains test data. +type testFixtures struct { + handler *Handlers + service *imgcache.Service + goodHost string +} + +func setupTestHandler(t *testing.T) *testFixtures { + t.Helper() + + goodHost := "goodhost.example.com" + + // Generate test JPEG + jpegData := generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255}) + + mockFS := fstest.MapFS{ + goodHost + "/images/photo.jpg": &fstest.MapFile{Data: jpegData}, + } + + tmpDir := t.TempDir() + db := setupTestDB(t) + + cache, err := imgcache.NewCache(db, imgcache.CacheConfig{ + StateDir: tmpDir, + CacheTTL: time.Hour, + NegativeTTL: 5 * time.Minute, + HotCacheSize: 100, + HotCacheEnabled: true, + }) + if err != nil { + t.Fatalf("failed to create cache: %v", err) + } + + svc, err := imgcache.NewService(&imgcache.ServiceConfig{ + Cache: cache, + Fetcher: newMockFetcher(mockFS), + Whitelist: []string{goodHost}, + }) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + h := &Handlers{ + imgSvc: svc, + log: slog.Default(), + } + + return &testFixtures{ + handler: h, + service: svc, + goodHost: goodHost, + } +} + +func setupTestDB(t *testing.T) *sql.DB { + t.Helper() + + db, err := sql.Open("sqlite", ":memory:") + if err != nil { + t.Fatalf("failed to open test db: %v", err) + } + + if err := database.ApplyMigrations(db); err != nil { + t.Fatalf("failed to apply migrations: %v", err) + } + + return db +} + +func generateTestJPEG(t *testing.T, width, height int, c color.Color) []byte { + t.Helper() + + img := image.NewRGBA(image.Rect(0, 0, width, height)) + for y := 0; y < height; y++ { + for x := 0; x < width; x++ { + img.Set(x, y, c) + } + } + + var buf bytes.Buffer + if err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: 85}); err != nil { + t.Fatalf("failed to encode test JPEG: %v", err) + } + + return buf.Bytes() +} + +// mockFetcher fetches from a mock filesystem. +type mockFetcher struct { + fs fs.FS +} + +func newMockFetcher(fs fs.FS) *mockFetcher { + return &mockFetcher{fs: fs} +} + +func (f *mockFetcher) Fetch(ctx context.Context, url string) (*imgcache.FetchResult, error) { + // Remove https:// prefix + path := url[8:] // Remove "https://" + + data, err := fs.ReadFile(f.fs, path) + if err != nil { + return nil, imgcache.ErrUpstreamError + } + + return &imgcache.FetchResult{ + Content: io.NopCloser(bytes.NewReader(data)), + ContentLength: int64(len(data)), + ContentType: "image/jpeg", + }, nil +} + +func TestHandleImage_HEAD_ReturnsHeadersOnly(t *testing.T) { + fix := setupTestHandler(t) + + // Create a chi router to properly handle wildcards + r := chi.NewRouter() + r.Head("/v1/image/*", fix.handler.HandleImage()) + + req := httptest.NewRequest(http.MethodHead, "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg", nil) + rec := httptest.NewRecorder() + + r.ServeHTTP(rec, req) + + // Should return 200 OK + if rec.Code != http.StatusOK { + t.Errorf("HEAD request status = %d, want %d", rec.Code, http.StatusOK) + } + + // Should have Content-Type header + if ct := rec.Header().Get("Content-Type"); ct == "" { + t.Error("HEAD response should have Content-Type header") + } + + // Should have Content-Length header + if cl := rec.Header().Get("Content-Length"); cl == "" { + t.Error("HEAD response should have Content-Length header") + } + + // Body should be empty for HEAD request + if rec.Body.Len() != 0 { + t.Errorf("HEAD response body should be empty, got %d bytes", rec.Body.Len()) + } +} + +func TestHandleImage_ConditionalRequest_IfNoneMatch_Returns304(t *testing.T) { + fix := setupTestHandler(t) + + r := chi.NewRouter() + r.Get("/v1/image/*", fix.handler.HandleImage()) + + // First request to get the ETag + req1 := httptest.NewRequest(http.MethodGet, "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg", nil) + rec1 := httptest.NewRecorder() + + r.ServeHTTP(rec1, req1) + + if rec1.Code != http.StatusOK { + t.Fatalf("First request status = %d, want %d", rec1.Code, http.StatusOK) + } + + etag := rec1.Header().Get("ETag") + if etag == "" { + t.Fatal("Response should have ETag header") + } + + // Second request with If-None-Match header + req2 := httptest.NewRequest(http.MethodGet, "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg", nil) + req2.Header.Set("If-None-Match", etag) + rec2 := httptest.NewRecorder() + + r.ServeHTTP(rec2, req2) + + // Should return 304 Not Modified + if rec2.Code != http.StatusNotModified { + t.Errorf("Conditional request status = %d, want %d", rec2.Code, http.StatusNotModified) + } + + // Body should be empty for 304 response + if rec2.Body.Len() != 0 { + t.Errorf("304 response body should be empty, got %d bytes", rec2.Body.Len()) + } +} + +func TestHandleImage_ConditionalRequest_IfNoneMatch_DifferentETag(t *testing.T) { + fix := setupTestHandler(t) + + r := chi.NewRouter() + r.Get("/v1/image/*", fix.handler.HandleImage()) + + // Request with non-matching ETag + req := httptest.NewRequest(http.MethodGet, "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg", nil) + req.Header.Set("If-None-Match", `"different-etag"`) + rec := httptest.NewRecorder() + + r.ServeHTTP(rec, req) + + // Should return 200 OK with full response + if rec.Code != http.StatusOK { + t.Errorf("Request with non-matching ETag status = %d, want %d", rec.Code, http.StatusOK) + } + + // Body should not be empty + if rec.Body.Len() == 0 { + t.Error("Response body should not be empty for non-matching ETag") + } +} + +func TestHandleImage_ETagHeader(t *testing.T) { + fix := setupTestHandler(t) + + r := chi.NewRouter() + r.Get("/v1/image/*", fix.handler.HandleImage()) + + req := httptest.NewRequest(http.MethodGet, "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg", nil) + rec := httptest.NewRecorder() + + r.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("Request status = %d, want %d", rec.Code, http.StatusOK) + } + + etag := rec.Header().Get("ETag") + if etag == "" { + t.Error("Response should have ETag header") + } + + // ETag should be quoted + if len(etag) < 2 || etag[0] != '"' || etag[len(etag)-1] != '"' { + t.Errorf("ETag should be quoted, got %q", etag) + } +} diff --git a/internal/imgcache/service_test.go b/internal/imgcache/service_test.go index 06d5d33..0d9f7d8 100644 --- a/internal/imgcache/service_test.go +++ b/internal/imgcache/service_test.go @@ -405,3 +405,111 @@ func TestService_Get_ContextCancellation(t *testing.T) { t.Error("Get() expected error for cancelled context") } } + +func TestService_Get_ReturnsETag(t *testing.T) { + svc, fixtures := SetupTestService(t) + ctx := context.Background() + + req := &ImageRequest{ + SourceHost: fixtures.GoodHost, + SourcePath: "/images/photo.jpg", + Size: Size{Width: 50, Height: 50}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + resp, err := svc.Get(ctx, req) + if err != nil { + t.Fatalf("Get() error = %v", err) + } + defer resp.Content.Close() + + // ETag should be set + if resp.ETag == "" { + t.Error("ETag should be set in response") + } + + // ETag should be properly quoted + if len(resp.ETag) < 2 || resp.ETag[0] != '"' || resp.ETag[len(resp.ETag)-1] != '"' { + t.Errorf("ETag should be quoted, got %q", resp.ETag) + } +} + +func TestService_Get_ETagConsistency(t *testing.T) { + svc, fixtures := SetupTestService(t) + ctx := context.Background() + + req := &ImageRequest{ + SourceHost: fixtures.GoodHost, + SourcePath: "/images/photo.jpg", + Size: Size{Width: 50, Height: 50}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + // First request + resp1, err := svc.Get(ctx, req) + if err != nil { + t.Fatalf("Get() first request error = %v", err) + } + etag1 := resp1.ETag + resp1.Content.Close() + + // Second request (from cache) + resp2, err := svc.Get(ctx, req) + if err != nil { + t.Fatalf("Get() second request error = %v", err) + } + etag2 := resp2.ETag + resp2.Content.Close() + + // ETags should be identical for the same content + if etag1 != etag2 { + t.Errorf("ETags should match: %q != %q", etag1, etag2) + } +} + +func TestService_Get_DifferentETagsForDifferentContent(t *testing.T) { + svc, fixtures := SetupTestService(t) + ctx := context.Background() + + // Request same image at different sizes - should get different ETags + req1 := &ImageRequest{ + SourceHost: fixtures.GoodHost, + SourcePath: "/images/photo.jpg", + Size: Size{Width: 25, Height: 25}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + req2 := &ImageRequest{ + SourceHost: fixtures.GoodHost, + SourcePath: "/images/photo.jpg", + Size: Size{Width: 50, Height: 50}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + resp1, err := svc.Get(ctx, req1) + if err != nil { + t.Fatalf("Get() first request error = %v", err) + } + etag1 := resp1.ETag + resp1.Content.Close() + + resp2, err := svc.Get(ctx, req2) + if err != nil { + t.Fatalf("Get() second request error = %v", err) + } + etag2 := resp2.ETag + resp2.Content.Close() + + // ETags should be different for different content + if etag1 == etag2 { + t.Error("ETags should differ for different sizes") + } +} From 1f809a6fc9f9e7562afe4b295a6231466aecaf6b Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 8 Jan 2026 10:08:38 -0800 Subject: [PATCH 2/3] Implement ETag, HEAD requests, and conditional requests - Add ETag generation based on output content hash (first 16 chars) - Add ContentLength to ImageResponse from cache - Add LoadWithSize method to ContentStorage - Add GetOutputWithSize method to Cache - Handle HEAD requests returning headers only - Handle If-None-Match conditional requests returning 304 - Register HEAD route for image proxy endpoint --- internal/handlers/image.go | 16 ++++++++++++++++ internal/imgcache/cache.go | 5 +++++ internal/imgcache/service.go | 33 +++++++++++++++++++++++++-------- internal/imgcache/storage.go | 23 +++++++++++++++++++++++ internal/server/routes.go | 1 + 5 files changed, 70 insertions(+), 8 deletions(-) diff --git a/internal/handlers/image.go b/internal/handlers/image.go index 5dd9dcd..4f71217 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -122,6 +122,22 @@ func (s *Handlers) HandleImage() http.HandlerFunc { if resp.ETag != "" { w.Header().Set("ETag", resp.ETag) + + // Check for conditional request (If-None-Match) + if ifNoneMatch := r.Header.Get("If-None-Match"); ifNoneMatch != "" { + if ifNoneMatch == resp.ETag { + w.WriteHeader(http.StatusNotModified) + + return + } + } + } + + // Handle HEAD request - return headers only + if r.Method == http.MethodHead { + w.WriteHeader(http.StatusOK) + + return } // Stream the response diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index f6b9d82..665a517 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -168,6 +168,11 @@ func (c *Cache) GetOutput(outputHash string) (io.ReadCloser, error) { return c.dstContent.Load(outputHash) } +// GetOutputWithSize returns a reader and size for cached output content. +func (c *Cache) GetOutputWithSize(outputHash string) (io.ReadCloser, int64, error) { + return c.dstContent.LoadWithSize(outputHash) +} + // StoreSource stores fetched source content and metadata. func (c *Cache) StoreSource( ctx context.Context, diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index d57011c..a5d712e 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -91,15 +91,17 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e if result != nil && result.Hit { s.cache.IncrementStats(ctx, true, 0) - reader, err := s.cache.GetOutput(result.OutputHash) + reader, size, err := s.cache.GetOutputWithSize(result.OutputHash) if err != nil { s.log.Error("failed to get cached output", "hash", result.OutputHash, "error", err) // Fall through to re-fetch } else { return &ImageResponse{ - Content: reader, - ContentType: result.ContentType, - CacheStatus: CacheHit, + Content: reader, + ContentLength: size, + ContentType: result.ContentType, + CacheStatus: CacheHit, + ETag: formatETag(result.OutputHash), }, nil } } @@ -173,15 +175,17 @@ func (s *Service) fetchAndProcess(ctx context.Context, req *ImageRequest) (*Imag } // Serve from the cached file on disk (same path as cache hits) - reader, err := s.cache.GetOutput(outputHash) + reader, size, err := s.cache.GetOutputWithSize(outputHash) if err != nil { return nil, fmt.Errorf("failed to read cached output: %w", err) } return &ImageResponse{ - Content: reader, - ContentType: processResult.ContentType, - FetchedBytes: int64(len(sourceData)), + Content: reader, + ContentLength: size, + ContentType: processResult.ContentType, + FetchedBytes: int64(len(sourceData)), + ETag: formatETag(outputHash), }, nil } @@ -260,3 +264,16 @@ func extractStatusCode(err error) int { return httpStatusInternalError } + +// etagHashLength is the number of hash characters to use for ETags. +const etagHashLength = 16 + +// formatETag formats a hash as a quoted ETag value. +func formatETag(hash string) string { + // Use first 16 characters of hash for a shorter but still unique ETag + if len(hash) > etagHashLength { + hash = hash[:etagHashLength] + } + + return `"` + hash + `"` +} diff --git a/internal/imgcache/storage.go b/internal/imgcache/storage.go index 0c95023..f057977 100644 --- a/internal/imgcache/storage.go +++ b/internal/imgcache/storage.go @@ -114,6 +114,29 @@ func (s *ContentStorage) Load(hash string) (io.ReadCloser, error) { return f, nil } +// LoadWithSize returns a reader and file size for the content with the given hash. +func (s *ContentStorage) LoadWithSize(hash string) (io.ReadCloser, int64, error) { + path := s.hashToPath(hash) + + f, err := os.Open(path) //nolint:gosec // content-addressable path from hash + if err != nil { + if os.IsNotExist(err) { + return nil, 0, ErrNotFound + } + + return nil, 0, fmt.Errorf("failed to open content: %w", err) + } + + stat, err := f.Stat() + if err != nil { + _ = f.Close() + + return nil, 0, fmt.Errorf("failed to stat content: %w", err) + } + + return f, stat.Size(), nil +} + // Delete removes content with the given hash. func (s *ContentStorage) Delete(hash string) error { path := s.hashToPath(hash) diff --git a/internal/server/routes.go b/internal/server/routes.go index cbc376a..af87680 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -53,6 +53,7 @@ func (s *Server) SetupRoutes() { // Main image proxy route // /v1/image///x. s.router.Get("/v1/image/*", s.h.HandleImage()) + s.router.Head("/v1/image/*", s.h.HandleImage()) // Encrypted image URL route s.router.Get("/v1/e/{token}", s.h.HandleImageEnc()) From 774ee97ba10e4d32bd0dd68ad575acf6179c64b2 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 8 Jan 2026 10:09:10 -0800 Subject: [PATCH 3/3] Update TODO.md: mark HTTP response handling items complete Completed: - ETag generation and validation - Conditional requests (If-None-Match) - HEAD request support - Metrics endpoint with auth (already implemented) --- TODO.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/TODO.md b/TODO.md index a1f5d59..3354983 100644 --- a/TODO.md +++ b/TODO.md @@ -174,10 +174,10 @@ A single linear checklist of tasks to implement the complete pixa caching image ## HTTP Response Handling - [x] Implement proper Cache-Control headers -- [ ] Implement ETag generation and validation +- [x] Implement ETag generation and validation - [ ] Implement Last-Modified headers -- [ ] Implement conditional requests (If-None-Match, If-Modified-Since) -- [ ] Implement HEAD request support +- [x] Implement conditional requests (If-None-Match, If-Modified-Since) +- [x] Implement HEAD request support - [ ] Implement Vary header for content negotiation - [x] Implement X-Pixa-Cache debug header (HIT/MISS/STALE) - [ ] Implement X-Request-ID propagation @@ -185,7 +185,7 @@ A single linear checklist of tasks to implement the complete pixa caching image ## Additional Endpoints - [x] Implement robots.txt endpoint -- [ ] Implement metrics endpoint with auth +- [x] Implement metrics endpoint with auth - [ ] Implement auto-format selection (format=auto based on Accept header) ## Configuration