Stop a transcode whose request has ended before it encodes (closes #243)

Process looked at its context only while it waited for a processing
slot, so a transcode went on decoding and encoding, holding its slot,
however long after its request's downstream_timeout that took.
acquireSlot now gives the slot back and returns the context's error
when the context has ended by the time it has a slot, and encode
returns that error without beginning. libvips cannot stop an encode
once it has begun, so that encode keeps its slot until it finishes.
README.md gives how long that can take per output format at the 8192
limits and no longer says the work stops when downstream_timeout ends;
TODO.md is corrected the same way.

Model: opus-5-5

README.md

@@ -433,11 +433,8 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit"
 Where:
 
 - `host` — source origin hostname (e.g. `cdn.example.com`)
-- `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
-  and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg`
-- `query` — source query string, empty string if none, with `%` and `:` written
-  as in `path`; neither then holds a `:`, so path `/a:b` with no query and path
-  `/a` with query `b:` are signed differently
+- `path` — source path (e.g. `/photos/cat.jpg`)
+- `query` — source query string, empty string if none
 - `width` — requested width in pixels, `0` for original
 - `height` — requested height in pixels, `0` for original
 - `format` — output format, one of those listed under Routes, with `original`

TODO.md

@@ -30,14 +30,6 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
-- 2026-10-15 a `:` in the source path or query no longer moves where the path
-  ends and the query begins (closes #247): the signed data and the cache key
-  joined host, path and query with `:` and no escaping, so a URL signed for
-  `https://h/a:b` verified for `https://h/a?b:`; the two also shared a cache
-  key, and whichever was fetched first was served for both. In both, the path
-  and the query now have each `%` written as `%25` and each `:` as `%3A`; a path
-  and query holding neither are signed and keyed as before. Signature
-  Specification in `README.md` says so.
 - 2026-10-15 a transcode stops once its request's `downstream_timeout` has
   ended, unless it has begun encoding (closes #243): the image processor looked
   at its context only while it waited for a processing slot, so an image was

internal/handlers/server_busy_internal_test.go

@@ -1,7 +1,6 @@
 package handlers
 
 import (
-	"context"
 	"fmt"
 	"log/slog"
 	"net/http"
@@ -48,39 +47,3 @@ func TestServerBusyAnswers503(t *testing.T) {
 		}
 	}
 }
-
-// TestProcessingTimedOutAnswers503 checks that both image routes answer 503
-// with the server busy error when the image service reports that processing
-// stopped at the deadline of the request doing it, wrapped as the service
-// wraps it. A request that waited for that processing gets this error while
-// its own deadline may have time left.
-func TestProcessingTimedOutAnswers503(t *testing.T) {
-	t.Parallel()
-
-	h := &Handlers{log: slog.New(slog.DiscardHandler)}
-	req := &imgcache.ImageRequest{SourceHost: "img.example.com", SourcePath: "/a.jpg"}
-	err := fmt.Errorf("%w: failed to encode: %w",
-		imgcache.ErrProcessingTimedOut, context.DeadlineExceeded)
-
-	plain := httptest.NewRecorder()
-	h.respondImageError(plain, req, err)
-
-	encrypted := httptest.NewRecorder()
-	h.handleImageError(encrypted, err)
-
-	for route, rec := range map[string]*httptest.ResponseRecorder{
-		"/v1/image/": plain, "/v1/e/": encrypted,
-	} {
-		t.Logf("%s: status = %d, body = %q", route, rec.Code, rec.Body.String())
-
-		if rec.Code != http.StatusServiceUnavailable {
-			t.Errorf("%s: status = %d, want %d",
-				route, rec.Code, http.StatusServiceUnavailable)
-		}
-
-		if !strings.Contains(rec.Body.String(), "server busy, try again later") {
-			t.Errorf("%s: body = %q, want the server busy error",
-				route, rec.Body.String())
-		}
-	}
-}

internal/imageprocessor/deadline_internal_test.go

@@ -88,31 +88,3 @@ func checkProcessStopsAtDeadline(
 		t.Errorf("%d slots still held after Process() returned", held)
 	}
 }
-
-// inputNotToBeRead is a Process input that fails the test when it is read.
-type inputNotToBeRead struct {
-	t *testing.T
-}
-
-func (r inputNotToBeRead) Read([]byte) (int, error) {
-	r.t.Error("Process read its input although its deadline had passed")
-
-	return 0, io.EOF
-}
-
-// TestProcessReadsNoInputOnceDeadlinePassed checks that Process, called with
-// a context whose deadline passed before it starts, gives its slot back
-// without reading its input. With a slot free, the select in acquireSlot
-// takes the slot or sees the ended context at random, so Process is called
-// many times.
-func TestProcessReadsNoInputOnceDeadlinePassed(t *testing.T) {
-	t.Parallel()
-
-	ctx, cancel := context.WithDeadline(t.Context(),
-		time.Now().Add(-time.Second))
-	defer cancel()
-
-	for range 100 {
-		checkProcessStopsAtDeadline(ctx, t, inputNotToBeRead{t: t})
-	}
-}

internal/imgcache/concurrent_misses_internal_test.go

@@ -15,7 +15,6 @@ import (
 
 	"github.com/getsentry/sentry-go"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
-	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/magic"
 )
 
@@ -436,47 +435,6 @@ func TestService_Get_ReturnsByItsDeadline(t *testing.T) {
 	}
 }
 
-// TestService_Get_WaiterGetsProcessingTimedOut has a request with no deadline
-// join the held fetch of a first request with a short one. With the only
-// processing slot held, the first request's transcode waits for it until the
-// first request's deadline ends, and stops then. The waiting request, whose
-// own context has not ended, must get ErrProcessingTimedOut, which the image
-// routes answer 503 as when pixa is busy.
-func TestService_Get_WaiterGetsProcessingTimedOut(t *testing.T) {
-	t.Parallel()
-
-	svc, fixtures, fetcher := setupHeldFetchService(t)
-	svc.processor = imageprocessor.New(
-		imageprocessor.Params{MaxConcurrentProcessing: 1},
-	)
-
-	holdProcessingSlot(t, svc.processor)
-
-	const firstTimeout = 500 * time.Millisecond
-
-	firstCtx, cancelFirst := context.WithTimeout(t.Context(), firstTimeout)
-	defer cancelFirst()
-
-	first := startGet(firstCtx, svc, photoVariant(fixtures, 85, FitCover))
-
-	<-fetcher.started
-
-	waiter := startGet(t.Context(), svc, photoVariant(fixtures, 85, FitCover))
-
-	time.Sleep(arrivalWait)
-	fetcher.releaseFetches()
-
-	t.Logf("first request: Get() error = %v", (<-first).err)
-
-	got := <-waiter
-	t.Logf("waiting request: Get() error = %v", got.err)
-
-	if !errors.Is(got.err, ErrProcessingTimedOut) {
-		t.Errorf("waiting request: Get() error = %v, want %v",
-			got.err, ErrProcessingTimedOut)
-	}
-}
-
 // panickingFetcher panics on every fetch.
 type panickingFetcher struct{}
 

internal/imgcache/service.go

@@ -128,12 +128,6 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 // ErrNegativeCached is returned when a URL is in the negative cache (recently failed).
 var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
 
-// ErrProcessingTimedOut is returned when processing an image stopped because
-// the downstream_timeout of the request doing it ended, to that request and to
-// every request that waited for its image.
-var ErrProcessingTimedOut = errors.New(
-	"image processing stopped at the deadline of the request doing it")
-
 // Get retrieves a processed image, fetching and processing if necessary.
 func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) {
 	// Propagate AllowHTTP setting to the request

internal/imgcache/storage.go

@@ -10,7 +10,6 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
-	"strings"
 	"time"
 )
 
@@ -372,13 +371,11 @@ func HashPath(path string) PathHash {
 
 // CacheKey generates a unique key for a request variant.
 // Format: sha256(host:path:query:width:height:format:quality:fit_mode)
-// The path and query go through escapePercentAndColon, so that a ":" in
-// either cannot be taken for the one between them.
 func CacheKey(req *ImageRequest) VariantKey {
 	data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s",
 		req.SourceHost,
-		escapePercentAndColon(req.SourcePath),
-		escapePercentAndColon(req.SourceQuery),
+		req.SourcePath,
+		req.SourceQuery,
 		req.Size.Width,
 		req.Size.Height,
 		req.Format,
@@ -390,14 +387,6 @@ func CacheKey(req *ImageRequest) VariantKey {
 	return VariantKey(hex.EncodeToString(h[:]))
 }
 
-// escapePercentAndColon returns s with each "%" written as "%25" and each
-// ":" as "%3A", so that the result holds no ":".
-func escapePercentAndColon(s string) string {
-	s = strings.ReplaceAll(s, "%", "%25")
-
-	return strings.ReplaceAll(s, ":", "%3A")
-}
-
 // VariantStorage handles key-based file storage for processed image variants.
 // Files are stored at: <basedir>/<ab>/<cd>/<cache_key>
 // Metadata is stored at: <basedir>/<ab>/<cd>/<cache_key>.meta

internal/imgcache/storage_internal_test.go

@@ -407,62 +407,6 @@ func TestCacheKey(t *testing.T) {
 	}
 }
 
-// TestCacheKey_ColonOrPercentInPathOrQuery checks that two different source
-// URLs whose path or query holds ":" or "%" get different keys.
-func TestCacheKey_ColonOrPercentInPathOrQuery(t *testing.T) {
-	t.Parallel()
-
-	cases := []struct {
-		name          string
-		path1, query1 string
-		path2, query2 string
-	}{
-		{
-			name:  "colon moved from path to query",
-			path1: "/a:b", query1: "",
-			path2: "/a", query2: "b:",
-		},
-		{
-			name:  "colon in path and its percent-encoding",
-			path1: "/a:b", query1: "",
-			path2: "/a%3Ab", query2: "",
-		},
-	}
-
-	for _, tt := range cases {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			key1 := CacheKey(&ImageRequest{
-				SourceHost:  testHostCDN,
-				SourcePath:  tt.path1,
-				SourceQuery: tt.query1,
-				Size:        Size{Width: 800, Height: 600},
-				Format:      FormatWebP,
-				Quality:     85,
-				FitMode:     FitCover,
-			})
-			key2 := CacheKey(&ImageRequest{
-				SourceHost:  testHostCDN,
-				SourcePath:  tt.path2,
-				SourceQuery: tt.query2,
-				Size:        Size{Width: 800, Height: 600},
-				Format:      FormatWebP,
-				Quality:     85,
-				FitMode:     FitCover,
-			})
-
-			t.Logf("CacheKey() of path %q query %q = %s", tt.path1, tt.query1, key1)
-			t.Logf("CacheKey() of path %q query %q = %s", tt.path2, tt.query2, key2)
-
-			if key1 == key2 {
-				t.Errorf("CacheKey() gave path %q query %q and path %q query %q "+
-					"the same key", tt.path1, tt.query1, tt.path2, tt.query2)
-			}
-		})
-	}
-}
-
 // TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write
 // that fails is logged at warn and the store still succeeds.
 func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {

internal/signature/escaping_test.go

@@ -1,113 +0,0 @@
-package signature_test
-
-import (
-	"errors"
-	"testing"
-	"time"
-
-	"sneak.berlin/go/pixa/internal/signature"
-)
-
-// TestSigner_Verify_ColonCannotMovePathQueryBoundary checks that a URL
-// signed for path "/a:b" with no query does not verify for path "/a" with
-// query "b:", a different resource on the same host.
-func TestSigner_Verify_ColonCannotMovePathQueryBoundary(t *testing.T) {
-	t.Parallel()
-
-	signer := signature.New(goldenSigningKey)
-
-	signed := &signature.Request{
-		SourceHost: testHost,
-		SourcePath: "/a:b",
-		Width:      800,
-		Height:     600,
-		Format:     testFormatWebP,
-		Quality:    85,
-		FitMode:    testFitCover,
-		Expires:    time.Now().Add(1 * time.Hour),
-	}
-	signed.Signature = signer.Sign(signed)
-
-	replayed := *signed
-	replayed.SourcePath = "/a"
-	replayed.SourceQuery = "b:"
-
-	t.Logf("signature for path %q query %q: %s",
-		signed.SourcePath, signed.SourceQuery, signed.Signature)
-	t.Logf("signature for path %q query %q: %s",
-		replayed.SourcePath, replayed.SourceQuery, signer.Sign(&replayed))
-
-	err := signer.Verify(&replayed)
-	if !errors.Is(err, signature.ErrInvalid) {
-		t.Errorf("Verify() of path %q query %q with the signature for "+
-			"path %q = %v, want %v",
-			replayed.SourcePath, replayed.SourceQuery, signed.SourcePath,
-			err, signature.ErrInvalid)
-	}
-}
-
-// TestSigner_EscapedGoldenVectors pins the signature of requests whose path
-// or query holds ":" or "%", which are signed with "%" written as "%25" and
-// ":" as "%3A". The expected values were computed once, outside pixa, from
-// the signed data in each comment.
-func TestSigner_EscapedGoldenVectors(t *testing.T) {
-	t.Parallel()
-
-	signer := signature.New(goldenSigningKey)
-
-	vectors := []struct {
-		name          string
-		path          string
-		query         string
-		wantSignature string
-	}{
-		{
-			name: "colon in path",
-			path: "/photos/a:b.jpg",
-			// Signed data:
-			// "cdn.example.com:/photos/a%3Ab.jpg::800:600:webp:1704067200:85:cover"
-			wantSignature: "WnEpZ7bQp1Zvw8WDatlj2vu-4wYKWE3BFBizV8m__6k=",
-		},
-		{
-			name:  "colon in query",
-			path:  testPath,
-			query: "t=1:2",
-			// Signed data:
-			// "cdn.example.com:/photos/cat.jpg:t=1%3A2:800:600:webp:1704067200:85:cover"
-			wantSignature: "85kRoN8hhr4cCUkEUjNvUsvVYKjYYfWdeffx5TjJjqA=",
-		},
-		{
-			name:  "percent in path and query",
-			path:  "/photos/a%20b.jpg",
-			query: "q=50%",
-			// Signed data:
-			// "cdn.example.com:/photos/a%2520b.jpg:q=50%25:800:600:webp:1704067200:85:cover"
-			wantSignature: "MjDiiRdgXMrsjqPMsENtD4aG1hp1e-a6zy67vKTX4CQ=",
-		},
-	}
-
-	for _, tt := range vectors {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			req := &signature.Request{
-				SourceHost:  testHost,
-				SourcePath:  tt.path,
-				SourceQuery: tt.query,
-				Width:       800,
-				Height:      600,
-				Format:      testFormatWebP,
-				Quality:     85,
-				FitMode:     testFitCover,
-				Expires:     time.Unix(goldenExpiresUnix, 0),
-			}
-
-			gotSignature := signer.Sign(req)
-			t.Logf("Sign() of path %q query %q = %s", tt.path, tt.query, gotSignature)
-
-			if gotSignature != tt.wantSignature {
-				t.Errorf("Sign() = %q, want %q", gotSignature, tt.wantSignature)
-			}
-		})
-	}
-}

internal/signature/signature.go

@@ -10,7 +10,6 @@ import (
 	"fmt"
 	"net/url"
 	"strconv"
-	"strings"
 	"time"
 )
 
@@ -155,17 +154,15 @@ func (s *Signer) GenerateSignedURL(
 
 // buildSignatureData creates the string to be signed.
 // Format: "host:path:query:width:height:format:expiration:quality:fit"
-// The path and query go through escapePercentAndColon, so that a ":" in
-// either cannot be taken for the one between them; every other component
-// is used verbatim. No normalization, suffix matching, or wildcard
-// expansion is performed. Quality and fit are the effective transform
-// values, so replaying a signed URL with a different quality or fit mode
-// fails verification.
+// All components are used verbatim (exact match). No normalization,
+// suffix matching, or wildcard expansion is performed. Quality and fit
+// are the effective transform values, so replaying a signed URL with a
+// different quality or fit mode fails verification.
 func (s *Signer) buildSignatureData(req *Request) string {
 	return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s",
 		req.SourceHost,
-		escapePercentAndColon(req.SourcePath),
-		escapePercentAndColon(req.SourceQuery),
+		req.SourcePath,
+		req.SourceQuery,
 		req.Width,
 		req.Height,
 		req.Format,
@@ -175,14 +172,6 @@ func (s *Signer) buildSignatureData(req *Request) string {
 	)
 }
 
-// escapePercentAndColon returns s with each "%" written as "%25" and each
-// ":" as "%3A", so that the result holds no ":".
-func escapePercentAndColon(s string) string {
-	s = strings.ReplaceAll(s, "%", "%25")
-
-	return strings.ReplaceAll(s, ":", "%3A")
-}
-
 // ParseParams extracts signature and expiration from query parameters.
 func ParseParams(sig, expStr string) (string, time.Time, error) {
 	if expStr == "" {