Add warning for icon demensions

packages/snaps-utils/package.json

@@ -81,7 +81,7 @@
     "cron-parser": "^4.5.0",
     "fast-deep-equal": "^3.1.3",
     "fast-json-stable-stringify": "^2.1.0",
-    "is-svg": "^4.4.0",
+    "fast-xml-parser": "^4.3.4",
     "rfdc": "^1.3.0",
     "semver": "^7.5.4",
     "ses": "^1.1.0",

packages/snaps-utils/src/icon.test.ts

@@ -4,6 +4,8 @@ import {
   SVG_MAX_BYTE_SIZE,
   SVG_MAX_BYTE_SIZE_TEXT,
   assertIsSnapIcon,
+  getSvgDimensions,
+  parseSvg,
 } from './icon';
 import { ALTERNATIVE_SNAP_ICON } from './test-utils';
 import { VirtualFile } from './virtual-file';
@@ -47,3 +49,53 @@ describe('assertIsSnapIcon', () => {
     expect(() => assertIsSnapIcon(icon)).not.toThrow();
   });
 });
+
+describe('parseSvg', () => {
+  it('parses valid SVGs', () => {
+    expect(parseSvg(ALTERNATIVE_SNAP_ICON)).toMatchInlineSnapshot(`
+      {
+        "@_fill": "none",
+        "@_height": 25,
+        "@_width": 24,
+        "@_xmlns": "http://www.w3.org/2000/svg",
+        "path": {
+          "@_d": "M17.037 0H6.975C2.605 0 0 2.617 0 6.987v10.05c0 4.37 2.605 6.975 6.975 6.975h10.05c4.37 0 6.975-2.605 6.975-6.976V6.988C24.012 2.617 21.407 0 17.037 0ZM11.49 17.757c0 .36-.18.684-.492.876a.975.975 0 0 1-.54.156 1.11 1.11 0 0 1-.469-.108l-4.202-2.1a1.811 1.811 0 0 1-.985-1.61v-3.973c0-.36.18-.685.493-.877a1.04 1.04 0 0 1 1.008-.048l4.202 2.101a1.8 1.8 0 0 1 .997 1.609v3.974h-.012Zm-.252-6.423L6.723 8.896a1.045 1.045 0 0 1-.528-.924c0-.384.204-.744.528-.924l4.515-2.438a1.631 1.631 0 0 1 1.524 0l4.515 2.438c.324.18.528.528.528.924s-.204.744-.528.924l-4.515 2.438c-.24.132-.504.192-.768.192a1.54 1.54 0 0 1-.756-.192Zm7.972 3.638c0 .684-.385 1.308-.997 1.608l-4.202 2.101c-.144.072-.3.108-.468.108a.975.975 0 0 1-.54-.156 1.017 1.017 0 0 1-.493-.876v-3.974c0-.684.384-1.309.997-1.609l4.202-2.101a1.04 1.04 0 0 1 1.008.048c.313.192.493.516.493.877v3.974Z",
+          "@_fill": "#24272A",
+        },
+      }
+    `);
+  });
+
+  it('throws for invalid SVGs', () => {
+    expect(() => parseSvg('')).toThrow('Snap icon must be a valid SVG.');
+    expect(() => parseSvg('foo')).toThrow('Snap icon must be a valid SVG.');
+  });
+});
+
+describe('getSvgDimensions', () => {
+  it('uses height and width when available', () => {
+    expect(getSvgDimensions(ALTERNATIVE_SNAP_ICON)).toStrictEqual({
+      height: 25,
+      width: 24,
+    });
+  });
+
+  it('uses viewBox as a fallback', () => {
+    expect(
+      getSvgDimensions(
+        '<svg viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"></svg>',
+      ),
+    ).toStrictEqual({
+      height: 24,
+      width: 24,
+    });
+  });
+
+  it('returns null if no dimensions are found', () => {
+    expect(
+      getSvgDimensions(
+        '<svg fill="none" xmlns="http://www.w3.org/2000/svg"></svg>',
+      ),
+    ).toBeNull();
+  });
+});

packages/snaps-utils/src/icon.ts

@@ -1,5 +1,5 @@
 import { assert } from '@metamask/utils';
-import isSvg from 'is-svg';
+import { XMLParser } from 'fast-xml-parser';
 
 import type { VirtualFile } from './virtual-file';
 
@@ -8,7 +8,12 @@ export const SVG_MAX_BYTE_SIZE_TEXT = `${Math.floor(
   SVG_MAX_BYTE_SIZE / 1000,
 )}kb`;
 
-export const assertIsSnapIcon = (icon: VirtualFile) => {
+/**
+ * Asserts that a virtual file containing a Snap icon is valid.
+ *
+ * @param icon - A virtual file containing a Snap icon.
+ */
+export function assertIsSnapIcon(icon: VirtualFile) {
   assert(icon.path.endsWith('.svg'), 'Expected snap icon to end in ".svg".');
 
   assert(
@@ -17,4 +22,82 @@ export const assertIsSnapIcon = (icon: VirtualFile) => {
   );
 
   assert(isSvg(icon.toString()), 'Snap icon must be a valid SVG.');
-};
+}
+
+/**
+ * Extract the dimensions of an image from an SVG string if possible.
+ *
+ * @param svg - An SVG string.
+ * @returns The height and width of the SVG or null.
+ */
+export function getSvgDimensions(svg: string): {
+  height: number;
+  width: number;
+} | null {
+  const parsed = parseSvg(svg);
+
+  const height = parsed['@_height'];
+  const width = parsed['@_width'];
+
+  if (height && width) {
+    return { height, width };
+  }
+
+  const viewBox = parsed['@_viewBox'];
+  if (viewBox) {
+    const split = viewBox.split(' ');
+
+    const viewBoxWidth = split[2];
+    const viewBoxHeight = split[3];
+
+    if (viewBoxWidth && viewBoxHeight) {
+      return {
+        width: parseInt(viewBoxWidth, 10),
+        height: parseInt(viewBoxHeight, 10),
+      };
+    }
+  }
+
+  return null;
+}
+
+/**
+ * Parse and validate a string as an SVG.
+ *
+ * @param svg - An SVG string.
+ * @returns The SVG, its attributes and children in an object format.
+ */
+export function parseSvg(svg: string) {
+  try {
+    const trimmed = svg.trim();
+
+    assert(trimmed.length > 0);
+
+    const parser = new XMLParser({
+      ignoreAttributes: false,
+      parseAttributeValue: true,
+    });
+    const parsed = parser.parse(trimmed, true);
+
+    assert(parsed.svg);
+
+    return parsed.svg;
+  } catch {
+    throw new Error('Snap icon must be a valid SVG.');
+  }
+}
+
+/**
+ * Validate that a string is a valid SVG.
+ *
+ * @param svg - An SVG string.
+ * @returns True if the SVG is valid otherwise false.
+ */
+export function isSvg(svg: string) {
+  try {
+    parseSvg(svg);
+    return true;
+  } catch {
+    return false;
+  }
+}

packages/snaps-utils/src/manifest/manifest.ts

@@ -7,6 +7,7 @@ import pathUtils from 'path';
 
 import { deepClone } from '../deep-clone';
 import { readJsonFile } from '../fs';
+import { getSvgDimensions } from '../icon';
 import { validateNpmSnap } from '../npm';
 import {
   getSnapChecksum,
@@ -197,6 +198,14 @@ export async function checkManifest(
     );
   }
 
+  const iconDimensions =
+    snapFiles.svgIcon && getSvgDimensions(snapFiles.svgIcon.toString());
+  if (iconDimensions && iconDimensions.height !== iconDimensions.width) {
+    warnings.push(
+      'Please use an icon with 1:1 ratio between height and height.',
+    );
+  }
+
   if (writeManifest) {
     try {
       const newManifest = `${JSON.stringify(

yarn.lock

@@ -5794,7 +5794,7 @@ __metadata:
     expect-webdriverio: ^4.4.1
     fast-deep-equal: ^3.1.3
     fast-json-stable-stringify: ^2.1.0
-    is-svg: ^4.4.0
+    fast-xml-parser: ^4.3.4
     istanbul-lib-coverage: ^3.2.0
     istanbul-lib-report: ^3.0.0
     istanbul-reports: ^3.1.5
@@ -13143,14 +13143,14 @@ __metadata:
   languageName: node
   linkType: hard
 
-"fast-xml-parser@npm:^4.1.3":
-  version: 4.2.5
-  resolution: "fast-xml-parser@npm:4.2.5"
+"fast-xml-parser@npm:^4.1.3, fast-xml-parser@npm:^4.3.4":
+  version: 4.3.4
+  resolution: "fast-xml-parser@npm:4.3.4"
   dependencies:
     strnum: ^1.0.5
   bin:
     fxparser: src/cli/cli.js
-  checksum: d32b22005504eeb207249bf40dc82d0994b5bb9ca9dcc731d335a1f425e47fe085b3cace3cf9d32172dd1a5544193c49e8615ca95b4bf95a4a4920a226b06d80
+  checksum: ab88177343f6d3d971d53462db3011003a83eb8a8db704840127ddaaf27105ea90cdf7903a0f9b2e1279ccc4adfca8dfc0277b33bae6262406f10c16bd60ccf9
   languageName: node
   linkType: hard