1 Commits
Author SHA1 Message Date
sneak adfb6ab50a Detect a fetched image's format from its bytes once (closes #319)
check / check (push) Canceled after 0s
magic.DetectFormat returns an imagerequest.Format. ValidateMagicBytes
returns the format it checked against the upstream Content-Type. The
cache stores that format in the source's row, so a variant made from a
cached source uses it too. The proxy hands it to Process, which still
refuses input libvips would load with another loader, such as an SVG
passing as AVIF. Format.MIMEType declares each MIME string once, and the
fetcher, the cache's variant read and the Accept matching use it.
magic.MIMEType, magic.ImageFormat, MIMEToImageFormat, magic's SVG
detection, detectFormat, formatFromString, FormatToMIME and
Result.InputFormat are gone.

Model: opus-5-5
2026-10-26 19:46:01 +00:00
13 changed files with 80 additions and 225 deletions
+5 -5
View File
@@ -191,11 +191,11 @@ allowlisted source hosts for open access.
name.
Each source image and each transformed image has one row in the database,
holding its size and when it was last used. pixa reads a cached source image's
format from its file's first bytes. When they match none of the source formats,
pixa fetches the source again. Two source URLs that serve the same bytes are
stored twice, once under each URL. A source image is stored by writing its file
and then its row; when the row cannot be written, the file is removed again.
holding its size and when it was last used. A source image's row also holds its
format, which pixa reads from the image's first bytes when it fetches it. Two
source URLs that serve the same bytes are stored twice, once under each URL. A
source image is stored by writing its file and then its row; when the row cannot
be written, the file is removed again.
Source and transformed image files are written under a temporary name, synced to
disk, and only then renamed into place, so a crash leaves no file under its own
+3 -5
View File
@@ -39,13 +39,11 @@ P2: security: per-origin rate limiting
- `Format.MIMEType` gives each format's MIME type. The fetcher, the mock
fetcher, the cache's read of a variant's type and the handler's `Accept`
matching use it.
- `Cache.Source` returns a source's format, read from its file's first bytes
as `Cache.Variant` reads a variant's. A file in no source format is an
error, and the proxy fetches the source again.
- The cache stores a source's format in its `source_content` row, and
`Cache.Source` returns it. `Cache.PutSource` takes it.
- The proxy hands the format to `ImageProcessor.Process`. The processor
refuses input that libvips would load as another format, as before, with
`ErrUnsupportedInputFormat`. It refuses a format that is not one of the
six source formats the same way.
`ErrUnsupportedInputFormat`.
- `internal/magic` no longer detects SVG, which pixa does not read. SVG data
is `ErrUnknownFormat`.
- Removed:
@@ -122,7 +122,7 @@ func writeConcurrently(t *testing.T, db *sql.DB) {
// record again.
func writeLikeOneRequest(ctx context.Context, db *sql.DB, key string) error {
_, err := db.ExecContext(ctx, `INSERT INTO source_content
(source_key, size_bytes) VALUES (?, 1)`, key)
(source_key, size_bytes, format) VALUES (?, 1, 'jpeg')`, key)
if err != nil {
return fmt.Errorf("storing a source: %w", err)
}
+3 -1
View File
@@ -3,10 +3,12 @@
-- Source images, one row per source URL
-- Files stored at: cache/sources/<ab>/<cd>/<source_key>, where source_key
-- is the SHA-256 of the source's host, path and query
-- is the SHA-256 of the source's host, path and query. format is the
-- image's format, detected from its first bytes when it was fetched.
CREATE TABLE IF NOT EXISTS source_content (
source_key TEXT PRIMARY KEY,
size_bytes INTEGER NOT NULL,
format TEXT NOT NULL,
last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
);
CREATE INDEX IF NOT EXISTS idx_source_content_last_accessed
+9 -29
View File
@@ -113,8 +113,7 @@ var ErrUnsupportedOutputFormat = errors.New("unsupported output format")
// ErrUnsupportedInputFormat is returned, before the input is decoded, when
// libvips reads the input's first bytes as another format than its source
// format, or as none. An SVG whose first bytes pass the first-bytes check for
// AVIF is one example. A source format other than the six pixa reads is also
// refused with it.
// AVIF is one example.
var ErrUnsupportedInputFormat = errors.New("unsupported input format")
// ErrTooManyImages is returned when MaxConcurrentProcessing images are being
@@ -183,11 +182,14 @@ func (p *ImageProcessor) Process(
return nil, err
}
// Input libvips would not load as sourceFormat is refused before it is
// decoded
err = checkInputFormat(data, sourceFormat)
if err != nil {
return nil, err
// govips picks the libvips loader from the input's first bytes with
// DetermineImageType. Input it would not load with the loader of
// sourceFormat is refused before libvips reads it. An SVG whose first
// bytes pass the first-bytes check for AVIF is one, which librsvg would
// otherwise render.
if vips.DetermineImageType(data) != vipsImageType(sourceFormat) {
return nil, fmt.Errorf("%w: libvips does not read the input as %s",
ErrUnsupportedInputFormat, sourceFormat)
}
// Decode image
@@ -358,28 +360,6 @@ func (p *ImageProcessor) readInput(input io.Reader) ([]byte, error) {
return data, nil
}
// checkInputFormat returns ErrUnsupportedInputFormat unless libvips would
// load data with the loader of sourceFormat. govips picks the libvips loader
// from the input's first bytes with DetermineImageType. It refuses an SVG
// whose first bytes pass the first-bytes check for AVIF, which librsvg would
// otherwise render.
func checkInputFormat(data []byte, sourceFormat imagerequest.Format) error {
// A format with no loader, such as orig or auto, has the type govips also
// gives input it cannot read, which the check below would let through.
sourceType := vipsImageType(sourceFormat)
if sourceType == vips.ImageTypeUnknown {
return fmt.Errorf("%w: %q is not a source format",
ErrUnsupportedInputFormat, sourceFormat)
}
if vips.DetermineImageType(data) != sourceType {
return fmt.Errorf("%w: libvips does not read the input as %s",
ErrUnsupportedInputFormat, sourceFormat)
}
return nil
}
// vipsImageType returns the govips image type whose libvips loader reads
// format. orig and auto stand for another format, so they have
// vips.ImageTypeUnknown, which govips has no loader for.
@@ -2,7 +2,6 @@ package imageprocessor
import (
"bytes"
"errors"
"io"
"testing"
@@ -49,27 +48,3 @@ func TestImageProcessor_LoadsEachSourceFormat(t *testing.T) {
}
}
}
// TestImageProcessor_NotASourceFormatRefused verifies that a source format
// other than the six, such as orig, auto or none, is refused with
// ErrUnsupportedInputFormat. The input is text, which libvips reads as no
// format.
func TestImageProcessor_NotASourceFormatRefused(t *testing.T) {
t.Parallel()
proc := New(Params{MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1})
for _, sourceFormat := range []imagerequest.Format{
imagerequest.FormatOriginal, imagerequest.FormatAuto, "",
} {
result, err := proc.Process(t.Context(),
bytes.NewReader([]byte("this is text, not an image")), sourceFormat,
&imagerequest.Transform{Format: imagerequest.FormatJPEG})
t.Logf("source format %q: error = %v", sourceFormat, err)
if result != nil || !errors.Is(err, ErrUnsupportedInputFormat) {
t.Errorf("Process() with source format %q = %v, error %v; want %v",
sourceFormat, result, err, ErrUnsupportedInputFormat)
}
}
}
+11 -5
View File
@@ -303,10 +303,9 @@ func withoutCancelKeepingDeadline(
return context.WithDeadline(context.WithoutCancel(ctx), deadline)
}
// loadCachedSource opens src's cached source, reading only the first bytes,
// which give its format, and returns it with its size and its format. It
// returns nil if the source is not in the cache, cannot be read, is in no
// source format or exceeds maxResponseSize.
// loadCachedSource opens src's cached source, without reading it, and
// returns it with its size and its format; nil if it is not in the cache,
// cannot be read, is empty or exceeds maxResponseSize.
func (p *Proxy) loadCachedSource(
ctx context.Context, src imagerequest.Source,
) (io.ReadCloser, int64, imagerequest.Format) {
@@ -334,6 +333,12 @@ func (p *Proxy) loadCachedSource(
return nil, 0, ""
}
if size == 0 {
_ = reader.Close()
return nil, 0, ""
}
return reader, size, format
}
@@ -452,7 +457,8 @@ func (p *Proxy) fetchAndProcess(
}
// Store source content
_, err = p.cache.PutSource(ctx, req.Source, bytes.NewReader(sourceData))
_, err = p.cache.PutSource(ctx, req.Source, sourceFormat,
bytes.NewReader(sourceData))
if err != nil {
p.log.Warn("failed to store source content", "error", err)
// Continue even if caching fails
@@ -58,11 +58,10 @@ func holdProcessingSlot(
// TestProxy_Get_WaitsForSlotBeforeReadingCachedSource checks that a
// request whose source is cached holds none of it while it waits for a
// processing slot: past the first bytes, which give the source's format, it
// reads the cached file only once it has a slot. With the only slot held, a
// request for a new width of the cached 100x100 photo waits; the cached file
// is then rewritten as a 100x50 image before the slot is freed, so the
// request must answer with that image scaled to 40x20.
// processing slot: it reads the cached file only once it has a slot. With
// the only slot held, a request for a new width of the cached 100x100 photo
// waits; the cached file is then rewritten as a 100x50 image before the slot
// is freed, so the request must answer with that image scaled to 40x20.
func TestProxy_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) {
t.Parallel()
@@ -7,7 +7,6 @@ import (
"testing"
"sneak.berlin/go/pixa/internal/imagerequest"
"sneak.berlin/go/pixa/internal/imgcache"
)
// getImage calls Get for req and returns the image it answered with and
@@ -105,40 +104,6 @@ func TestProxy_Get_FetchesAgainForRemovedCachedSource(t *testing.T) {
}
}
// TestProxy_Get_FetchesAgainSourceOfNoSourceFormat replaces the cached
// source's file with text. The next request for a new size of the source
// must fetch it again and cache it whole.
func TestProxy_Get_FetchesAgainSourceOfNoSourceFormat(t *testing.T) {
t.Parallel()
svc, fixtures := setupTestProxy(t)
getImage(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
wholeSize := fileSize(t, path)
err := os.WriteFile(path, []byte("this is text, not an image"),
imgcache.StorageFilePerm)
if err != nil {
t.Fatalf("replacing the source's file: %v", err)
}
getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover))
fetches := readCounters(t, svc).upstreamFetchCount
t.Logf("upstream fetches = %d", fetches)
if fetches != 2 {
t.Errorf("upstream fetches = %d, want 2", fetches)
}
size := fileSize(t, path)
if size != wholeSize {
t.Errorf("cached source is %d bytes, want %d", size, wholeSize)
}
}
// TestProxy_Get_ConvertsAgainForRemovedCachedVariant caches a variant of
// the photo and removes its row and file, as the reconciliation pass does
// when the file is empty or cut short. internal/imgcache tests that the
+21 -26
View File
@@ -228,10 +228,9 @@ func (c *Cache) PutVariant(
}
// Source opens the cached source of src and returns a reader, its size in
// bytes and its format, read from the file's first bytes. A source whose row
// or file is missing is ErrNotFound, as is any source of a disabled cache; a
// file whose first bytes are not those of a source format is another error.
// A source it opens has its access time handed to the background writer
// bytes and its format, the one PutSource stored with it. A source whose row
// or file is missing is ErrNotFound, as is any source of a disabled cache. A
// source it opens has its access time handed to the background writer
// without waiting (see touchSourceContent).
func (c *Cache) Source(
ctx context.Context, src imagerequest.Source,
@@ -242,38 +241,32 @@ func (c *Cache) Source(
key := src.Key()
var rowExists bool
var format imagerequest.Format
err := c.db.QueryRowContext(ctx, `
SELECT EXISTS(SELECT 1 FROM source_content WHERE source_key = ?)
`, string(key)).Scan(&rowExists)
SELECT format FROM source_content WHERE source_key = ?
`, string(key)).Scan(&format)
if errors.Is(err, sql.ErrNoRows) {
return nil, 0, "", ErrNotFound
}
if err != nil {
return nil, 0, "", fmt.Errorf("failed to lookup source: %w", err)
}
if !rowExists {
return nil, 0, "", ErrNotFound
}
file, size, err := c.srcContent.Open(string(key))
if err != nil {
return nil, 0, "", err
}
format, err := readFormat(file)
if err != nil {
_ = file.Close()
return nil, 0, "", err
}
c.touchSourceContent(key)
return file, size, format, nil
}
// PutSource stores fetched source content: it writes the file named by
// the source's key, then the source's row. When the row cannot be written
// PutSource stores fetched source content and its format, which magic
// detected from its first bytes. It writes the file named by the source's
// key, then the source's row with the format. When the row cannot be written
// the file is removed and the store fails, as a file with no row is never
// served or evicted; if that removal fails too, the next reconciliation
// pass removes the file. On a disabled cache it is a no-op returning an
@@ -281,6 +274,7 @@ func (c *Cache) Source(
func (c *Cache) PutSource(
ctx context.Context,
src imagerequest.Source,
format imagerequest.Format,
content io.Reader,
) (imagerequest.SourceKey, error) {
if c.config.MaxBytes == 0 {
@@ -302,12 +296,13 @@ func (c *Cache) PutSource(
size := int64(len(data))
_, err = c.db.ExecContext(ctx, `
INSERT INTO source_content (source_key, size_bytes)
VALUES (?, ?)
INSERT INTO source_content (source_key, size_bytes, format)
VALUES (?, ?, ?)
ON CONFLICT(source_key) DO UPDATE SET
size_bytes = excluded.size_bytes,
format = excluded.format,
last_accessed_at = CURRENT_TIMESTAMP
`, string(key), size)
`, string(key), size, string(format))
if err != nil {
deleteErr := c.srcContent.Delete(string(key))
if deleteErr != nil {
@@ -449,9 +444,9 @@ func (c *Cache) cleanExpiredBatch(ctx context.Context) (int64, error) {
return deleted, nil
}
// readFormat reads the first bytes of a source or variant file, without
// moving its offset, and returns the format they show. Sources and variants
// are in the same six formats.
// readFormat reads the first bytes of content, without moving its offset,
// and returns the format they show. pixa writes each format it reads, so
// that is an output format.
func readFormat(content io.ReaderAt) (imagerequest.Format, error) {
firstBytes := make([]byte, magic.MinMagicBytes)
+7 -4
View File
@@ -79,8 +79,9 @@ func newEvictionTestCache(t *testing.T, maxBytes int64) (*Cache, string) {
return cache, tmpDir
}
// storeEvictionTestSource stores content as a fetched source for
// host/path and returns the resulting source key.
// storeEvictionTestSource stores content as a fetched JPEG source for
// host/path and returns the resulting source key. The cache does not read
// the content, so it need not be a JPEG.
func storeEvictionTestSource(
t *testing.T, cache *Cache, host, path string, content []byte,
) imagerequest.SourceKey {
@@ -99,7 +100,8 @@ func storeEvictionTestSource(
}
key, err := cache.PutSource(
context.Background(), req.Source, bytes.NewReader(content),
context.Background(), req.Source, imagerequest.FormatJPEG,
bytes.NewReader(content),
)
if err != nil {
t.Fatalf("PutSource(%s%s) failed: %v", host, path, err)
@@ -436,7 +438,8 @@ func assertDisabledCacheWritesAreNoOps(
t.Fatalf("PutVariant on disabled cache must be a no-op, got error: %v", err)
}
key, err := cache.PutSource(ctx, req.Source, bytes.NewReader([]byte("data")))
key, err := cache.PutSource(ctx, req.Source, imagerequest.FormatJPEG,
bytes.NewReader([]byte("data")))
if err != nil {
t.Fatalf("PutSource on disabled cache must be a no-op, got error: %v", err)
}
@@ -5,7 +5,6 @@ import (
"context"
"database/sql"
"errors"
"image/color"
"io"
"log/slog"
"strings"
@@ -205,8 +204,8 @@ func TestSource_HandsAccessTimeToTheBackgroundWriter(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
content := generateTestJPEG(t, 10, 10, color.RGBA{0, 0, 255, 255})
key := storeEvictionTestSource(t, cache, "example.com", "/photo.jpg", content)
key := storeEvictionTestSource(t, cache, "example.com", "/photo.jpg",
[]byte("source bytes"))
anHourAgo := time.Now().Add(-time.Hour)
setSourceLastAccessed(t, cache, key, anHourAgo)
@@ -225,7 +224,7 @@ func TestSource_HandsAccessTimeToTheBackgroundWriter(t *testing.T) {
found, err := io.ReadAll(reader)
_ = reader.Close()
if err != nil || !bytes.Equal(found, content) {
if err != nil || string(found) != "source bytes" {
t.Fatalf("Source() reads %q, %v; want the source of %q", found, err, key)
}
@@ -2,17 +2,12 @@ package imgcache
import (
"bytes"
"errors"
"fmt"
"image/color"
"io"
"os"
"path/filepath"
"testing"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imagerequest"
"sneak.berlin/go/pixa/internal/magic"
)
// TestPutSource_StoresIdenticalBytesOfTwoURLsTwice stores the same bytes
@@ -69,7 +64,7 @@ func TestEvictSourceBlob_LeavesSourceWithIdenticalBytesServed(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
content := generateTestJPEG(t, 20, 20, color.RGBA{0x5B, 0x5B, 0x5B, 255})
content := bytes.Repeat([]byte{0x5B}, 400)
evicted := storeEvictionTestSource(t, cache, testHostCDN, "/evicted.jpg", content)
storeEvictionTestSource(t, cache, testHostCDN, "/kept.jpg", content)
@@ -127,7 +122,7 @@ func TestPutSource_FailedRowWriteLeavesNoFile(t *testing.T) {
req := &imagerequest.Request{
Source: imagerequest.Source{Host: testHostCDN, Path: testPathCat},
}
key, err := cache.PutSource(t.Context(), req.Source,
key, err := cache.PutSource(t.Context(), req.Source, imagerequest.FormatJPEG,
bytes.NewReader([]byte("source bytes")))
t.Logf("PutSource() = %q, error = %v", key, err)
@@ -141,98 +136,36 @@ func TestPutSource_FailedRowWriteLeavesNoFile(t *testing.T) {
}
}
// TestSource_FormatFromFirstBytes stores an image in each source format
// under one source URL, in turn. After each store, Source must return that
// image whole, with its format read from the file.
func TestSource_FormatFromFirstBytes(t *testing.T) {
// TestSource_ReturnsTheFormatPutSourceStored stores a source as PNG, then
// the same source URL again as WebP. After each store, Source must return
// the format that store gave.
func TestSource_ReturnsTheFormatPutSourceStored(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
processor := imageprocessor.New(imageprocessor.Params{
MaxInputBytes: 50 << 20,
MaxConcurrentProcessing: 1,
})
jpegImage := generateTestJPEG(t, 40, 30, color.RGBA{200, 40, 40, 255})
src := imagerequest.Source{Host: testHostCDN, Path: testPathCat}
for _, format := range []imagerequest.Format{
imagerequest.FormatJPEG, imagerequest.FormatPNG, imagerequest.FormatGIF,
imagerequest.FormatWebP, imagerequest.FormatAVIF, imagerequest.FormatJXL,
imagerequest.FormatPNG, imagerequest.FormatWebP,
} {
result, err := processor.Process(t.Context(), bytes.NewReader(jpegImage),
imagerequest.FormatJPEG, &imagerequest.Transform{Format: format})
if err != nil {
t.Fatalf("making a %s image: %v", format, err)
}
image, err := io.ReadAll(result.Content)
if err != nil {
t.Fatalf("reading the %s image: %v", format, err)
}
_, err = cache.PutSource(t.Context(), src, bytes.NewReader(image))
_, err := cache.PutSource(t.Context(), src, format,
bytes.NewReader([]byte("source bytes")))
if err != nil {
t.Fatalf("PutSource(%s) error = %v", format, err)
}
reader, size, got, err := cache.Source(t.Context(), src)
reader, _, got, err := cache.Source(t.Context(), src)
if err != nil {
t.Fatalf("Source() of the %s image error = %v", format, err)
t.Fatalf("Source() after PutSource(%s) error = %v", format, err)
}
served, err := io.ReadAll(reader)
_ = reader.Close()
if err != nil {
t.Fatalf("reading the %s source: %v", format, err)
}
t.Logf("%s: %d bytes read as %s", format, size, got)
t.Logf("Source() after PutSource(%s): format %s", format, got)
if got != format {
t.Errorf("Source() of the %s image format = %q, want %q",
t.Errorf("Source() after PutSource(%s) format = %q, want %q",
format, got, format)
}
if size != int64(len(image)) || !bytes.Equal(served, image) {
t.Errorf("%s source read as %d bytes, size %d, want the %d bytes stored",
format, len(served), size, len(image))
}
}
}
// TestSource_RefusesFileOfNoSourceFormat checks that Source refuses a
// cached source whose file's first bytes are not those of a source format,
// with an error other than ErrNotFound.
func TestSource_RefusesFileOfNoSourceFormat(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
jpegImage := generateTestJPEG(t, 10, 10, color.RGBA{0, 0, 255, 255})
tests := []struct {
name string
content []byte
wantErr error
}{
{"text", []byte("this is text, not an image"), magic.ErrUnknownFormat},
{"SVG", []byte(`<svg xmlns="http://www.w3.org/2000/svg"></svg>`),
magic.ErrUnknownFormat},
{"JPEG cut to 10 bytes", jpegImage[:10], io.EOF},
{"empty file", nil, io.EOF},
}
for i, tt := range tests {
path := fmt.Sprintf("/refused%d.jpg", i)
storeEvictionTestSource(t, cache, testHostCDN, path, tt.content)
reader, _, format, err := cache.Source(t.Context(),
imagerequest.Source{Host: testHostCDN, Path: path})
t.Logf("%s: Source() error = %v", tt.name, err)
if reader != nil || format != "" || !errors.Is(err, tt.wantErr) {
t.Errorf("%s: Source() = reader %v, format %q, error %v; "+
"want no reader and %v", tt.name, reader, format, err, tt.wantErr)
}
}
}