diff --git a/README.md b/README.md index 85af3f2..4765703 100644 --- a/README.md +++ b/README.md @@ -111,6 +111,21 @@ with a private address can choose the address it is counted by through its own its own address is trusted too. Setting `trusted_proxies` to the proxy's own address closes this. +### Image Metadata + +pixa decodes and re-encodes every image it serves, and removes all metadata from +the output: EXIF (GPS position, camera make, model and serial number, capture +time, embedded thumbnail), XMP, IPTC and the ICC colour profile. This cannot be +turned off. + +- The `orig` format means the source's own format, not the source's bytes: an + `orig` image is re-encoded and stripped like any other. +- An image with an EXIF orientation is turned upright first, so it displays the + same without the tag; a requested size applies to the upright image. +- An image with an ICC profile is converted to sRGB first, since clients show an + image with no profile as sRGB. Colours outside sRGB, such as the most + saturated ones in a Display P3 photo, are clipped. + ### Source Hosts Source hosts may be allowlisted in the configuration. Non-allowlisted diff --git a/TODO.md b/TODO.md index 486c8eb..56d0745 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,12 @@ exhaustion # Completed Steps +- 2026-09-28 strip metadata from processed images (closes #82): every output is + exported with govips' `StripMetadata`, so it carries no EXIF, XMP, IPTC or ICC + profile; the image is first turned upright with `AutoRotate` (before sizes are + worked out) and, when it has an ICC profile, converted to sRGB; the `orig` + format is re-encoded and stripped like any other, as pixa never serves the + source bytes; there is no setting to keep metadata; documented in `README.md`. - 2026-09-28 rate limit the login form (closes #66): `POST /` is limited to 5 attempts per minute per client address, and an attempt over the limit is refused with 429 and a `Retry-After` header; the address is the one @@ -251,7 +257,6 @@ exhaustion # Future Steps -- P1: strip EXIF and other metadata from processed images (privacy) - P2: security - referer blacklist - per-IP rate limiting on the image routes diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index 32e7b7a..dd77ac1 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -161,6 +161,13 @@ func (p *ImageProcessor) Process( } defer img.Close() + // Turn the image upright now: encode strips the EXIF orientation tag, + // and sizes below must be worked out on the upright image. + err = img.AutoRotate() + if err != nil { + return nil, fmt.Errorf("failed to auto-rotate: %w", err) + } + // Get original dimensions origWidth := img.Width() origHeight := img.Height() @@ -404,6 +411,21 @@ func (p *ImageProcessor) encode( return nil, fmt.Errorf("%w: %s", ErrUnsupportedOutputFormat, format) } + // Stripping drops the ICC profile as well, and clients show an image + // with no profile as sRGB, so convert to sRGB first. "srgb" names + // libvips' built-in profile; govips' own sRGB path variable is set on + // first use but read without a lock, so concurrent requests race on it. + if img.HasICCProfile() { + err := img.TransformICCProfileWithFallback("srgb", "srgb") + if err != nil { + return nil, fmt.Errorf("failed to convert to sRGB: %w", err) + } + } + + // Drop EXIF, XMP, IPTC and the ICC profile. govips ignores this for + // GIF, which carries none of them. + params.StripMetadata = true + output, _, err := img.Export(¶ms) if err != nil { return nil, err diff --git a/internal/imageprocessor/imageprocessor_internal_test.go b/internal/imageprocessor/imageprocessor_internal_test.go index 21de6c5..73decd0 100644 --- a/internal/imageprocessor/imageprocessor_internal_test.go +++ b/internal/imageprocessor/imageprocessor_internal_test.go @@ -9,7 +9,9 @@ import ( "image/jpeg" "image/png" "io" + "math" "os" + "slices" "testing" "github.com/davidbyttow/govips/v2/vips" @@ -561,3 +563,152 @@ func TestImageProcessor_EncodeAVIF(t *testing.T) { encodeAndCheck(t, FormatAVIF, 85, mimeAVIF) } + +// processAndDecode runs input through Process and decodes the output with +// vips, so a test can inspect the image a client would receive. +func processAndDecode(t *testing.T, input []byte, req *Request) *vips.ImageRef { + t.Helper() + + result, err := New(Params{}).Process( + context.Background(), bytes.NewReader(input), req, + ) + if err != nil { + t.Fatalf("Process() error = %v", err) + } + + defer func() { _ = result.Content.Close() }() + + data, err := io.ReadAll(result.Content) + if err != nil { + t.Fatalf("failed to read result: %v", err) + } + + output, err := vips.NewImageFromBuffer(data) + if err != nil { + t.Fatalf("failed to decode output: %v", err) + } + + t.Cleanup(output.Close) + + return output +} + +func TestImageProcessor_StripsEXIF(t *testing.T) { + t.Parallel() + + // gps-exif.jpg carries GPS coordinates, a camera make, model and serial + // number, and a capture time. + input, err := os.ReadFile("testdata/gps-exif.jpg") + if err != nil { + t.Fatalf("failed to read test JPEG: %v", err) + } + + fixture, err := vips.NewImageFromBuffer(input) + if err != nil { + t.Fatalf("failed to decode test JPEG: %v", err) + } + + t.Cleanup(fixture.Close) + + if !slices.Contains(fixture.GetFields(), "exif-ifd3-GPSLatitude") { + t.Fatal("testdata/gps-exif.jpg has no GPS latitude") + } + + formats := []Format{ + FormatJPEG, FormatPNG, FormatWebP, FormatAVIF, FormatGIF, FormatOriginal, + } + + for _, format := range formats { + t.Run(string(format), func(t *testing.T) { + t.Parallel() + + output := processAndDecode(t, input, &Request{Format: format}) + + if output.HasExif() { + t.Errorf("output has EXIF: %v", output.GetExif()) + } + }) + } +} + +func TestImageProcessor_AppliesEXIFOrientation(t *testing.T) { + t.Parallel() + + // orientation-6.jpg is stored 16x8, red on the left and blue on the + // right, with EXIF orientation 6 (turn 90 degrees clockwise to view). + // Upright it is 8x16, red on top and blue below. + input, err := os.ReadFile("testdata/orientation-6.jpg") + if err != nil { + t.Fatalf("failed to read test JPEG: %v", err) + } + + tests := []struct { + name string + size Size + wantW int + wantH int + }{ + {name: "original size", size: Size{}, wantW: 8, wantH: 16}, + {name: "width only", size: Size{Width: 4}, wantW: 4, wantH: 8}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + output := processAndDecode(t, input, &Request{ + Size: tt.size, + Format: FormatPNG, + }) + + if output.Width() != tt.wantW || output.Height() != tt.wantH { + t.Fatalf("output is %dx%d, want %dx%d", + output.Width(), output.Height(), tt.wantW, tt.wantH) + } + + top, err := output.GetPoint(tt.wantW/2, 0) + if err != nil { + t.Fatalf("GetPoint() error = %v", err) + } + + bottom, err := output.GetPoint(tt.wantW/2, tt.wantH-1) + if err != nil { + t.Fatalf("GetPoint() error = %v", err) + } + + if top[0] <= top[2] || bottom[2] <= bottom[0] { + t.Errorf("top pixel = %v, bottom pixel = %v, want red above blue", + top, bottom) + } + }) + } +} + +func TestImageProcessor_ConvertsWideGamutToSRGB(t *testing.T) { + t.Parallel() + + // display-p3.jpg is a flat 8x8 image with the Display P3 profile + // embedded, filled with Display P3 (234, 51, 35), which is sRGB red. + input, err := os.ReadFile("testdata/display-p3.jpg") + if err != nil { + t.Fatalf("failed to read test JPEG: %v", err) + } + + output := processAndDecode(t, input, &Request{Format: FormatPNG}) + + if output.HasICCProfile() { + t.Error("output has an ICC profile") + } + + pixel, err := output.GetPoint(4, 4) + if err != nil { + t.Fatalf("GetPoint() error = %v", err) + } + + want := []float64{255, 0, 0} + for i := range want { + if math.Abs(pixel[i]-want[i]) > 5 { + t.Fatalf("pixel = %v, want within 5 of %v", pixel, want) + } + } +} diff --git a/internal/imageprocessor/testdata/display-p3.jpg b/internal/imageprocessor/testdata/display-p3.jpg new file mode 100644 index 0000000..c2c49a6 Binary files /dev/null and b/internal/imageprocessor/testdata/display-p3.jpg differ diff --git a/internal/imageprocessor/testdata/gps-exif.jpg b/internal/imageprocessor/testdata/gps-exif.jpg new file mode 100644 index 0000000..8deb6af Binary files /dev/null and b/internal/imageprocessor/testdata/gps-exif.jpg differ diff --git a/internal/imageprocessor/testdata/orientation-6.jpg b/internal/imageprocessor/testdata/orientation-6.jpg new file mode 100644 index 0000000..da8dd48 Binary files /dev/null and b/internal/imageprocessor/testdata/orientation-6.jpg differ