refactor(common): drop unnecessary srcset sanitization (#47302)

This commit updates runtime and compiler to drop unnecessary `srcset` sanitization. The sanitization was needed previously for old browsers, but all modern browsers can handle `srcset` safely without any additional sanitization.

See prior discussion in https://github.com/angular/angular/pull/45182.

Resolves #45164.

PR Close #47302

packages/compiler/src/schema/dom_security_schema.ts

@@ -35,10 +35,24 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} {
     registerContext(SecurityContext.STYLE, ['*|style']);
     // NB: no SCRIPT contexts here, they are never allowed due to the parser stripping them.
     registerContext(SecurityContext.URL, [
-      '*|formAction', 'area|href',       'area|ping',       'audio|src',    'a|href',
-      'a|ping',       'blockquote|cite', 'body|background', 'del|cite',     'form|action',
-      'img|src',      'img|srcset',      'input|src',       'ins|cite',     'q|cite',
-      'source|src',   'source|srcset',   'track|src',       'video|poster', 'video|src',
+      '*|formAction',
+      'area|href',
+      'area|ping',
+      'audio|src',
+      'a|href',
+      'a|ping',
+      'blockquote|cite',
+      'body|background',
+      'del|cite',
+      'form|action',
+      'img|src',
+      'input|src',
+      'ins|cite',
+      'q|cite',
+      'source|src',
+      'track|src',
+      'video|poster',
+      'video|src',
     ]);
     registerContext(SecurityContext.RESOURCE_URL, [
       'applet|code',

packages/core/src/render3/i18n/i18n_parse.ts

@@ -8,9 +8,9 @@
 import '../../util/ng_dev_mode';
 import '../../util/ng_i18n_closure_mode';
 
-import {getTemplateContent, SRCSET_ATTRS, URI_ATTRS, VALID_ATTRS, VALID_ELEMENTS} from '../../sanitization/html_sanitizer';
+import {getTemplateContent, URI_ATTRS, VALID_ATTRS, VALID_ELEMENTS} from '../../sanitization/html_sanitizer';
 import {getInertBodyHelper} from '../../sanitization/inert_body';
-import {_sanitizeUrl, sanitizeSrcset} from '../../sanitization/url_sanitizer';
+import {_sanitizeUrl} from '../../sanitization/url_sanitizer';
 import {assertDefined, assertEqual, assertGreaterThanOrEqual, assertOneOf, assertString} from '../../util/assert';
 import {CharCode} from '../../util/char_code';
 import {loadIcuContainerVisitor} from '../instructions/i18n_icu_container_visitor';
@@ -624,9 +624,6 @@ function walkIcuTree(
                 if (URI_ATTRS[lowerAttrName]) {
                   generateBindingUpdateOpCodes(
                       update, attr.value, newIndex, attr.name, 0, _sanitizeUrl);
-                } else if (SRCSET_ATTRS[lowerAttrName]) {
-                  generateBindingUpdateOpCodes(
-                      update, attr.value, newIndex, attr.name, 0, sanitizeSrcset);
                 } else {
                   generateBindingUpdateOpCodes(update, attr.value, newIndex, attr.name, 0, null);
                 }

packages/core/src/sanitization/html_sanitizer.ts

@@ -8,8 +8,9 @@
 
 import {TrustedHTML} from '../util/security/trusted_type_defs';
 import {trustedHTMLFromString} from '../util/security/trusted_types';
+
 import {getInertBodyHelper, InertBodyHelper} from './inert_body';
-import {_sanitizeUrl, sanitizeSrcset} from './url_sanitizer';
+import {_sanitizeUrl} from './url_sanitizer';
 
 function tagSet(tags: string): {[k: string]: boolean} {
   const res: {[k: string]: boolean} = {};
@@ -64,14 +65,11 @@ export const VALID_ELEMENTS =
 // Attributes that have href and hence need to be sanitized
 export const URI_ATTRS = tagSet('background,cite,href,itemtype,longdesc,poster,src,xlink:href');
 
-// Attributes that have special href set hence need to be sanitized
-export const SRCSET_ATTRS = tagSet('srcset');
-
 const HTML_ATTRS = tagSet(
     'abbr,accesskey,align,alt,autoplay,axis,bgcolor,border,cellpadding,cellspacing,class,clear,color,cols,colspan,' +
     'compact,controls,coords,datetime,default,dir,download,face,headers,height,hidden,hreflang,hspace,' +
     'ismap,itemscope,itemprop,kind,label,lang,language,loop,media,muted,nohref,nowrap,open,preload,rel,rev,role,rows,rowspan,rules,' +
-    'scope,scrolling,shape,size,sizes,span,srclang,start,summary,tabindex,target,title,translate,type,usemap,' +
+    'scope,scrolling,shape,size,sizes,span,srclang,srcset,start,summary,tabindex,target,title,translate,type,usemap,' +
     'valign,value,vspace,width');
 
 // Accessibility attributes as per WAI-ARIA 1.1 (W3C Working Draft 14 December 2018)
@@ -92,7 +90,7 @@ const ARIA_ATTRS = tagSet(
 // can be sanitized, but they increase security surface area without a legitimate use case, so they
 // are left out here.
 
-export const VALID_ATTRS = merge(URI_ATTRS, SRCSET_ATTRS, HTML_ATTRS, ARIA_ATTRS);
+export const VALID_ATTRS = merge(URI_ATTRS, HTML_ATTRS, ARIA_ATTRS);
 
 // Elements whose content should not be traversed/preserved, if the elements themselves are invalid.
 //
@@ -177,7 +175,6 @@ class SanitizingHtmlSerializer {
       let value = elAttr!.value;
       // TODO(martinprobst): Special case image URIs for data:image/...
       if (URI_ATTRS[lower]) value = _sanitizeUrl(value);
-      if (SRCSET_ATTRS[lower]) value = sanitizeSrcset(value);
       this.buf.push(' ', attrName, '="', encodeEntities(value), '"');
     }
     this.buf.push('>');

packages/core/src/sanitization/url_sanitizer.ts

@@ -45,8 +45,3 @@ export function _sanitizeUrl(url: string): string {
 
   return 'unsafe:' + url;
 }
-
-export function sanitizeSrcset(srcset: string): string {
-  srcset = String(srcset);
-  return srcset.split(',').map((srcset) => _sanitizeUrl(srcset.trim())).join(', ');
-}

packages/core/test/sanitization/html_sanitizer_spec.ts

@@ -69,9 +69,15 @@ function sanitizeHtml(defaultDoc: any, unsafeHtmlInput: string): string {
           .toEqual('<img src="pteranodon.jpg" aria-details="details">');
     });
 
-    it('sanitizes srcset attributes', () => {
+    it('ignores srcset attributes', () => {
+      // Modern browsers can handle `srcset` safely without any additional sanitization.
       expect(sanitizeHtml(defaultDoc, '<img srcset="/foo.png 400px, javascript:evil() 23px">'))
-          .toEqual('<img srcset="/foo.png 400px, unsafe:javascript:evil() 23px">');
+          .toEqual('<img srcset="/foo.png 400px, javascript:evil() 23px">');
+
+      // Verify that complex `srcset` with URLs that contain commas are retained as is.
+      const content = '<img src="https://localhost/h_450,w_450/logo.jpg" ' +
+          'srcset="https://localhost/h_450,w_450/logo.jpg 450w, https://localhost/h_300,w_300/logo.jpg 300w">';
+      expect(sanitizeHtml(defaultDoc, content)).toEqual(content);
     });
 
     it('supports sanitizing plain text', () => {

packages/core/test/sanitization/url_sanitizer_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {_sanitizeUrl, sanitizeSrcset} from '../../src/sanitization/url_sanitizer';
+import {_sanitizeUrl} from '../../src/sanitization/url_sanitizer';
 
 {
   describe('URL sanitizer', () => {
@@ -71,42 +71,5 @@ import {_sanitizeUrl, sanitizeSrcset} from '../../src/sanitization/url_sanitizer
         it(`valid ${url}`, () => expect(_sanitizeUrl(url)).toMatch(/^unsafe:/));
       }
     });
-
-    describe('valid srcsets', () => {
-      const validSrcsets = [
-        '',
-        'http://angular.io/images/test.png',
-        'http://angular.io/images/test.png, http://angular.io/images/test.png',
-        'http://angular.io/images/test.png, http://angular.io/images/test.png, http://angular.io/images/test.png',
-        'http://angular.io/images/test.png 2x',
-        'http://angular.io/images/test.png 2x, http://angular.io/images/test.png 3x',
-        'http://angular.io/images/test.png 1.5x',
-        'http://angular.io/images/test.png 1.25x',
-        'http://angular.io/images/test.png 200w, http://angular.io/images/test.png 300w',
-        'https://angular.io/images/test.png, http://angular.io/images/test.png',
-        'http://angular.io:80/images/test.png, http://angular.io:8080/images/test.png',
-        'http://www.angular.io:80/images/test.png, http://www.angular.io:8080/images/test.png',
-        'https://angular.io/images/test.png, https://angular.io/images/test.png',
-        '//angular.io/images/test.png, //angular.io/images/test.png',
-        '/images/test.png, /images/test.png',
-        'images/test.png, images/test.png',
-        'http://angular.io/images/test.png?12345, http://angular.io/images/test.png?12345',
-        'http://angular.io/images/test.png?maxage, http://angular.io/images/test.png?maxage',
-        'http://angular.io/images/test.png?maxage=234, http://angular.io/images/test.png?maxage=234',
-      ];
-      for (const srcset of validSrcsets) {
-        it(`valid ${srcset}`, () => expect(sanitizeSrcset(srcset)).toEqual(srcset));
-      }
-    });
-
-    describe('invalid srcsets', () => {
-      const invalidSrcsets = [
-        'ht:tp://angular.io/images/test.png',
-        'http://angular.io/images/test.png, ht:tp://angular.io/images/test.png',
-      ];
-      for (const srcset of invalidSrcsets) {
-        it(`valid ${srcset}`, () => expect(sanitizeSrcset(srcset)).toMatch(/unsafe:/));
-      }
-    });
   });
 }