[pull] master from angular:master

aio/content/examples/schematics-for-libraries/projects/my-lib/schematics/my-service/index.ts

@@ -6,37 +6,53 @@ import {
   chain, mergeWith
 } from '@angular-devkit/schematics';
 
-import { strings, normalize, experimental } from '@angular-devkit/core';
+import { strings, normalize, virtualFs, workspaces } from '@angular-devkit/core';
 // #enddocregion schematics-imports
 
 import { Schema as MyServiceSchema } from './schema';
 // #enddocregion schema-imports
 
-export function myService(options: MyServiceSchema): Rule {
-  return (tree: Tree) => {
-    const workspaceConfig = tree.read('/angular.json');
-    if (!workspaceConfig) {
-      throw new SchematicsException('Could not find Angular workspace configuration');
-    }
+function createHost(tree: Tree): workspaces.WorkspaceHost {
+  return {
+    async readFile(path: string): Promise<string> {
+      const data = tree.read(path);
+      if (!data) {
+        throw new SchematicsException('File not found.');
+      }
+      return virtualFs.fileBufferToString(data);
+    },
+    async writeFile(path: string, data: string): Promise<void> {
+      return tree.overwrite(path, data);
+    },
+    async isDirectory(path: string): Promise<boolean> {
+      return !tree.exists(path) && tree.getDir(path).subfiles.length > 0;
+    },
+    async isFile(path: string): Promise<boolean> {
+      return tree.exists(path);
+    },
+  };
+}
 
-    // convert workspace to string
-    const workspaceContent = workspaceConfig.toString();
+export function myService(options: MyServiceSchema): Rule {
+  return async (tree: Tree) => {
+    const host = createHost(tree);
+    const { workspace } = await workspaces.readWorkspace('/', host);
 
-    // parse workspace string into JSON object
-    const workspace: experimental.workspace.WorkspaceSchema = JSON.parse(workspaceContent);
 // #enddocregion workspace
+
+// #docregion project-info
 // #docregion project-fallback
     if (!options.project) {
-      options.project = workspace.defaultProject;
+      options.project = workspace.extensions.defaultProject;
     }
 // #enddocregion project-fallback
 
-// #docregion project-info
-    const projectName = options.project as string;
-
-    const project = workspace.projects[projectName];
+    const project = workspace.projects.get(options.project);
+    if (!project) {
+      throw new SchematicsException(`Invalid project name: ${options.project}`);
+    }
 
-    const projectType = project.projectType === 'application' ? 'app' : 'lib';
+    const projectType = project.extensions.projectType === 'application' ? 'app' : 'lib';
 // #enddocregion project-info
 
 // #docregion path

aio/content/examples/testing/src/app/banner/banner-initial.component.spec.ts

@@ -1,14 +1,14 @@
 // #docplaster
-// #docregion import-by
-// #enddocregion import-by
 // #docregion import-debug-element
 import { DebugElement } from '@angular/core';
 // #enddocregion import-debug-element
 // #docregion v1
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
-import { By } from '@angular/platform-browser';
 
 // #enddocregion v1
+// #docregion import-by
+import { By } from '@angular/platform-browser';
+// #enddocregion import-by
 import { BannerComponent } from './banner-initial.component';
 
 /*

aio/content/guide/schematics-for-libraries.md

@@ -218,17 +218,16 @@ The `Tree` methods give you access to the complete file tree in your workspace,
 
 ### Get the project configuration
 
-1. To determine the destination project, use the `Tree.read()` method to read the contents of the workspace configuration file, `angular.json`, at the root of the workspace.
+1. To determine the destination project, use the `workspaces.readWorkspace` method to read the contents of the workspace configuration file, `angular.json`.
+   To use `workspaces.readWorkspace` you need to create a `workspaces.WorkspaceHost` from the `Tree`.
    Add the following code to your factory function.
 
 <code-example header="projects/my-lib/schematics/my-service/index.ts (Schema Import)" path="schematics-for-libraries/projects/my-lib/schematics/my-service/index.ts" region="workspace">
 </code-example>
 
   * Be sure to check that the context exists and throw the appropriate error.
 
-  * After reading the contents into a string, parse the configuration into a JSON object, typed to the `WorkspaceSchema`.
-
-1. The `WorkspaceSchema` contains all the properties of the workspace configuration, including a `defaultProject` value for determining which project to use if not provided.
+1. The `WorkspaceDefinition`, `extensions` property includes a `defaultProject` value for determining which project to use if not provided.
    We will use that value as a fallback, if no project is explicitly specified in the `ng generate` command.
 
 <code-example header="projects/my-lib/schematics/my-service/index.ts (Default Project)" path="schematics-for-libraries/projects/my-lib/schematics/my-service/index.ts" region="project-fallback">

packages/core/src/util/dom.ts

@@ -6,15 +6,27 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-const END_COMMENT = /-->/g;
-const END_COMMENT_ESCAPED = '-\u200B-\u200B>';
+/**
+ * Disallowed strings in the comment.
+ *
+ * see: https://html.spec.whatwg.org/multipage/syntax.html#comments
+ */
+const COMMENT_DISALLOWED = /^>|^->|<!--|-->|--!>|<!-$/g;
+/**
+ * Delimiter in the disallowed strings which needs to be wrapped with zero with character.
+ */
+const COMMENT_DELIMITER = /(<|>)/;
+const COMMENT_DELIMITER_ESCAPED = '\u200B$1\u200B';
 
 /**
- * Escape the content of the strings so that it can be safely inserted into a comment node.
+ * Escape the content of comment strings so that it can be safely inserted into a comment node.
  *
  * The issue is that HTML does not specify any way to escape comment end text inside the comment.
- * `<!-- The way you close a comment is with "-->". -->`. Above the `"-->"` is meant to be text not
- * an end to the comment. This can be created programmatically through DOM APIs.
+ * Consider: `<!-- The way you close a comment is with ">", and "->" at the beginning or by "-->" or
+ * "--!>" at the end. -->`. Above the `"-->"` is meant to be text not an end to the comment. This
+ * can be created programmatically through DOM APIs. (`<!--` are also disallowed.)
+ *
+ * see: https://html.spec.whatwg.org/multipage/syntax.html#comments
  *
  * ```
  * div.innerHTML = div.innerHTML
@@ -25,13 +37,15 @@ const END_COMMENT_ESCAPED = '-\u200B-\u200B>';
  * opening up the application for XSS attack. (In SSR we programmatically create comment nodes which
  * may contain such text and expect them to be safe.)
  *
- * This function escapes the comment text by looking for the closing char sequence `-->` and replace
- * it with `-_-_>` where the `_` is a zero width space `\u200B`. The result is that if a comment
- * contains `-->` text it will render normally but it will not cause the HTML parser to close the
- * comment.
+ * This function escapes the comment text by looking for comment delimiters (`<` and `>`) and
+ * surrounding them with `_>_` where the `_` is a zero width space `\u200B`. The result is that if a
+ * comment contains any of the comment start/end delimiters (such as `<!--`, `-->` or `--!>`) the
+ * text it will render normally but it will not cause the HTML parser to close/open the comment.
  *
- * @param value text to make safe for comment node by escaping the comment close character sequence
+ * @param value text to make safe for comment node by escaping the comment open/close character
+ *     sequence.
  */
 export function escapeCommentText(value: string): string {
-  return value.replace(END_COMMENT, END_COMMENT_ESCAPED);
+  return value.replace(
+      COMMENT_DISALLOWED, (text) => text.replace(COMMENT_DELIMITER, COMMENT_DELIMITER_ESCAPED));
 }

packages/core/test/acceptance/security_spec.ts

@@ -11,24 +11,33 @@ import {TestBed} from '@angular/core/testing';
 
 
 describe('comment node text escaping', () => {
-  it('should not be possible to do XSS through comment reflect data', () => {
-    @Component({template: `<div><span *ngIf="xssValue"></span><div>`})
-    class XSSComp {
-      xssValue: string = '--> --><script>"evil"</script>';
-    }
+  // see: https://html.spec.whatwg.org/multipage/syntax.html#comments
+  ['>',         // self closing
+   '-->',       // standard closing
+   '--!>',      // alternate closing
+   '<!-- -->',  // embedded comment.
+  ].forEach((xssValue) => {
+    it('should not be possible to do XSS through comment reflect data when writing: ' + xssValue,
+       () => {
+         @Component({template: `<div><span *ngIf="xssValue"></span><div>`})
+         class XSSComp {
+           // ngIf serializes the `xssValue` into a comment for debugging purposes.
+           xssValue: string = xssValue + '<script>"evil"</script>';
+         }
 
-    TestBed.configureTestingModule({declarations: [XSSComp]});
-    const fixture = TestBed.createComponent(XSSComp);
-    fixture.detectChanges();
-    const div = fixture.nativeElement.querySelector('div') as HTMLElement;
-    // Serialize into a string to mimic SSR serialization.
-    const html = div.innerHTML;
-    // This must be escaped or we have XSS.
-    expect(html).not.toContain('--><script');
-    // Now parse it back into DOM (from string)
-    div.innerHTML = html;
-    // Verify that we did not accidentally deserialize the `<script>`
-    const script = div.querySelector('script');
-    expect(script).toBeFalsy();
+         TestBed.configureTestingModule({declarations: [XSSComp]});
+         const fixture = TestBed.createComponent(XSSComp);
+         fixture.detectChanges();
+         const div = fixture.nativeElement.querySelector('div') as HTMLElement;
+         // Serialize into a string to mimic SSR serialization.
+         const html = div.innerHTML;
+         // This must be escaped or we have XSS.
+         expect(html).not.toContain('--><script');
+         // Now parse it back into DOM (from string)
+         div.innerHTML = html;
+         // Verify that we did not accidentally deserialize the `<script>`
+         const script = div.querySelector('script');
+         expect(script).toBeFalsy();
+       });
   });
 });

packages/core/test/util/dom_spec.ts

@@ -14,13 +14,35 @@ describe('comment node text escaping', () => {
       expect(escapeCommentText('text')).toEqual('text');
     });
 
+    it('should escape "<" or ">"', () => {
+      expect(escapeCommentText('<!--')).toEqual('\u200b<\u200b!--');
+      expect(escapeCommentText('<!--<!--')).toEqual('\u200b<\u200b!--\u200b<\u200b!--');
+      expect(escapeCommentText('>')).toEqual('\u200b>\u200b');
+      expect(escapeCommentText('>-->')).toEqual('\u200b>\u200b--\u200b>\u200b');
+    });
+
     it('should escape end marker', () => {
-      expect(escapeCommentText('before-->after')).toEqual('before-\u200b-\u200b>after');
+      expect(escapeCommentText('before-->after')).toEqual('before--\u200b>\u200bafter');
     });
 
     it('should escape multiple markers', () => {
       expect(escapeCommentText('before-->inline-->after'))
-          .toEqual('before-\u200b-\u200b>inline-\u200b-\u200b>after');
+          .toEqual('before--\u200b>\u200binline--\u200b>\u200bafter');
+    });
+
+    it('should caver the spec', () => {
+      // https://html.spec.whatwg.org/multipage/syntax.html#comments
+      expect(escapeCommentText('>')).toEqual('\u200b>\u200b');
+      expect(escapeCommentText('->')).toEqual('-\u200b>\u200b');
+      expect(escapeCommentText('<!--')).toEqual('\u200b<\u200b!--');
+      expect(escapeCommentText('-->')).toEqual('--\u200b>\u200b');
+      expect(escapeCommentText('--!>')).toEqual('--!\u200b>\u200b');
+      expect(escapeCommentText('<!-')).toEqual('\u200b<\u200b!-');
+
+      // Things which are OK
+      expect(escapeCommentText('.>')).toEqual('.>');
+      expect(escapeCommentText('.->')).toEqual('.->');
+      expect(escapeCommentText('<!-.')).toEqual('<!-.');
     });
   });
 });