Keep a single preview variant and verify read lock boundaries
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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},
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user