From 67765c17f453b3538223df399491710107c525e9 Mon Sep 17 00:00:00 2001 From: Codex Bot Date: Fri, 6 Nov 2026 21:36:07 +0800 Subject: [PATCH 1/2] Add authenticated image previews and shorten canvas read locks --- README.md | 25 +++ internal/platform/asset_preview.go | 250 +++++++++++++++++++++++ internal/platform/asset_preview_test.go | 137 +++++++++++++ internal/platform/assets.go | 37 ++++ internal/platform/canvas.go | 3 + internal/platform/canvas_capabilities.go | 4 + internal/platform/integration_test.go | 16 ++ 7 files changed, 472 insertions(+) create mode 100644 internal/platform/asset_preview.go create mode 100644 internal/platform/asset_preview_test.go diff --git a/README.md b/README.md index c23d762..66a2475 100644 --- a/README.md +++ b/README.md @@ -604,3 +604,28 @@ api "$B/ai/runs/$(jq -r '.run_id' <<<"$propagated_run")?after_seq=0&limit=100" The live catalog, rather than the isolated verification IDs above, determines whether each request can run. A 202 response acknowledges a queued job; use its 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 +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 +`assetId`, then select the display variant only in the renderer. + +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 +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. + +Legacy inline images must be migrated through an explicit editor action: upload +unchanged bytes with idempotency keys, then save the same node IDs/geometry with +managed asset references and the current canvas version. Opening a canvas does +not migrate it or cause an implicit PATCH. diff --git a/internal/platform/asset_preview.go b/internal/platform/asset_preview.go new file mode 100644 index 0000000..97c94df --- /dev/null +++ b/internal/platform/asset_preview.go @@ -0,0 +1,250 @@ +package platform + +import ( + "bytes" + "container/list" + "context" + "crypto/sha256" + "encoding/binary" + "errors" + "fmt" + "image" + "image/color" + "image/jpeg" + "image/png" + "strconv" + "sync" + + "golang.org/x/image/draw" +) + +type assetPreviewResult struct { + data []byte + mime string +} +type assetPreviewEntry struct { + key string + value assetPreviewResult +} + +var previewMemory = struct { + sync.Mutex + entries map[string]*list.Element + order *list.List + bytes int +}{entries: make(map[string]*list.Element), order: list.New()} +var previewSlots = make(chan struct{}, 1) + +func previewWidth(variant, width string) (int, error) { + if variant == "" && width == "" { + return 0, nil + } + if variant != "preview" { + return 0, errors.New("invalid_asset_variant") + } + 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 +} + +// Cache only immutable bytes, after the caller checks current asset permission. +// Bound both decoded concurrency and retained encoded data. +func cachedAssetPreview(ctx context.Context, encoded []byte, mime string, width int) (assetPreviewResult, error) { + key := fmt.Sprintf("%x:%d", sha256.Sum256(encoded), width) + previewMemory.Lock() + if entry := previewMemory.entries[key]; entry != nil { + previewMemory.order.MoveToFront(entry) + result := entry.Value.(assetPreviewEntry).value + previewMemory.Unlock() + return result, nil + } + previewMemory.Unlock() + select { + case previewSlots <- struct{}{}: + case <-ctx.Done(): + return assetPreviewResult{}, ctx.Err() + } + defer func() { <-previewSlots }() + // Another request may have finished this same preview while we waited. + previewMemory.Lock() + if entry := previewMemory.entries[key]; entry != nil { + result := entry.Value.(assetPreviewEntry).value + previewMemory.order.MoveToFront(entry) + previewMemory.Unlock() + return result, nil + } + previewMemory.Unlock() + result, err := makeAssetPreview(encoded, mime, width) + if err != nil { + return result, err + } + if len(result.data) > 8<<20 { + return result, nil + } + previewMemory.Lock() + defer previewMemory.Unlock() + for previewMemory.bytes+len(result.data) > 32<<20 || previewMemory.order.Len() >= 128 { + tail := previewMemory.order.Back() + if tail == nil { + break + } + old := tail.Value.(assetPreviewEntry) + previewMemory.bytes -= len(old.value.data) + delete(previewMemory.entries, old.key) + previewMemory.order.Remove(tail) + } + previewMemory.entries[key] = previewMemory.order.PushFront(assetPreviewEntry{key, result}) + previewMemory.bytes += len(result.data) + return result, nil +} + +func makeAssetPreview(encoded []byte, mime string, width int) (assetPreviewResult, error) { + original := assetPreviewResult{encoded, mime} + // Keep animation intact. Original bytes remain the canonical AI/download input. + if mime == "image/gif" { + return original, nil + } + config, _, err := image.DecodeConfig(bytes.NewReader(encoded)) + if err != nil { + return original, err + } + if config.Width <= 0 || config.Height <= 0 || int64(config.Width)*int64(config.Height) > assetPixelLimit { + return original, errors.New("invalid_image_dimensions") + } + if config.Width <= width && config.Height <= width && len(encoded) <= 64<<10 { + return original, nil + } + source, _, err := image.Decode(bytes.NewReader(encoded)) + if err != nil { + return original, err + } + orientation := jpegOrientation(encoded) + if orientation != 1 { + source = orientedAsset{source, orientation} + } + bounds := source.Bounds() + w, h := bounds.Dx(), bounds.Dy() + if w > width || h > width { + if w >= h { + h = max(1, h*width/w) + w = width + } else { + w = max(1, w*width/h) + h = width + } + } + resized := image.NewNRGBA(image.Rect(0, 0, w, h)) + draw.CatmullRom.Scale(resized, resized.Bounds(), source, source.Bounds(), draw.Src, nil) + var output bytes.Buffer + outputMime := "image/png" + if resized.Opaque() { + err = jpeg.Encode(&output, resized, &jpeg.Options{Quality: 82}) + outputMime = "image/jpeg" + } else { + err = png.Encode(&output, resized) + } + if err != nil { + return original, err + } + if output.Len() >= len(encoded) && orientation == 1 { + return original, nil + } + return assetPreviewResult{output.Bytes(), outputMime}, nil +} + +// JPEG EXIF orientation is applied to previews instead of silently rotating a +// phone photograph after its EXIF metadata is removed by the image encoder. +func jpegOrientation(data []byte) int { + if len(data) < 4 || data[0] != 0xff || data[1] != 0xd8 { + return 1 + } + for at := 2; at+4 <= len(data); { + if data[at] != 0xff { + return 1 + } + marker := data[at+1] + if marker == 0xda || marker == 0xd9 { + return 1 + } + length := int(binary.BigEndian.Uint16(data[at+2 : at+4])) + if length < 2 || at+2+length > len(data) { + return 1 + } + part := data[at+4 : at+2+length] + if marker == 0xe1 && len(part) >= 14 && string(part[:6]) == "Exif\x00\x00" { + tiff := part[6:] + var order binary.ByteOrder + switch string(tiff[:2]) { + case "II": + order = binary.LittleEndian + case "MM": + order = binary.BigEndian + default: + return 1 + } + if order.Uint16(tiff[2:4]) != 42 { + return 1 + } + offset := int(order.Uint32(tiff[4:8])) + if offset < 8 || offset+2 > len(tiff) { + return 1 + } + count := int(order.Uint16(tiff[offset : offset+2])) + for i := 0; i < count; i++ { + entry := offset + 2 + i*12 + if entry+12 > len(tiff) { + return 1 + } + if order.Uint16(tiff[entry:entry+2]) == 0x112 && order.Uint16(tiff[entry+2:entry+4]) == 3 && order.Uint32(tiff[entry+4:entry+8]) == 1 { + value := int(order.Uint16(tiff[entry+8 : entry+10])) + if value >= 1 && value <= 8 { + return value + } + return 1 + } + } + } + at += 2 + length + } + return 1 +} + +type orientedAsset struct { + image.Image + orientation int +} + +func (im orientedAsset) Bounds() image.Rectangle { + b := im.Image.Bounds() + if im.orientation >= 5 { + return image.Rect(0, 0, b.Dy(), b.Dx()) + } + return image.Rect(0, 0, b.Dx(), b.Dy()) +} +func (im orientedAsset) ColorModel() color.Model { return im.Image.ColorModel() } +func (im orientedAsset) At(x, y int) color.Color { + b := im.Image.Bounds() + w, h := b.Dx(), b.Dy() + switch im.orientation { + case 2: + x = w - 1 - x + case 3: + x, y = w-1-x, h-1-y + case 4: + y = h - 1 - y + case 5: + x, y = y, x + case 6: + x, y = y, h-1-x + case 7: + x, y = w-1-y, h-1-x + case 8: + x, y = w-1-y, x + } + return im.Image.At(b.Min.X+x, b.Min.Y+y) +} diff --git a/internal/platform/asset_preview_test.go b/internal/platform/asset_preview_test.go new file mode 100644 index 0000000..7819767 --- /dev/null +++ b/internal/platform/asset_preview_test.go @@ -0,0 +1,137 @@ +package platform + +import ( + "bytes" + "context" + "encoding/binary" + "image" + "image/color" + "image/jpeg" + "image/png" + "testing" +) + +func TestAssetPreviewDownscalesWithoutChangingOriginal(t *testing.T) { + source := image.NewNRGBA(image.Rect(0, 0, 1600, 900)) + for y := 0; y < 900; y++ { + for x := 0; x < 1600; x++ { + source.SetNRGBA(x, y, color.NRGBA{uint8(x), uint8(y), uint8(x * y), 255}) + } + } + var encoded bytes.Buffer + if err := png.Encode(&encoded, source); err != nil { + t.Fatal(err) + } + original := append([]byte(nil), encoded.Bytes()...) + preview, err := makeAssetPreview(encoded.Bytes(), "image/png", 512) + if err != nil { + t.Fatal(err) + } + config, format, err := image.DecodeConfig(bytes.NewReader(preview.data)) + if err != nil || config.Width != 512 || config.Height != 288 || format != "jpeg" { + t.Fatalf("bad preview %s %dx%d %v", format, config.Width, config.Height, err) + } + if len(preview.data) >= len(original) || !bytes.Equal(original, encoded.Bytes()) { + t.Fatal("preview must shrink without changing original") + } + cached, err := cachedAssetPreview(context.Background(), original, "image/png", 512) + if err != nil || !bytes.Equal(cached.data, preview.data) { + t.Fatal("cache result mismatch") + } +} +func TestAssetPreviewPreservesTransparencyAndAnimation(t *testing.T) { + source := image.NewNRGBA(image.Rect(0, 0, 800, 400)) + source.SetNRGBA(1, 1, color.NRGBA{255, 0, 0, 255}) + var encoded bytes.Buffer + _ = png.Encode(&encoded, source) + preview, err := makeAssetPreview(encoded.Bytes(), "image/png", 256) + if err != nil { + t.Fatal(err) + } + decoded, format, err := image.Decode(bytes.NewReader(preview.data)) + if err != nil || format != "png" { + t.Fatal("transparent image must stay PNG") + } + _, _, _, alpha := decoded.At(200, 100).RGBA() + if alpha != 0 { + t.Fatal("transparent background was lost") + } + original := []byte("GIF original animation bytes") + gif, err := makeAssetPreview(original, "image/gif", 512) + if err != nil || !bytes.Equal(gif.data, original) { + t.Fatal("GIF animation changed") + } +} +func TestAssetPreviewRejectsUnboundedWidths(t *testing.T) { + for _, value := range []string{"0", "1", "513", "999999", "-1", "abc"} { + if _, err := previewWidth("preview", value); err == nil { + t.Errorf("accepted width %s", value) + } + } + for _, value := range []string{"256", "512", "1024", "2048"} { + if _, err := previewWidth("preview", value); err != nil { + t.Fatal(err) + } + } + if n, err := previewWidth("", ""); err != nil || n != 0 { + t.Fatal("original request changed") + } + if _, err := previewWidth("other", ""); err == nil { + t.Fatal("unknown variant accepted") + } + if _, err := makeAssetPreview([]byte("invalid"), "image/png", 512); err == nil { + t.Fatal("malformed image accepted") + } +} +func TestAssetPreviewAppliesJPEGOrientation(t *testing.T) { + source := image.NewNRGBA(image.Rect(0, 0, 800, 400)) + for y := 0; y < 400; y++ { + for x := 0; x < 800; x++ { + source.SetNRGBA(x, y, color.NRGBA{255, 0, 0, 255}) + } + } + var original bytes.Buffer + _ = jpeg.Encode(&original, source, nil) + tiff := make([]byte, 26) + copy(tiff, "II") + binary.LittleEndian.PutUint16(tiff[2:], 42) + binary.LittleEndian.PutUint32(tiff[4:], 8) + binary.LittleEndian.PutUint16(tiff[8:], 1) + binary.LittleEndian.PutUint16(tiff[10:], 0x112) + binary.LittleEndian.PutUint16(tiff[12:], 3) + binary.LittleEndian.PutUint32(tiff[14:], 1) + binary.LittleEndian.PutUint16(tiff[18:], 6) + exif := append([]byte("Exif\x00\x00"), tiff...) + segment := append([]byte{0xff, 0xe1, 0, byte(len(exif) + 2)}, exif...) + encoded := append(append(append([]byte{}, original.Bytes()[:2]...), segment...), original.Bytes()[2:]...) + if jpegOrientation(encoded) != 6 { + t.Fatal("EXIF orientation not decoded") + } + preview, err := makeAssetPreview(encoded, "image/jpeg", 256) + if err != nil { + t.Fatal(err) + } + config, _, err := image.DecodeConfig(bytes.NewReader(preview.data)) + if err != nil || config.Width != 128 || config.Height != 256 { + t.Fatalf("rotated dimensions %dx%d %v", config.Width, config.Height, err) + } +} +func TestAllAssetOrientationMappings(t *testing.T) { + source := image.NewNRGBA(image.Rect(0, 0, 3, 2)) + source.SetNRGBA(0, 0, color.NRGBA{255, 0, 0, 255}) + for orientation := 1; orientation <= 8; orientation++ { + oriented := orientedAsset{source, orientation} + red := 0 + for y := 0; y < oriented.Bounds().Dy(); y++ { + for x := 0; x < oriented.Bounds().Dx(); x++ { + r, _, _, _ := oriented.At(x, y).RGBA() + if r > 0 { + red++ + } + } + } + if red != 1 { + t.Errorf("orientation %d loses/repeats pixels", orientation) + } + } +} diff --git a/internal/platform/assets.go b/internal/platform/assets.go index 0a3c933..27cbbfa 100644 --- a/internal/platform/assets.go +++ b/internal/platform/assets.go @@ -79,6 +79,10 @@ func (s *service) uploadAsset(w http.ResponseWriter, r *http.Request, project, a reject(w, 403, "forbidden") return nil } + // Do not hold a workspace lock during a slow multipart upload. Recheck below. + if err = tx.Rollback(); err != nil { + return err + } r.Body = http.MaxBytesReader(w, r.Body, assetLimit+(64<<10)) reader, err := r.MultipartReader() if err != nil { @@ -123,6 +127,19 @@ func (s *service) uploadAsset(w http.ResponseWriter, r *http.Request, project, a reject(w, 400, "invalid_asset") return nil } + tx, err = s.db.BeginTx(r.Context(), nil) + if err != nil { + return err + } + defer tx.Rollback() + _, role, err = lockWorkspace(r.Context(), tx, project, workspace, account) + if err != nil { + return err + } + if role == "viewer" { + reject(w, 403, "forbidden") + return nil + } digest := sha256.Sum256(encoded) hash := hex.EncodeToString(digest[:]) info := assetInfo{MIME: mime, Bytes: len(encoded), Width: width, Height: height} @@ -204,6 +221,11 @@ func (s *service) getAsset(w http.ResponseWriter, r *http.Request, project, acco reject(w, 405, "method_not_allowed") return nil } + width, variantErr := previewWidth(r.URL.Query().Get("variant"), r.URL.Query().Get("width")) + if variantErr != nil { + reject(w, 400, "invalid_asset_variant") + return nil + } id := r.PathValue("asset") if !uuidSlugPattern.MatchString(id) { reject(w, 404, "not_found") @@ -235,6 +257,21 @@ func (s *service) getAsset(w http.ResponseWriter, r *http.Request, project, acco if err != nil { return err } + // Permission and immutable bytes are now read consistently. Release locks + // before resizing or waiting for a slow public client to consume the body. + if err = tx.Rollback(); err != nil { + return err + } + variant := "original" + if width > 0 { + preview, e := cachedAssetPreview(r.Context(), encoded, mime, width) + if e != nil { + return e + } + encoded, mime = preview.data, preview.mime + variant = "preview" + } + w.Header().Set("X-Asset-Variant", variant) w.Header().Set("Content-Type", mime) w.Header().Set("Content-Length", strconv.Itoa(len(encoded))) w.Header().Set("X-Content-Type-Options", "nosniff") diff --git a/internal/platform/canvas.go b/internal/platform/canvas.go index 76f7f40..6122529 100644 --- a/internal/platform/canvas.go +++ b/internal/platform/canvas.go @@ -345,6 +345,9 @@ func (s *service) canvas(w http.ResponseWriter, r *http.Request, project, accoun return nil } if r.Method == http.MethodGet { + if err = tx.Rollback(); err != nil { + return err + } w.Header().Set("X-Canvas-Schema-Version", strconv.Itoa(canvasSchema(document))) setVersion(w, version) reply(w, 200, response{"version": version, "document": document}) diff --git a/internal/platform/canvas_capabilities.go b/internal/platform/canvas_capabilities.go index 30391a2..d09a956 100644 --- a/internal/platform/canvas_capabilities.go +++ b/internal/platform/canvas_capabilities.go @@ -24,6 +24,9 @@ func (s *service) canvasCapabilities(w http.ResponseWriter, r *http.Request, pro reject(w, 400, "invalid_material") return nil } + if err = tx.Rollback(); err != nil { + return err + } ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second) defer cancel() models, modelErr := s.canvasAI.Models(ctx) @@ -46,6 +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}, "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 3286501..dadc5ba 100644 --- a/internal/platform/integration_test.go +++ b/internal/platform/integration_test.go @@ -530,6 +530,22 @@ func TestOnboardingAndSharing(t *testing.T) { t.Fatal("asset bytes changed") } } + // Warm the immutable preview cache, then verify it never bypasses permission. + for _, sample := range []struct { + 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.Header.Set("Authorization", "Bearer "+sample.token) + result := httptest.NewRecorder() + mux.ServeHTTP(result, request) + if result.Code != sample.status { + t.Fatalf("preview permission got %d want %d", result.Code, sample.status) + } + if result.Code == 200 && (!bytes.Equal(result.Body.Bytes(), assetPNG.Bytes()) || result.Header().Get("Cache-Control") != "private, no-store") { + t.Fatal("preview changed small original or privacy headers") + } + } download(tokens[0], 200) download(tokens[1], 404) download(tokens[2], 404) -- 2.52.0 From 8abc79dc5b4062ca44ca99b9e61d4c3b09491f54 Mon Sep 17 00:00:00 2001 From: Codex Bot Date: Fri, 6 Nov 2026 21:43:48 +0800 Subject: [PATCH 2/2] 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) +} -- 2.52.0