From c0f750af4ea7ab6f662eef7dc3ba73d972265d0f Mon Sep 17 00:00:00 2001 From: Dzmitry Shylovich Date: Wed, 21 Dec 2016 04:51:02 +0300 Subject: [PATCH] fix(compiler): ignore @import in comments (#13368) * refactor(compiler): clean up style url resolver * fix(compiler): ignore @import in css comments Closes #12196 --- .../compiler/src/style_url_resolver.ts | 31 ++++++++++--------- .../compiler/test/style_url_resolver_spec.ts | 11 +++++++ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/modules/@angular/compiler/src/style_url_resolver.ts b/modules/@angular/compiler/src/style_url_resolver.ts index e105ccee5b..1f2bbb5dd6 100644 --- a/modules/@angular/compiler/src/style_url_resolver.ts +++ b/modules/@angular/compiler/src/style_url_resolver.ts @@ -9,8 +9,6 @@ // Some of the code comes from WebComponents.JS // https://github.com/webcomponents/webcomponentsjs/blob/master/src/HTMLImports/path.js -import {isBlank, isPresent} from './facade/lang'; - import {UrlResolver} from './url_resolver'; export class StyleWithImports { @@ -18,8 +16,8 @@ export class StyleWithImports { } export function isStyleUrlResolvable(url: string): boolean { - if (isBlank(url) || url.length === 0 || url[0] == '/') return false; - const schemeMatch = url.match(_urlWithSchemaRe); + if (url == null || url.length === 0 || url[0] == '/') return false; + const schemeMatch = url.match(URL_WITH_SCHEMA_REGEXP); return schemeMatch === null || schemeMatch[1] == 'package' || schemeMatch[1] == 'asset'; } @@ -30,17 +28,20 @@ export function isStyleUrlResolvable(url: string): boolean { export function extractStyleUrls( resolver: UrlResolver, baseUrl: string, cssText: string): StyleWithImports { const foundUrls: string[] = []; - const modifiedCssText = cssText.replace(_cssImportRe, function(...m: string[]) { - const url = m[1] || m[2]; - if (!isStyleUrlResolvable(url)) { - // Do not attempt to resolve non-package absolute URLs with URI scheme - return m[0]; - } - foundUrls.push(resolver.resolve(baseUrl, url)); - return ''; - }); + + const modifiedCssText = + cssText.replace(CSS_COMMENT_REGEXP, '').replace(CSS_IMPORT_REGEXP, (...m: string[]) => { + const url = m[1] || m[2]; + if (!isStyleUrlResolvable(url)) { + // Do not attempt to resolve non-package absolute URLs with URI scheme + return m[0]; + } + foundUrls.push(resolver.resolve(baseUrl, url)); + return ''; + }); return new StyleWithImports(modifiedCssText, foundUrls); } -const _cssImportRe = /@import\s+(?:url\()?\s*(?:(?:['"]([^'"]*))|([^;\)\s]*))[^;]*;?/g; -const _urlWithSchemaRe = /^([^:/?#]+):/; +const CSS_IMPORT_REGEXP = /@import\s+(?:url\()?\s*(?:(?:['"]([^'"]*))|([^;\)\s]*))[^;]*;?/g; +const CSS_COMMENT_REGEXP = /\/\*.+?\*\//g; +const URL_WITH_SCHEMA_REGEXP = /^([^:/?#]+):/; diff --git a/modules/@angular/compiler/test/style_url_resolver_spec.ts b/modules/@angular/compiler/test/style_url_resolver_spec.ts index 700a8631ae..819ad7d0a6 100644 --- a/modules/@angular/compiler/test/style_url_resolver_spec.ts +++ b/modules/@angular/compiler/test/style_url_resolver_spec.ts @@ -36,6 +36,17 @@ export function main() { expect(styleWithImports.styleUrls).toEqual(['http://ng.io/1.css', 'http://ng.io/2.css']); }); + it('should ignore "@import" in comments', () => { + const css = ` + @import '1.css'; + /*@import '2.css';*/ + `; + const styleWithImports = extractStyleUrls(urlResolver, 'http://ng.io', css); + expect(styleWithImports.style.trim()).toEqual(''); + expect(styleWithImports.styleUrls).toContain('http://ng.io/1.css'); + expect(styleWithImports.styleUrls).not.toContain('http://ng.io/2.css'); + }); + it('should extract "@import url()" urls', () => { const css = ` @import url('3.css');