Skip to content

Commit

Permalink
fix(icon): remove svgSrc, only allow trusted urls (#1933)
Browse files Browse the repository at this point in the history
* fix(icon): remove svgSrc, only allow trusted urls

* rxjs
  • Loading branch information
jelbourn authored Dec 21, 2016
1 parent 221c234 commit 4571561
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 146 deletions.
7 changes: 1 addition & 6 deletions src/demo-app/icon/icon-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@
These are some icons.
</p>

<p>
From URL:
<md-icon svgSrc="/icon/assets/search-icon.svg"></md-icon>
</p>

<p>
By name registered with MdIconProvider:
<md-icon svgIcon="thumb-up" class="green"></md-icon>
Expand Down Expand Up @@ -37,4 +32,4 @@
Custom icon font and CSS:
<md-icon fontSet="fontawesome" fontIcon="fa-birthday-cake"></md-icon>
</p>
</div>
</div>
9 changes: 6 additions & 3 deletions src/demo-app/icon/icon-demo.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Component, ViewEncapsulation} from '@angular/core';
import {DomSanitizer} from '@angular/platform-browser';
import {MdIconRegistry} from '@angular/material';

@Component({
Expand All @@ -10,10 +11,12 @@ import {MdIconRegistry} from '@angular/material';
encapsulation: ViewEncapsulation.None,
})
export class IconDemo {
constructor(mdIconRegistry: MdIconRegistry) {
constructor(mdIconRegistry: MdIconRegistry, sanitizer: DomSanitizer) {
mdIconRegistry
.addSvgIcon('thumb-up', '/icon/assets/thumbup-icon.svg')
.addSvgIconSetInNamespace('core', '/icon/assets/core-icon-set.svg')
.addSvgIcon('thumb-up',
sanitizer.bypassSecurityTrustResourceUrl('/icon/assets/thumbup-icon.svg'))
.addSvgIconSetInNamespace('core',
sanitizer.bypassSecurityTrustResourceUrl('/icon/assets/core-icon-set.svg'))
.registerFontClassAlias('fontawesome', 'fa');
}
}
35 changes: 22 additions & 13 deletions src/lib/icon/icon-registry.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import {Injectable} from '@angular/core';
import {Injectable, SecurityContext} from '@angular/core';
import {SafeResourceUrl, DomSanitizer} from '@angular/platform-browser';
import {Http} from '@angular/http';
import {MdError} from '../core';
import {Observable} from 'rxjs/Observable';
import 'rxjs/add/observable/forkJoin';
import 'rxjs/add/observable/of';
import 'rxjs/add/observable/throw';
import 'rxjs/add/operator/map';
import 'rxjs/add/operator/filter';
import 'rxjs/add/operator/do';
Expand All @@ -18,7 +20,7 @@ import 'rxjs/add/operator/catch';
*/
export class MdIconNameNotFoundError extends MdError {
constructor(iconName: string) {
super(`Unable to find icon with the name "${iconName}"`);
super(`Unable to find icon with the name "${iconName}"`);
}
}

Expand All @@ -29,7 +31,7 @@ export class MdIconNameNotFoundError extends MdError {
*/
export class MdIconSvgTagNotFoundError extends MdError {
constructor() {
super('<svg> tag not found');
super('<svg> tag not found');
}
}

Expand All @@ -39,7 +41,7 @@ export class MdIconSvgTagNotFoundError extends MdError {
*/
class SvgIconConfig {
svgElement: SVGElement = null;
constructor(public url: string) { }
constructor(public url: SafeResourceUrl) { }
}

/** Returns the cache key to use for an icon namespace and name. */
Expand Down Expand Up @@ -81,27 +83,27 @@ export class MdIconRegistry {
*/
private _defaultFontSetClass = 'material-icons';

constructor(private _http: Http) {}
constructor(private _http: Http, private _sanitizer: DomSanitizer) {}

/** Registers an icon by URL in the default namespace. */
addSvgIcon(iconName: string, url: string): this {
addSvgIcon(iconName: string, url: SafeResourceUrl): this {
return this.addSvgIconInNamespace('', iconName, url);
}

/** Registers an icon by URL in the specified namespace. */
addSvgIconInNamespace(namespace: string, iconName: string, url: string): this {
addSvgIconInNamespace(namespace: string, iconName: string, url: SafeResourceUrl): this {
const key = iconKey(namespace, iconName);
this._svgIconConfigs.set(key, new SvgIconConfig(url));
return this;
}

/** Registers an icon set by URL in the default namespace. */
addSvgIconSet(url: string): this {
addSvgIconSet(url: SafeResourceUrl): this {
return this.addSvgIconSetInNamespace('', url);
}

/** Registers an icon set by URL in the specified namespace. */
addSvgIconSetInNamespace(namespace: string, url: string): this {
addSvgIconSetInNamespace(namespace: string, url: SafeResourceUrl): this {
const config = new SvgIconConfig(url);
if (this._iconSetConfigs.has(namespace)) {
this._iconSetConfigs.get(namespace).push(config);
Expand Down Expand Up @@ -152,7 +154,9 @@ export class MdIconRegistry {
* the produced element will always be a new copy of the originally fetched icon. (That is,
* it will not contain any modifications made to elements previously returned).
*/
getSvgIconFromUrl(url: string): Observable<SVGElement> {
getSvgIconFromUrl(safeUrl: SafeResourceUrl): Observable<SVGElement> {
let url = this._sanitizer.sanitize(SecurityContext.RESOURCE_URL, safeUrl);

if (this._cachedIconsByUrl.has(url)) {
return Observable.of(cloneSvg(this._cachedIconsByUrl.get(url)));
}
Expand Down Expand Up @@ -221,9 +225,12 @@ export class MdIconRegistry {
.map(iconSetConfig =>
this._loadSvgIconSetFromConfig(iconSetConfig)
.catch((err: any, caught: Observable<SVGElement>): Observable<SVGElement> => {
let url =
this._sanitizer.sanitize(SecurityContext.RESOURCE_URL, iconSetConfig.url);

// Swallow errors fetching individual URLs so the combined Observable won't
// necessarily fail.
console.log(`Loading icon set URL: ${iconSetConfig.url} failed: ${err}`);
console.log(`Loading icon set URL: ${url} failed: ${err}`);
return Observable.of(null);
})
.do(svg => {
Expand Down Expand Up @@ -280,7 +287,7 @@ export class MdIconRegistry {
private _loadSvgIconSetFromConfig(config: SvgIconConfig): Observable<SVGElement> {
// TODO: Document that icons should only be loaded from trusted sources.
return this._fetchUrl(config.url)
.map((svgText) => this._svgElementFromString(svgText));
.map(svgText => this._svgElementFromString(svgText));
}

/**
Expand Down Expand Up @@ -353,7 +360,9 @@ export class MdIconRegistry {
* Returns an Observable which produces the string contents of the given URL. Results may be
* cached, so future calls with the same URL may not cause another HTTP request.
*/
private _fetchUrl(url: string): Observable<string> {
private _fetchUrl(safeUrl: SafeResourceUrl): Observable<string> {
let url = this._sanitizer.sanitize(SecurityContext.RESOURCE_URL, safeUrl);

// Store in-progress fetches to avoid sending a duplicate request for a URL when there is
// already a request in progress for that URL. It's necessary to call share() on the
// Observable returned by http.get() so that multiple subscribers don't cause multiple XHRs.
Expand Down
Loading

0 comments on commit 4571561

Please sign in to comment.