Skip to content

fix(worker): handle data: URL images in processImage #194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bbornino wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(worker): handle data: URL images in processImage #194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53432c3
fix(worker): handle data: URL images in processImage
bbornino Jul 12, 2026
1da3a9f
Merge branch 'main' into fix/inline-data-url-images
fennifith Jul 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions apps/worker/src/tasks/url-metadata/utils/processImage.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import crypto from "crypto";
import { s3 } from "@playfulprogramming/s3";
import { type Mock } from "vitest";
import { mockEndpoint } from "../../../../test-utils/server.ts";
import { processImages } from "./processImage.ts";

function md5(value: string): string {
return crypto.createHash("md5").update(value).digest("hex");
}

async function readStreamToString(readable: NodeJS.ReadableStream) {
const chunks: Buffer[] = [];
for await (const chunk of readable) {
chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
}
return Buffer.concat(chunks).toString("utf-8");
}

const onePixelPngBase64 =
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR42mMAAQAABQABoIJXOQAAAABJRU5ErkJggg==";

test("decodes an inline percent-encoded SVG data URL without a network request", async () => {
// Matches the shape from issue #28: an SVG data URL whose payload has an
// unescaped `#`. The WHATWG URL parser splits that off into url.hash, so
// url.pathname alone silently loses everything after it - the fix has to
// parse from url.href instead.
const dataUrl = new URL(
"data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 10 10'%3E%3Ccircle fill='#ff0000' cx='5' cy='5' r='5'/%3E%3C/svg%3E",
);

const result = await processImages(
[dataUrl],
24,
"test-bucket",
"remote-icon",
"job-1",
new AbortController().signal,
);

const expectedKey = `remote-icon-${md5(dataUrl.href)}.svg`;
expect(result).toEqual({ key: expectedKey });

expect(s3.upload).toBeCalledTimes(1);
expect(s3.upload).toBeCalledWith(
"test-bucket",
expectedKey,
"job-1",
expect.anything(),
"image/svg+xml",
);

const uploadedStream = (s3.upload as Mock).mock.calls[0][3];
const uploadedSvg = await readStreamToString(uploadedStream);
expect(uploadedSvg).toContain("<svg");

Check failure on line 54 in apps/worker/src/tasks/url-metadata/utils/processImage.test.ts

View workflow job for this annotation

GitHub Actions / unit-tests

src/tasks/url-metadata/utils/processImage.test.ts > decodes an inline percent-encoded SVG data URL without a network request 

AssertionError: expected '' to contain '<svg'

- Expected
+ Received

- <svg

 ❯ src/tasks/url-metadata/utils/processImage.test.ts:54:22
// cx/cy/r sit after the unescaped `#` in the raw data URL - their
// presence proves url.href (not the hash-truncated url.pathname) was
// used to parse the payload
expect(uploadedSvg).toContain('cx="5"');
expect(uploadedSvg).toContain('cy="5"');
expect(uploadedSvg).toContain('r="5"');
});

test("decodes an inline base64-encoded PNG data URL without a network request", async () => {
const dataUrl = new URL(`data:image/png;base64,${onePixelPngBase64}`);

const result = await processImages(
[dataUrl],
896,
"test-bucket",
"remote-banner",
"job-2",
new AbortController().signal,
);

const expectedKey = `remote-banner-${md5(dataUrl.href)}.png`;
expect(result).toEqual({
key: expectedKey,
width: 1,
height: 1,
});

expect(s3.upload).toBeCalledWith(
"test-bucket",
expectedKey,
"job-2",
expect.anything(),
"image/png",
);
});

test("still fetches and processes an http(s) image over the network", async () => {
const url = new URL("https://example.test/banner.png");
mockEndpoint({
path: url,
body: Uint8Array.from(Buffer.from(onePixelPngBase64, "base64")).buffer,
});

const result = await processImages(
[url],
896,
"test-bucket",
"remote-banner",
"job-3",
new AbortController().signal,
);

const expectedKey = `remote-banner-${md5(url.href)}.png`;
expect(result).toEqual({
key: expectedKey,
width: 1,
height: 1,
});

expect(s3.upload).toBeCalledWith(
"test-bucket",
expectedKey,
"job-3",
expect.anything(),
"image/png",
);
});
215 changes: 165 additions & 50 deletions apps/worker/src/tasks/url-metadata/utils/processImage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,57 +47,35 @@ async function compareLastModified(
return false;
}

async function processImage(
async function uploadSvg(
svg: string,
bucket: string,
uploadKey: string,
tag: string | undefined,
): Promise<void> {
const optimizedSvg = svgo.optimize(svg, { multipass: true }).data;
await s3.upload(
bucket,
uploadKey,
tag,
stream.Readable.from([optimizedSvg]),
"image/svg+xml",
);
}

async function uploadRasterImage(
body: NodeJS.ReadableStream,
url: URL,
urlHash: string,
width: number,
bucket: string,
key: string,
tag?: string,
signal?: AbortSignal,
tag: string | undefined,
signal: AbortSignal | undefined,
// undefined for sources with no HTTP resource to compare a last-modified
// header against (e.g. data: URLs) - always upload in that case
request: Dispatcher.ResponseData<null> | undefined,
): Promise<ProcessImageResult | undefined> {
const request = await fetchAsBot({ url, method: "GET", signal }).catch(
(e) => {
console.error(`Error fetching ${url}`, e);
if (e instanceof DOMException && e.name === "TimeoutError") {
throw e;
}
return undefined;
},
);
const body = request?.body;
if (!body) {
console.error(`Request body for ${url} is null`);
return undefined;
}

const urlHash = crypto.createHash("md5").update(url.href).digest("hex");

const isSvg =
request.headers["content-type"]?.includes("image/svg") ||
(!("content-type" in request.headers) &&
path.extname(url.pathname) === ".svg");

if (isSvg) {
const uploadKey = `${key}-${urlHash}.svg`;

if (await compareLastModified(request, bucket, uploadKey, signal)) {
console.log(`Skipping ${uploadKey}, as it has already been stored.`);
await body.dump();
} else {
const svg = await body.text();
const optimizedSvg = svgo.optimize(svg, { multipass: true }).data;
await s3.upload(
bucket,
uploadKey,
tag,
stream.Readable.from([optimizedSvg]),
"image/svg+xml",
);
}

return { key: uploadKey };
}

const pipeline = sharp();
const metadataStream = body.pipe(pipeline);
const metadata = await Promise.race([
Expand All @@ -107,7 +85,6 @@ async function processImage(

if (!metadata || !metadata.format) {
console.error(`Image format for ${url} could not be found.`);
await body.dump();
return undefined;
}

Expand All @@ -119,7 +96,11 @@ async function processImage(
? Math.round(metadata.height * (transformWidth / metadata.width))
: undefined;

if (await compareLastModified(request, bucket, uploadKey, signal)) {
const alreadyStored =
request !== undefined &&
(await compareLastModified(request, bucket, uploadKey, signal));

if (alreadyStored) {
console.log(`Skipping ${uploadKey}, as it has already been stored.`);
metadataStream.destroy();
} else {
Expand All @@ -135,15 +116,149 @@ async function processImage(
);
}

await body.dump();

return {
key: uploadKey,
width: transformWidth,
height: transformHeight,
};
}

interface ParsedDataUrl {
mediaType: string;
isBase64: boolean;
payload: string;
}

// The WHATWG URL parser treats `data:` as an opaque-path scheme, so an
// unescaped `#` in the payload gets split off into url.hash and url.pathname
// alone silently loses everything after it. url.href keeps the full string
// intact, so parse the data:<mediatype>[;base64],<payload> shape from there.
function parseDataUrl(url: URL): ParsedDataUrl | undefined {
const href = url.href;
const commaIndex = href.indexOf(",");
if (!href.startsWith("data:") || commaIndex === -1) {
return undefined;
}

const meta = href.slice("data:".length, commaIndex);
const payload = href.slice(commaIndex + 1);
const isBase64 = /;base64$/i.test(meta);
const mediaType =
(isBase64 ? meta.slice(0, -";base64".length) : meta) ||
"text/plain;charset=US-ASCII";

return { mediaType, isBase64, payload };
}

async function processDataUrlImage(
url: URL,
width: number,
bucket: string,
key: string,
tag: string | undefined,
signal: AbortSignal | undefined,
): Promise<ProcessImageResult | undefined> {
const parsed = parseDataUrl(url);
if (!parsed) {
console.error(`Unable to parse data URL ${url}`);
return undefined;
}

const urlHash = crypto.createHash("md5").update(url.href).digest("hex");
const isSvg = parsed.mediaType.includes("image/svg");

if (isSvg) {
const svg = parsed.isBase64
? Buffer.from(parsed.payload, "base64").toString("utf-8")
: decodeURIComponent(parsed.payload);

const uploadKey = `${key}-${urlHash}.svg`;
await uploadSvg(svg, bucket, uploadKey, tag);
return { key: uploadKey };
}

const buffer = parsed.isBase64
? Buffer.from(parsed.payload, "base64")
: Buffer.from(decodeURIComponent(parsed.payload), "utf-8");

const body = stream.Readable.from(buffer);

return uploadRasterImage(
body,
url,
urlHash,
width,
bucket,
key,
tag,
signal,
undefined,
);
}

async function processImage(

@fennifith fennifith Jul 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good overall, though I have an organizational nit - processImage and processDataImage have almost the same execution flow (computing a URL hash, branching if the image is an svg, otherwise passing into uploadRasterImage). That makes this a bit confusing to follow.

I find that early returns are great for asserting simple preconditions, but the url.protocol === "data:" / isSvg branches are more involved.

It would be nice to flatten this logic and have one readImage(url, signal) that returns a consistent { body: ReadableStream, format: 'svg' | 'png' | 'jpeg' | 'webp' | 'gif' | ..., lastModified?: Date }

readImage can then parse data URLs, detect SVGs, or otherwise fetch an HTTP response & process sharp().metadata() to construct that result.

processImage then uses that result and has one consistent upload implementation - creating urlHash, determining the uploadKey, checking lastModified if available, then passing the stream to uploadSvg or uploadRasterImage accordingly.

That should eliminate a lot of the duplication here.

url: URL,
width: number,
bucket: string,
key: string,
tag?: string,
signal?: AbortSignal,
): Promise<ProcessImageResult | undefined> {
if (url.protocol === "data:") {
return processDataUrlImage(url, width, bucket, key, tag, signal);
}

const request = await fetchAsBot({ url, method: "GET", signal }).catch(
(e) => {
console.error(`Error fetching ${url}`, e);
if (e instanceof DOMException && e.name === "TimeoutError") {
throw e;
}
return undefined;
},
);
const body = request?.body;
if (!body) {
console.error(`Request body for ${url} is null`);
return undefined;
}

const urlHash = crypto.createHash("md5").update(url.href).digest("hex");

const isSvg =
request.headers["content-type"]?.includes("image/svg") ||
(!("content-type" in request.headers) &&
path.extname(url.pathname) === ".svg");

if (isSvg) {
const uploadKey = `${key}-${urlHash}.svg`;

if (await compareLastModified(request, bucket, uploadKey, signal)) {
console.log(`Skipping ${uploadKey}, as it has already been stored.`);
await body.dump();
} else {
const svg = await body.text();
await uploadSvg(svg, bucket, uploadKey, tag);
}

return { key: uploadKey };
}

const result = await uploadRasterImage(
body,
url,
urlHash,
width,
bucket,
key,
tag,
signal,
request,
);
await body.dump();
return result;
}

export async function processImages(
urls: URL[],
width: number,
Expand Down
Loading