#7673: Avoid CORS issues in ImageReader

[fregante]

What does this PR do?

• Fixes Image Reader: Failed to execute 'toDataURL' on 'HTMLCanvasElement': Tainted canvases may not be exported. #7673
• Clarifies the expectations of the affected functions
• Reuses existing code
• Distinguishes "image" data (via canvas) from "binary" data (via fetch)

Demo

Repro available on the original issue

Screen.Recording.mov

Future Work

• Maybe further reorganize related functions after #7622: MV3: Add support for SVG theme icons #7925

Checklist

• Add tests
• Designate a primary reviewer: @grahamlangford

[codecov]

Codecov Report

Attention: Patch coverage is 13.63636% with 38 lines in your changes are missing coverage. Please review.

Project coverage is 72.90%. Comparing base (7565b6c) to head (2a156f1).
Report is 3 commits behind head on main.

❗ Current head 2a156f1 differs from pull request most recent head 765565f. Consider uploading reports for the commit 765565f to get more accurate results

Files	Patch %	Lines
src/utils/imageUtils.ts	9.75%	37 Missing ⚠️
src/bricks/readers/ImageExifReader.ts	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7972      +/-   ##
==========================================
- Coverage   72.96%   72.90%   -0.06%     
==========================================
  Files        1295     1293       -2     
  Lines       40248    40253       +5     
  Branches     7490     7489       -1     
==========================================
- Hits        29366    29347      -19     
- Misses      10882    10906      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[fregante]

Original implementation:

pixiebrix-extension/src/bricks/readers/ImageExifReader.ts

Lines 25 to 47 in 117b5a3

	// TODO: I think axios alone can handle all of this, or FileReader can handle both data and blob in a CSP-safe way
	async function getData(img: HTMLImageElement): Promise<ArrayBuffer> {
	// Adapted from https://github.com/exif-js/exif-js/blob/master/exif.js#L384
	if (/^data:/i.test(img.src)) {
	// Data URI
	return convertDataUrl(img.src, "ArrayBuffer");
	}
	if (/^blob:/i.test(img.src)) {
	// Object URL
	const blob = await fetch(img.src).then(async (r) => r.blob());
	return blob.arrayBuffer();
	}
	const response = await axios.get<ArrayBuffer>(img.src, {
	responseType: "arraybuffer",
	});
	if (response.status !== 200) {
	throw new Error(`Error fetching image ${img.src}: ${response.statusText}`);
	}
	return response.data;
	}

Changes:

• accept a string instead of <img> since img is not strictly required (can't read binary from it)
• throw proper error on SVG images
• drop redundant axios-based fetch
• drop TODO

[fregante]

Previous implementation:

pixiebrix-extension/src/bricks/readers/ImageReader.ts

Lines 21 to 41 in 117b5a3

	// Copied from https://stackoverflow.com/questions/934012/get-image-data-url-in-javascript
	// TODO: Replace with `getData` from ImageExifReader?
	function getBase64Image(img: HTMLImageElement) {
	// Create an empty canvas element
	const canvas = document.createElement("canvas");
	canvas.width = img.width;
	canvas.height = img.height;
	// Copy the image contents to the canvas
	// eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion -- 2d always exists
	const context = canvas.getContext("2d")!;
	context.drawImage(img, 0, 0);
	// Get the data-URL formatted image
	// Firefox supports PNG and JPEG. You could check img.src to
	// guess the original format, but be aware the using "image/jpg"
	// will re-encode the image.
	const dataURL = canvas.toDataURL("image/png");
	return dataURL.replace(/^data:image\/(png|jpg);base64,/, "");
	}

Changes:

• clarify expectations of function, add warnings
• if already a base64, skip conversion
• use natural image size instead of the authored <img width height>
• detect CORS issues and re-load image anonymously

[grahamlangford]

If parseDataUrl does fail, it would be good to report the dataURL to DataDog to make it easier to debug. I don't think it's very likely though.

[fregante]

Let's wait until that error shows up in the logs before we spend time on it

[github-actions]

No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack.

Do not edit this comment manually.