fix(common): escape query selector used when anchor scrolling (#29577)

jasonaden · jasonaden · commit 7671c736555f · 2019-03-29T10:39:40.000-07:00
When an anchor scroll happens, we run document.querySelector. This value can be taken directly from the user. Therefore it's possible to throw an error on scrolling, which can cause the application to fail. This PR escapes the selector before using it. Related to #28193 [Internal discussion](https://groups.google.com/a/google.com/forum/#!topic/angular-users/d82GHfmRKLc) PR Close #29577
diff --git a/packages/common/src/viewport_scroller.ts b/packages/common/src/viewport_scroller.ts
@@ -6,10 +6,12 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {defineInjectable, inject} from '@angular/core';
+import {ErrorHandler, defineInjectable, inject} from '@angular/core';
 
 import {DOCUMENT} from './dom_tokens';
 
+
+
 /**
  * Defines a scroll position manager. Implemented by `BrowserViewportScroller`.
  *
@@ -19,8 +21,10 @@ export abstract class ViewportScroller {
   // De-sugared tree-shakable injection
   // See #23917
   /** @nocollapse */
-  static ngInjectableDef = defineInjectable(
-      {providedIn: 'root', factory: () => new BrowserViewportScroller(inject(DOCUMENT), window)});
+  static ngInjectableDef = defineInjectable({
+    providedIn: 'root',
+    factory: () => new BrowserViewportScroller(inject(DOCUMENT), window, inject(ErrorHandler))
+  });
 
   /**
    * Configures the top offset used when scrolling to an anchor.
@@ -62,7 +66,7 @@ export abstract class ViewportScroller {
 export class BrowserViewportScroller implements ViewportScroller {
   private offset: () => [number, number] = () => [0, 0];
 
-  constructor(private document: any, private window: any) {}
+  constructor(private document: any, private window: any, private errorHandler: ErrorHandler) {}
 
   /**
    * Configures the top offset used when scrolling to an anchor.
@@ -106,15 +110,26 @@ export class BrowserViewportScroller implements ViewportScroller {
    */
   scrollToAnchor(anchor: string): void {
     if (this.supportScrollRestoration()) {
-      const elSelectedById = this.document.querySelector(`#${anchor}`);
-      if (elSelectedById) {
-        this.scrollToElement(elSelectedById);
-        return;
+      // Escape anything passed to `querySelector` as it can throw errors and stop the application
+      // from working if invalid values are passed.
+      if (this.window.CSS && this.window.CSS.escape) {
+        anchor = this.window.CSS.escape(anchor);
+      } else {
+        anchor = anchor.replace(/(\"|\'\ |:|\.|\[|\]|,|=)/g, '\\$1');
       }
-      const elSelectedByName = this.document.querySelector(`[name='${anchor}']`);
-      if (elSelectedByName) {
-        this.scrollToElement(elSelectedByName);
-        return;
+      try {
+        const elSelectedById = this.document.querySelector(`#${anchor}`);
+        if (elSelectedById) {
+          this.scrollToElement(elSelectedById);
+          return;
+        }
+        const elSelectedByName = this.document.querySelector(`[name='${anchor}']`);
+        if (elSelectedByName) {
+          this.scrollToElement(elSelectedByName);
+          return;
+        }
+      } catch (e) {
+        this.errorHandler.handleError(e);
       }
     }
   }
diff --git a/packages/common/test/viewport_scroller_spec.ts b/packages/common/test/viewport_scroller_spec.ts
@@ -0,0 +1,40 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+
+
+/**
+* @license
+* Copyright Google Inc. All Rights Reserved.
+*
+* Use of this source code is governed by an MIT-style license that can be
+* found in the LICENSE file at https://angular.io/license
+*/
+
+import {describe, expect, it} from '@angular/core/testing/src/testing_internal';
+import {BrowserViewportScroller, ViewportScroller} from '../src/viewport_scroller';
+
+{
+  describe('BrowserViewportScroller', () => {
+    let scroller: ViewportScroller;
+    let documentSpy: any;
+    beforeEach(() => {
+      documentSpy = jasmine.createSpyObj('document', ['querySelector']);
+      scroller = new BrowserViewportScroller(documentSpy, {scrollTo: 1}, null !);
+    });
+    it('escapes invalid characters selectors', () => {
+      const invalidSelectorChars = `"' :.[],=`;
+      // Double escaped to make sure we match the actual value passed to `querySelector`
+      const escapedInvalids = `\\"\\' \\:\\.\\[\\]\\,\\=`;
+      scroller.scrollToAnchor(`specials=${invalidSelectorChars}`);
+      expect(documentSpy.querySelector).toHaveBeenCalledWith(`#specials\\=${escapedInvalids}`);
+      expect(documentSpy.querySelector)
+          .toHaveBeenCalledWith(`[name='specials\\=${escapedInvalids}']`);
+    });
+  });
+}
