From 438a29429c861443616e35b7098fd19317a25a1e Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Thu, 13 Aug 2026 02:17:09 +0200 Subject: [PATCH 1/2] fix(cloudflare): honour fit and position in the image endpoint The endpoint parsed only w/h/f/q and built its own transform, so the `fit` Astro asks for never reached the Images binding. A request for a square cover-crop got width and height alone, and the binding fell back to its default fit: the image was scaled down inside the box and letterboxed instead of cropped to fill it. Astro's stock Cloudflare endpoint forwards `fit`, so the same markup cropped correctly on Node and did not on Cloudflare. Parse `fit` and `position` alongside the existing params and map them onto the binding's vocabulary, which only partly overlaps Astro's: `fill` is the binding's `squeeze`, `inside` is `contain`, and sharp's compass names are the edges they describe. Values with no binding equivalent -- `outside`, a compound `left top` -- are dropped rather than swapped for a fit that would crop, leaving the previous behaviour rather than a confidently wrong one. Both params are advisory: an unrecognised value is dropped instead of failing the request, because Astro's ImageFit is open-ended and each backend supports a different subset. Also adds @emdash-cms/cloudflare to test:unit. CI runs only that script, so the package's tests -- including this regression test -- never ran. Closes #2228 --- .changeset/cf-image-endpoint-fit.md | 6 + package.json | 2 +- packages/cloudflare/src/image-endpoint.ts | 68 +++++++++- .../tests/image-endpoint-fit.test.ts | 122 ++++++++++++++++++ packages/core/src/media/image-endpoint.ts | 63 ++++++++- .../tests/unit/media/image-endpoint.test.ts | 36 ++++++ 6 files changed, 291 insertions(+), 6 deletions(-) create mode 100644 .changeset/cf-image-endpoint-fit.md create mode 100644 packages/cloudflare/tests/image-endpoint-fit.test.ts diff --git a/.changeset/cf-image-endpoint-fit.md b/.changeset/cf-image-endpoint-fit.md new file mode 100644 index 0000000000..03024bbd50 --- /dev/null +++ b/.changeset/cf-image-endpoint-fit.md @@ -0,0 +1,6 @@ +--- +"@emdash-cms/cloudflare": patch +"emdash": patch +--- + +Fixes cropping for EmDash media on Cloudflare. Images asked for a `cover` crop — square avatars, fixed-ratio thumbnails — came back scaled down and letterboxed inside the requested box instead of filling it, because the endpoint never passed the requested fit to the Images binding. `fit` and `position` are now honoured, so a crop crops and its focal side is respected. diff --git a/package.json b/package.json index 9f2060cde7..c4e6b42731 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "typecheck:templates": "pnpm run --workspace-concurrency=1 --filter {./templates/*} typecheck", "check": "pnpm run typecheck && pnpm run --filter {./packages/*} check", "test": "pnpm run --filter {./packages/*} test", - "test:unit": "pnpm run --filter emdash --filter @emdash-cms/auth --filter @emdash-cms/blocks --filter @emdash-cms/gutenberg-to-portable-text --filter @emdash-cms/marketplace --filter @emdash-cms/plugin-cli --filter @emdash-cms/plugin-forms --filter @emdash-cms/plugin-types --filter @emdash-cms/registry-client --filter @emdash-cms/registry-lexicons test", + "test:unit": "pnpm run --filter emdash --filter @emdash-cms/auth --filter @emdash-cms/blocks --filter @emdash-cms/cloudflare --filter @emdash-cms/gutenberg-to-portable-text --filter @emdash-cms/marketplace --filter @emdash-cms/plugin-cli --filter @emdash-cms/plugin-forms --filter @emdash-cms/plugin-types --filter @emdash-cms/registry-client --filter @emdash-cms/registry-lexicons test", "test:browser": "pnpm run --filter @emdash-cms/admin test", "test:e2e": "playwright test", "test:e2e:ui": "playwright test --ui", diff --git a/packages/cloudflare/src/image-endpoint.ts b/packages/cloudflare/src/image-endpoint.ts index c11ea91887..335719786d 100644 --- a/packages/cloudflare/src/image-endpoint.ts +++ b/packages/cloudflare/src/image-endpoint.ts @@ -21,6 +21,7 @@ import { originalMediaHeaders, parseTransformParams, resolveTransformQuality, + type ImageTransformFit, type ImageTransformFormat, } from "emdash/media/image-endpoint"; @@ -33,6 +34,60 @@ const FORMAT_MIME: Record = png: "image/png", }; +/** + * Astro's `fit` vocabulary mapped onto the Images binding's. + * + * Most names line up. Astro's `fill` distorts the image to fill the box, which + * the binding calls `squeeze`. `inside` (accepted by Astro's sharp service) + * resizes to fit within the box, which is the binding's `contain`. `outside` + * has no binding equivalent -- nothing there resizes to *exceed* the box + * without cropping -- so it is dropped and the dimensions apply on their own, + * rather than substituting a fit that would crop. + */ +const FIT_TO_BINDING: Record = { + fill: "squeeze", + contain: "contain", + cover: "cover", + "scale-down": "scale-down", + inside: "contain", + outside: undefined, +}; + +/** + * Astro's `position` mapped onto the binding's `gravity`. + * + * Astro passes sharp's vocabulary straight through, and the two only partly + * overlap. The shared keywords pass unchanged; sharp's compass names mean the + * same edges under different words; `centre` is the same as `center`; and + * `attention` (sharp's saliency-based crop) is the binding's `auto`. + * + * A compound position such as `left top` names a corner, which the binding can + * only express as coordinates. Rather than guess at them, it is dropped and the + * binding keeps its own gravity -- the same outcome as before this mapping + * existed, and better than cropping to a confidently wrong edge. + */ +const GRAVITY_BY_POSITION = new Map([ + ["face", "face"], + ["left", "left"], + ["right", "right"], + ["top", "top"], + ["bottom", "bottom"], + ["center", "center"], + ["centre", "center"], + ["auto", "auto"], + ["entropy", "entropy"], + ["attention", "auto"], + // sharp's compass vocabulary. + ["north", "top"], + ["south", "bottom"], + ["east", "right"], + ["west", "left"], +]); + +function toBindingGravity(position: string): ImageTransform["gravity"] | undefined { + return GRAVITY_BY_POSITION.get(position.trim().toLowerCase()); +} + /** Resolve the Images binding by the name the Cloudflare adapter configured. */ function resolveImagesBinding(): ImagesBinding | undefined { const configured = (globalThis as { __ASTRO_IMAGES_BINDING_NAME?: unknown }) @@ -82,11 +137,22 @@ export const GET: APIRoute = async (ctx) => { return streamOriginal(source.body, source.contentType); } - const { width, height, format, quality } = parsed.options; + const { width, height, format, quality, fit, position } = parsed.options; const outputMime = FORMAT_MIME[format] ?? "image/webp"; const transform: ImageTransform = {}; if (width) transform.width = width; if (height) transform.height = height; + // Without these the binding falls back to its own default fit, so a + // square `fit=cover` request came back scaled-down and letterboxed + // instead of cropped. + if (fit) { + const bindingFit = FIT_TO_BINDING[fit]; + if (bindingFit) transform.fit = bindingFit; + } + if (position) { + const gravity = toBindingGravity(position); + if (gravity) transform.gravity = gravity; + } // Lossy formats get an explicit quality: the Images binding has no // default of its own and encodes near-losslessly without one, producing // renditions several times the size of the original. PNG is exempt — diff --git a/packages/cloudflare/tests/image-endpoint-fit.test.ts b/packages/cloudflare/tests/image-endpoint-fit.test.ts new file mode 100644 index 0000000000..55a9e7912f --- /dev/null +++ b/packages/cloudflare/tests/image-endpoint-fit.test.ts @@ -0,0 +1,122 @@ +import type { APIContext } from "astro"; +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const adapterGET = vi.fn(() => new Response("adapter", { status: 200 })); +vi.mock("@astrojs/cloudflare/image-transform-endpoint", () => ({ GET: adapterGET })); + +/** Records the transform the endpoint hands to the Images binding. */ +const transform = vi.fn(); +const output = vi.fn(() => ({ + response: () => new Response("bytes", { headers: { "Content-Type": "image/webp" } }), +})); + +const images = { + input: () => ({ + transform: (options: unknown) => { + transform(options); + return { output }; + }, + }), +}; + +vi.mock("cloudflare:workers", () => ({ env: { IMAGES: images } })); + +const { GET } = await import("../src/image-endpoint.js"); + +const storage = { + download: async () => ({ + body: new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array([1, 2, 3])); + controller.close(); + }, + }), + contentType: "image/jpeg", + }), +}; + +/** Request the endpoint the way Astro's image service does. */ +function request(params: string): Promise { + const href = encodeURIComponent("/_emdash/api/media/file/01J5ABC.webp"); + const ctx = { + request: new Request(`https://example.com/_image?href=${href}&${params}`), + locals: { emdash: { storage } }, + } as unknown as APIContext; + return GET(ctx) as Promise; +} + +/** The options passed to the Images binding for the most recent request. */ +function lastTransform(): Record { + // eslint-disable-next-line typescript/no-unsafe-type-assertion -- vitest mock call args + return transform.mock.calls.at(-1)?.[0] as Record; +} + +beforeEach(() => { + transform.mockClear(); +}); + +describe("Cloudflare image endpoint: fit and position", () => { + it("forwards fit=cover so a square crop crops instead of letterboxing", async () => { + // Astro emits `fit`/`position` for constrained images. Dropping `fit` + // left the binding on its default, so a 32x32 avatar came back + // scaled-down inside the box rather than cover-cropped to fill it. + await request("w=32&h=32&f=webp&fit=cover&position=center"); + + expect(lastTransform()).toMatchObject({ width: 32, height: 32, fit: "cover" }); + }); + + it("maps position to the binding's gravity vocabulary", async () => { + await request("w=32&h=32&fit=cover&position=top"); + expect(lastTransform().gravity).toBe("top"); + + // sharp spells it "centre" and calls saliency-based cropping + // "attention"; the binding uses "center" and "auto". + await request("w=32&h=32&fit=cover&position=centre"); + expect(lastTransform().gravity).toBe("center"); + + await request("w=32&h=32&fit=cover&position=attention"); + expect(lastTransform().gravity).toBe("auto"); + }); + + it("maps sharp's compass names onto the edges they mean", async () => { + // Astro forwards sharp's vocabulary verbatim, so `position="north"` + // reaches the endpoint and means the same edge as `top`. + for (const [position, gravity] of [ + ["north", "top"], + ["south", "bottom"], + ["east", "right"], + ["west", "left"], + ]) { + await request(`w=32&h=32&fit=cover&position=${position}`); + expect(lastTransform().gravity).toBe(gravity); + } + }); + + it("maps Astro's sharp-only inside fit onto contain", async () => { + await request("w=64&h=64&fit=inside"); + expect(lastTransform().fit).toBe("contain"); + }); + + it("maps Astro's fill to the binding's squeeze", async () => { + await request("w=64&h=64&fit=fill"); + expect(lastTransform().fit).toBe("squeeze"); + }); + + it("omits fit and gravity the binding would reject rather than forwarding them", async () => { + // `outside` resizes to exceed the box without cropping, which the binding + // cannot express, and a compound position names a corner it has no keyword + // for. Both are dropped so the rendition still resolves, and neither is + // swapped for a fit that would crop. + await request("w=64&h=64&fit=outside&position=top%20left"); + + const options = lastTransform(); + expect(options.fit).toBeUndefined(); + expect(options.gravity).toBeUndefined(); + expect(options).toMatchObject({ width: 64, height: 64 }); + }); + + it("leaves fit unset when the request carries none", async () => { + await request("w=800&f=webp"); + expect(lastTransform().fit).toBeUndefined(); + }); +}); diff --git a/packages/core/src/media/image-endpoint.ts b/packages/core/src/media/image-endpoint.ts index a524fc5eb8..76ccc178e8 100644 --- a/packages/core/src/media/image-endpoint.ts +++ b/packages/core/src/media/image-endpoint.ts @@ -37,6 +37,33 @@ export const MAX_TRANSFORM_DIMENSION = 4000; /** A format string accepted by {@link ImageTransformOptions.format}. */ export type ImageTransformFormat = (typeof ALLOWED_TRANSFORM_FORMATS)[number]; +/** + * The `fit` values Astro's image service emits. Astro's `ImageFit` is + * open-ended (`string & {}`), so this is an allowlist: an unrecognised fit is + * dropped rather than forwarded to a backend that would reject it. + * + * `inside` and `outside` are not in `ImageFit` but are accepted by Astro's + * sharp service, so a site can already use them on Node. `none` is absent on + * purpose: Astro deletes it before building the URL, so it never reaches an + * endpoint. + */ +export const ALLOWED_TRANSFORM_FITS = [ + "fill", + "contain", + "cover", + "scale-down", + "inside", + "outside", +] as const; + +/** A fit string accepted by {@link ImageTransformOptions.fit}. */ +export type ImageTransformFit = (typeof ALLOWED_TRANSFORM_FITS)[number]; + +/** Type guard for {@link ImageTransformFit}. */ +export function isTransformFit(value: string): value is ImageTransformFit { + return (ALLOWED_TRANSFORM_FITS as readonly string[]).includes(value); +} + /** Validated options for a single transform. */ export interface ImageTransformOptions { width?: number; @@ -48,6 +75,19 @@ export interface ImageTransformOptions { * {@link DEFAULT_TRANSFORM_QUALITY}); lossless PNG deliberately gets none. */ quality?: number; + /** + * How the rendition fills the requested box, from Astro's `fit`. Left + * `undefined` when the request carried none or carried a value outside + * {@link ALLOWED_TRANSFORM_FITS}, so callers keep their backend's default. + */ + fit?: ImageTransformFit; + /** + * Which part of the image survives a crop, from Astro's `position`. Only + * meaningful for fits that crop. Kept as the raw string: the vocabulary is + * the backend's (a keyword like `center`, or coordinates), so mapping it is + * the adapter's job. + */ + position?: string; } /** Long-lived immutable cache -- transform output is deterministic per key+params. */ @@ -150,13 +190,19 @@ export function resolveTransformQuality( } /** - * Parse and validate `?w=&h=&f=&q=` query params. Width is required (it sizes - * the rendition); dimensions are bounded so a request can't ask for an - * unbounded or nonsensical transform. Format falls back to + * Parse and validate `?w=&h=&f=&q=&fit=&position=` query params. Width is + * required (it sizes the rendition); dimensions are bounded so a request can't + * ask for an unbounded or nonsensical transform. Format falls back to * {@link DEFAULT_TRANSFORM_FORMAT} when not requested. `q` is validated when * present but otherwise left `undefined` so the caller can apply a per-format * default (lossy formats get one, lossless PNG does not — see * {@link DEFAULT_TRANSFORM_QUALITY}). + * + * `fit` and `position` describe how the rendition fills its box. Unlike the + * others they are advisory: an unrecognised value is dropped rather than + * failing the request, because Astro's `ImageFit` is open-ended and each + * backend supports a different subset. Mapping them onto a backend's + * vocabulary is the platform endpoint's job. */ export function parseTransformParams(params: URLSearchParams): ParsedTransformParams { const width = parseDimension(params.get("w")); @@ -185,7 +231,16 @@ export function parseTransformParams(params: URLSearchParams): ParsedTransformPa quality = q; } - return { ok: true, options: { width, height, format, quality } }; + // `fit` and `position` come straight from Astro's image service. Both are + // advisory: an unrecognised value is dropped rather than failing the + // request, so a rendition still resolves on a backend that doesn't know it. + const fitRaw = params.get("fit"); + const fit = fitRaw !== null && isTransformFit(fitRaw) ? fitRaw : undefined; + + const positionRaw = params.get("position"); + const position = positionRaw !== null && positionRaw !== "" ? positionRaw : undefined; + + return { ok: true, options: { width, height, format, quality, fit, position } }; } /** diff --git a/packages/core/tests/unit/media/image-endpoint.test.ts b/packages/core/tests/unit/media/image-endpoint.test.ts index f6d567d38b..79ae804adf 100644 --- a/packages/core/tests/unit/media/image-endpoint.test.ts +++ b/packages/core/tests/unit/media/image-endpoint.test.ts @@ -114,6 +114,42 @@ describe("parseTransformParams", () => { expect(parse("w=640&h=-1").ok).toBe(false); }); + it("parses fit and position from Astro's image service", () => { + const r = parse("w=32&h=32&f=webp&fit=cover&position=center"); + expect(r.ok).toBe(true); + if (r.ok) { + expect(r.options.fit).toBe("cover"); + expect(r.options.position).toBe("center"); + } + }); + + it("drops an unrecognised fit instead of failing the request", () => { + // Astro's ImageFit is open-ended (`string & {}`), so an unknown value + // must not break a rendition -- the backend keeps its own default. + const r = parse("w=32&fit=lopsided"); + expect(r.ok).toBe(true); + if (r.ok) expect(r.options.fit).toBeUndefined(); + }); + + it("accepts the fits Astro's sharp service adds beyond ImageFit", () => { + // `inside`/`outside` are absent from Astro's ImageFit union but accepted + // by its sharp service, so a site can already be using them on Node. + for (const fit of ["inside", "outside"]) { + const r = parse(`w=32&fit=${fit}`); + expect(r.ok).toBe(true); + if (r.ok) expect(r.options.fit).toBe(fit); + } + }); + + it("leaves fit and position undefined when not requested", () => { + const r = parse("w=800"); + expect(r.ok).toBe(true); + if (r.ok) { + expect(r.options.fit).toBeUndefined(); + expect(r.options.position).toBeUndefined(); + } + }); + it("rejects unsupported format and bad quality", () => { expect(parse("w=640&f=gif").ok).toBe(false); expect(parse("w=640&q=0").ok).toBe(false); From 6c86e352ca1f77011ac4b36699f91d1d4d3c1e4e Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Thu, 13 Aug 2026 02:31:22 +0200 Subject: [PATCH 2/2] docs: trim the fit/position comments to what a reader needs Per review: the new JSDoc argued for the mapping rather than describing it, which belongs in the PR rather than the source. Also normalises a whitespace-only `position` at parse time and fixes the changeset wording. --- .changeset/cf-image-endpoint-fit.md | 2 +- packages/cloudflare/src/image-endpoint.ts | 28 ++++------------------- packages/core/src/media/image-endpoint.ts | 22 +++++------------- 3 files changed, 12 insertions(+), 40 deletions(-) diff --git a/.changeset/cf-image-endpoint-fit.md b/.changeset/cf-image-endpoint-fit.md index 03024bbd50..757647adb2 100644 --- a/.changeset/cf-image-endpoint-fit.md +++ b/.changeset/cf-image-endpoint-fit.md @@ -3,4 +3,4 @@ "emdash": patch --- -Fixes cropping for EmDash media on Cloudflare. Images asked for a `cover` crop — square avatars, fixed-ratio thumbnails — came back scaled down and letterboxed inside the requested box instead of filling it, because the endpoint never passed the requested fit to the Images binding. `fit` and `position` are now honoured, so a crop crops and its focal side is respected. +Fixes cropping for EmDash media on Cloudflare. An image asked to fill a fixed box — a square avatar, a fixed-ratio thumbnail — came back scaled down and letterboxed inside it, because the endpoint never passed the requested fit to the Images binding. `fit` and `position` are now honoured, so a crop crops and its focal side is respected. diff --git a/packages/cloudflare/src/image-endpoint.ts b/packages/cloudflare/src/image-endpoint.ts index 335719786d..1ba39c8aac 100644 --- a/packages/cloudflare/src/image-endpoint.ts +++ b/packages/cloudflare/src/image-endpoint.ts @@ -35,14 +35,9 @@ const FORMAT_MIME: Record = }; /** - * Astro's `fit` vocabulary mapped onto the Images binding's. - * - * Most names line up. Astro's `fill` distorts the image to fill the box, which - * the binding calls `squeeze`. `inside` (accepted by Astro's sharp service) - * resizes to fit within the box, which is the binding's `contain`. `outside` - * has no binding equivalent -- nothing there resizes to *exceed* the box - * without cropping -- so it is dropped and the dimensions apply on their own, - * rather than substituting a fit that would crop. + * Maps Astro `fit` values to the Cloudflare Images binding's fit vocabulary. + * Unmapped values (e.g. `outside`) become `undefined`, leaving the binding's + * default behaviour unchanged. */ const FIT_TO_BINDING: Record = { fill: "squeeze", @@ -54,17 +49,8 @@ const FIT_TO_BINDING: Record([ ["face", "face"], @@ -77,7 +63,6 @@ const GRAVITY_BY_POSITION = new Map([ ["auto", "auto"], ["entropy", "entropy"], ["attention", "auto"], - // sharp's compass vocabulary. ["north", "top"], ["south", "bottom"], ["east", "right"], @@ -142,9 +127,6 @@ export const GET: APIRoute = async (ctx) => { const transform: ImageTransform = {}; if (width) transform.width = width; if (height) transform.height = height; - // Without these the binding falls back to its own default fit, so a - // square `fit=cover` request came back scaled-down and letterboxed - // instead of cropped. if (fit) { const bindingFit = FIT_TO_BINDING[fit]; if (bindingFit) transform.fit = bindingFit; diff --git a/packages/core/src/media/image-endpoint.ts b/packages/core/src/media/image-endpoint.ts index 76ccc178e8..54aa9871d8 100644 --- a/packages/core/src/media/image-endpoint.ts +++ b/packages/core/src/media/image-endpoint.ts @@ -38,14 +38,8 @@ export const MAX_TRANSFORM_DIMENSION = 4000; export type ImageTransformFormat = (typeof ALLOWED_TRANSFORM_FORMATS)[number]; /** - * The `fit` values Astro's image service emits. Astro's `ImageFit` is - * open-ended (`string & {}`), so this is an allowlist: an unrecognised fit is - * dropped rather than forwarded to a backend that would reject it. - * - * `inside` and `outside` are not in `ImageFit` but are accepted by Astro's - * sharp service, so a site can already use them on Node. `none` is absent on - * purpose: Astro deletes it before building the URL, so it never reaches an - * endpoint. + * Fit values Astro's image service may emit. Unknown values are dropped so + * backends keep their default behaviour. */ export const ALLOWED_TRANSFORM_FITS = [ "fill", @@ -200,9 +194,8 @@ export function resolveTransformQuality( * * `fit` and `position` describe how the rendition fills its box. Unlike the * others they are advisory: an unrecognised value is dropped rather than - * failing the request, because Astro's `ImageFit` is open-ended and each - * backend supports a different subset. Mapping them onto a backend's - * vocabulary is the platform endpoint's job. + * failing the request. The platform endpoint maps them onto its backend's + * vocabulary. */ export function parseTransformParams(params: URLSearchParams): ParsedTransformParams { const width = parseDimension(params.get("w")); @@ -231,14 +224,11 @@ export function parseTransformParams(params: URLSearchParams): ParsedTransformPa quality = q; } - // `fit` and `position` come straight from Astro's image service. Both are - // advisory: an unrecognised value is dropped rather than failing the - // request, so a rendition still resolves on a backend that doesn't know it. const fitRaw = params.get("fit"); const fit = fitRaw !== null && isTransformFit(fitRaw) ? fitRaw : undefined; - const positionRaw = params.get("position"); - const position = positionRaw !== null && positionRaw !== "" ? positionRaw : undefined; + const positionRaw = params.get("position")?.trim(); + const position = positionRaw ? positionRaw : undefined; return { ok: true, options: { width, height, format, quality, fit, position } }; }