Skip to content

Commit

Permalink
prevent svg icons
Browse files Browse the repository at this point in the history
  • Loading branch information
joaomoreno committed Jun 23, 2017
1 parent 7cee285 commit e0dff61
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,10 @@ export function validateManifest(manifest: Manifest): Manifest {

validateEngineCompatibility(manifest.engines['vscode']);

if (/\.svg$/i.test(manifest.icon || '')) {
throw new Error(`SVGs can't be used as icons: ${manifest.icon}`);
}

return manifest;
}

Expand Down
16 changes: 16 additions & 0 deletions src/test/package.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,17 @@ function assertMissingProperty(manifest: XMLManifest, name: string): void {
assert.equal(property.length, 0, `Property '${name}' should not exist`);
}

function createManifest(extra: Partial<Manifest>): Manifest {
return {
name: 'test',
publisher: 'mocha',
version: '0.0.1',
description: 'test extension',
engines: { vscode: '*' },
...extra
};
}

describe('collect', () => {

it('should catch all files', () => {
Expand Down Expand Up @@ -192,6 +203,11 @@ describe('validateManifest', () => {
assert.throws(() => { validateManifest({ publisher: 'demo', name: 'demo', version: '1.0.0', engines: null }); });
assert.throws(() => { validateManifest({ publisher: 'demo', name: 'demo', version: '1.0.0', engines: { vscode: null } }); });
});

it('should prevent SVG icons', () => {
assert(validateManifest(createManifest({ icon: 'icon.png' })));
assert.throws(() => { validateManifest(createManifest({ icon: 'icon.svg' })); });
});
});

describe('toVsixManifest', () => {
Expand Down

2 comments on commit e0dff61

@eamodio
Copy link
Contributor

Choose a reason for hiding this comment

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

@joaomoreno why is this now disallowed?

@adamvoss
Copy link

Choose a reason for hiding this comment

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

@eamodio According to the docs this is for security reasons. Googling suggests the security concern may be JavaScript embedded in the SVG images.

Please sign in to comment.