From 8abc79dc5b4062ca44ca99b9e61d4c3b09491f54 Mon Sep 17 00:00:00 2001 From: Codex Bot Date: Fri, 6 Nov 2026 21:43:48 +0800 Subject: [PATCH] Keep a single preview variant and verify read lock boundaries --- README.md | 7 +- internal/platform/asset_preview.go | 14 +--- internal/platform/asset_preview_test.go | 4 +- internal/platform/canvas_capabilities.go | 2 +- internal/platform/integration_test.go | 95 +++++++++++++++++++++++- 5 files changed, 103 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 66a2475..6884ed5 100644 --- a/README.md +++ b/README.md @@ -609,9 +609,8 @@ terminal status and authenticated canvas/asset rereads as completion evidence. ### Managed image previews Original asset bytes remain immutable and are used by the ordinary asset URL, -downloads and AI inputs. Authenticated `GET .../assets/{id}?variant=preview&width=512` -returns a bounded display rendition; widths are 256, 512, 1024 or 2048 (default -512). Opaque previews use JPEG quality 82, transparency stays PNG, GIF preserves +downloads and AI inputs. Authenticated `GET .../assets/{id}?variant=preview` +returns one display rendition bounded to 1024 pixels on its longest edge. Opaque previews use JPEG quality 82, transparency stays PNG, GIF preserves the original animation, and JPEG EXIF orientation is applied. Small originals may be returned unchanged when this is more efficient. No preview URL belongs in node `data.src`: persist the original managed URL and optional original @@ -620,7 +619,7 @@ in node `data.src`: persist the original managed URL and optional original Permissions are checked on every read, including cache hits. Originals are not replaced. Generation is serialized and the immutable encoded preview cache is bounded to 32 MiB / 128 entries. Current capabilities advertise `assets.previews` -and `assets.preview_widths`. No database migration is needed. Asset reads and +and `assets.preview_max_edge`. No database migration is needed. Asset reads and Canvas GET release their permission transaction before encoding/writing the response. Uploads release the initial workspace lock before receiving a large multipart body and recheck current write permission before committing. diff --git a/internal/platform/asset_preview.go b/internal/platform/asset_preview.go index 97c94df..46f9636 100644 --- a/internal/platform/asset_preview.go +++ b/internal/platform/asset_preview.go @@ -12,7 +12,6 @@ import ( "image/color" "image/jpeg" "image/png" - "strconv" "sync" "golang.org/x/image/draw" @@ -39,17 +38,10 @@ func previewWidth(variant, width string) (int, error) { if variant == "" && width == "" { return 0, nil } - if variant != "preview" { - return 0, errors.New("invalid_asset_variant") + if variant == "preview" && (width == "" || width == "1024") { + return 1024, nil } - if width == "" { - return 512, nil - } - n, err := strconv.Atoi(width) - if err != nil || (n != 256 && n != 512 && n != 1024 && n != 2048) { - return 0, errors.New("invalid_asset_variant") - } - return n, nil + return 0, errors.New("invalid_asset_variant") } // Cache only immutable bytes, after the caller checks current asset permission. diff --git a/internal/platform/asset_preview_test.go b/internal/platform/asset_preview_test.go index 7819767..bd213fe 100644 --- a/internal/platform/asset_preview_test.go +++ b/internal/platform/asset_preview_test.go @@ -63,12 +63,12 @@ func TestAssetPreviewPreservesTransparencyAndAnimation(t *testing.T) { } } func TestAssetPreviewRejectsUnboundedWidths(t *testing.T) { - for _, value := range []string{"0", "1", "513", "999999", "-1", "abc"} { + for _, value := range []string{"0", "1", "256", "512", "2048", "999999", "-1", "abc"} { if _, err := previewWidth("preview", value); err == nil { t.Errorf("accepted width %s", value) } } - for _, value := range []string{"256", "512", "1024", "2048"} { + for _, value := range []string{"", "1024"} { if _, err := previewWidth("preview", value); err != nil { t.Fatal(err) } diff --git a/internal/platform/canvas_capabilities.go b/internal/platform/canvas_capabilities.go index d09a956..9a5e33e 100644 --- a/internal/platform/canvas_capabilities.go +++ b/internal/platform/canvas_capabilities.go @@ -49,7 +49,7 @@ func (s *service) canvasCapabilities(w http.ResponseWriter, r *http.Request, pro "edge_features": response{"appearance": true, "markers": true, "bezier_chain": true, "animation_configuration": true, "execution_events": true}, "subgraphs": response{"create": true, "migrate": true, "undo": true, "members": true, "dissolve": true, "connections": true}, "ai": ai, - "assets": response{"previews": true, "preview_widths": []int{256, 512, 1024, 2048}, "originals_preserved": true}, + "assets": response{"previews": true, "preview_max_edge": 1024, "originals_preserved": true}, "limits": response{"max_depth": 8, "max_nodes": 500, "max_edges": 2000, "max_frame_ports": 4000, "max_curve_knots": 64, "max_snapshot_bytes": canvasLimit, "max_active_tasks": 128, "max_run_steps": 64, "max_execution_depth": 32, "max_input_images": 4, "max_input_image_bytes": 20 << 20, "max_coordinate": 1000000}, "retention": response{"idempotency_min_seconds": 86400, "undo_receipt_seconds": 604800}, }) diff --git a/internal/platform/integration_test.go b/internal/platform/integration_test.go index dadc5ba..d786eff 100644 --- a/internal/platform/integration_test.go +++ b/internal/platform/integration_test.go @@ -12,6 +12,7 @@ import ( "gitea.entroia.com/Zayden-Jung/foundry-core/internal/auth" "image" "image/png" + "io" "mime/multipart" "net/http" "net/http/httptest" @@ -19,7 +20,9 @@ import ( "regexp" "strconv" "strings" + "sync" "testing" + "time" _ "github.com/jackc/pgx/v5/stdlib" ) @@ -535,7 +538,7 @@ func TestOnboardingAndSharing(t *testing.T) { token string status int }{{tokens[0], 200}, {tokens[1], 404}, {tokens[2], 404}} { - request := httptest.NewRequest("GET", workspacePath+"/assets/"+uploaded["id"].(string)+"?variant=preview&width=256", nil) + request := httptest.NewRequest("GET", workspacePath+"/assets/"+uploaded["id"].(string)+"?variant=preview", nil) request.Header.Set("Authorization", "Bearer "+sample.token) result := httptest.NewRecorder() mux.ServeHTTP(result, request) @@ -546,6 +549,80 @@ func TestOnboardingAndSharing(t *testing.T) { t.Fatal("preview changed small original or privacy headers") } } + // Neither a snapshot nor an asset response may keep the workspace locked + // while a slow downstream consumer is accepting its body. + for _, path := range []string{canvasPath, workspacePath + "/assets/" + uploaded["id"].(string)} { + var probeErr error + writer := &readLockProbe{ResponseRecorder: httptest.NewRecorder(), probe: func() { + lockTx, e := db.BeginTx(ctx, nil) + if e != nil { + probeErr = e + return + } + defer lockTx.Rollback() + _, probeErr = lockTx.ExecContext(ctx, "SELECT id FROM workspaces WHERE project=$1 AND id=$2 FOR UPDATE NOWAIT", project, workspace) + }} + request := httptest.NewRequest("GET", path, nil) + request.Header.Set("Authorization", "Bearer "+tokens[0]) + request.Header.Set("Accept", accept) + mux.ServeHTTP(writer, request) + if writer.Code != 200 || probeErr != nil { + t.Fatalf("read held workspace lock during response: status=%d error=%v", writer.Code, probeErr) + } + } + // A slow multipart body must not hold the workspace lock either. + func() { + reader, pipe := io.Pipe() + defer reader.Close() + defer pipe.Close() + form := multipart.NewWriter(pipe) + headers := make(chan struct{}) + release := make(chan struct{}) + done := make(chan struct{}) + var once sync.Once + unblock := func() { once.Do(func() { close(release) }) } + defer unblock() + request := httptest.NewRequest("POST", workspacePath+"/assets", reader) + request.Header.Set("Authorization", "Bearer "+tokens[0]) + request.Header.Set("Content-Type", form.FormDataContentType()) + request.Header.Set("Idempotency-Key", "99999999-9999-4999-8999-999999999998") + result := httptest.NewRecorder() + go func() { mux.ServeHTTP(result, request); close(done) }() + go func() { + part, e := form.CreateFormFile("image", "slow.png") + close(headers) + if e != nil { + return + } + <-release + _, _ = part.Write(assetPNG.Bytes()) + _ = form.Close() + _ = pipe.Close() + }() + select { + case <-headers: + case <-time.After(5 * time.Second): + t.Fatal("upload did not consume headers") + } + lockTx, e := db.BeginTx(ctx, nil) + if e != nil { + t.Fatal(e) + } + _, e = lockTx.ExecContext(ctx, "SELECT id FROM workspaces WHERE project=$1 AND id=$2 FOR UPDATE NOWAIT", project, workspace) + _ = lockTx.Rollback() + unblock() + if e != nil { + t.Fatalf("upload held workspace lock while receiving body: %v", e) + } + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("upload did not finish") + } + if result.Code != 201 { + t.Fatalf("slow upload failed: %d", result.Code) + } + }() download(tokens[0], 200) download(tokens[1], 404) download(tokens[2], 404) @@ -684,3 +761,19 @@ func TestOnboardingAndSharing(t *testing.T) { t.Fatalf("click accounting incorrect %v", err) } } + +// A real SQL NOWAIT probe at the response-write boundary catches locks that +// would otherwise remain invisible in a buffered httptest recorder. +type readLockProbe struct { + *httptest.ResponseRecorder + probe func() +} + +func (w *readLockProbe) Write(body []byte) (int, error) { + if w.probe != nil { + probe := w.probe + w.probe = nil + probe() + } + return w.ResponseRecorder.Write(body) +}