angular · martinsik · Apr 24, 2020 · Nov 21, 2021 · martinsik · Apr 25, 2020
diff --git a/packages/common/http/src/jsonp.ts b/packages/common/http/src/jsonp.ts
@@ -21,6 +21,12 @@ import {HttpErrorResponse, HttpEvent, HttpEventType, HttpResponse, HttpStatusCod
 // is shared among all applications on the page.
 let nextRequestId: number = 0;
 
+/**
+ * When a pending <script> is unsubscribed we'll move it to this document, so it won't be
+ * executed.
+ */
+let foreignDocument: Document|undefined;
+
 // Error text given when a JSONP script is injected, but doesn't invoke the callback
 // passed in its URL.
 export const JSONP_ERR_NO_CALLBACK = 'JSONP injected script did not invoke callback.';
@@ -101,22 +107,13 @@ export class JsonpClientBackend implements HttpBackend {
       // Whether the response callback has been called.
       let finished: boolean = false;
 
-      // Whether the request has been cancelled (and thus any other callbacks)
-      // should be ignored.
-      let cancelled: boolean = false;
-
       // Set the response callback in this.callbackMap (which will be the window
       // object in the browser. The script being loaded via the <script> tag will
       // eventually call this callback.
       this.callbackMap[callback] = (data?: any) => {
         // Data has been received from the JSONP script. Firstly, delete this callback.
         delete this.callbackMap[callback];
 
-        // Next, make sure the request wasn't cancelled in the meantime.
-        if (cancelled) {
-          return;
-        }
-
         // Set state to indicate data was received.
         body = data;
         finished = true;
@@ -141,11 +138,6 @@ export class JsonpClientBackend implements HttpBackend {
       // If something went wrong, onLoad() may run without the response callback
       // having been invoked.
       const onLoad = (event: Event) => {
-        // Do nothing if the request has been cancelled.
-        if (cancelled) {
-          return;
-        }
-
         // We wrap it in an extra Promise, to ensure the microtask
         // is scheduled after the loaded endpoint has executed any potential microtask itself,
         // which is not guaranteed in Internet Explorer and EdgeHTML. See issue #39496
@@ -184,10 +176,6 @@ export class JsonpClientBackend implements HttpBackend {
       // a Javascript error. It emits the error via the Observable error channel as
       // a HttpErrorResponse.
       const onError: any = (error: Error) => {
-        // If the request was already cancelled, no need to emit anything.
-        if (cancelled) {
-          return;
-        }
         cleanup();
 
         // Wrap the error in a HttpErrorResponse.
@@ -210,18 +198,25 @@ export class JsonpClientBackend implements HttpBackend {
 
       // Cancellation handler.
       return () => {
-        // Track the cancellation so event listeners won't do anything even if already scheduled.
-        cancelled = true;
-
-        // Remove the event listeners so they won't run if the events later fire.
-        node.removeEventListener('load', onLoad);
-        node.removeEventListener('error', onError);
+        if (!finished) {
+          this.removeListeners(node);
+        }
 
         // And finally, clean up the page.
         cleanup();
       };
     });
   }
+
+  private removeListeners(script: HTMLScriptElement): void {
+    // Issue #34818
+    // Changing <script>'s ownerDocument will prevent it from execution.
+    // https://html.spec.whatwg.org/multipage/scripting.html#execute-the-script-block
+    if (!foreignDocument) {
+      foreignDocument = (this.document.implementation as DOMImplementation).createHTMLDocument();
+    }
+    foreignDocument.adoptNode(script);
+  }
 }
 
 /**

diff --git a/packages/common/http/test/jsonp_mock.ts b/packages/common/http/test/jsonp_mock.ts
@@ -7,7 +7,7 @@
  */
 
 export class MockScriptElement {
-  constructor() {}
+  constructor(public ownerDocument: MockDocument) {}
 
   listeners: {
     load?: (event: Event) => void,
@@ -28,8 +28,12 @@ export class MockDocument {
   mock!: MockScriptElement|null;
   readonly body: any = this;
 
+  implementation = {
+    createHTMLDocument: () => new MockDocument(),
+  };
+
   createElement(tag: 'script'): HTMLScriptElement {
-    return new MockScriptElement() as any as HTMLScriptElement;
+    return new MockScriptElement(this) as any as HTMLScriptElement;
   }
 
   appendChild(node: any): void {
@@ -42,11 +46,23 @@ export class MockDocument {
     }
   }
 
+  adoptNode(node: any) {
+    node.ownerDocument = this;
+  }
+
   mockLoad(): void {
-    this.mock!.listeners.load!(null as any);
+    // Mimic behavior described by
+    // https://html.spec.whatwg.org/multipage/scripting.html#execute-the-script-block
+    if (this.mock!.ownerDocument === this) {
+      this.mock!.listeners.load!(null as any);
+    }
   }
 
   mockError(err: Error) {
-    this.mock!.listeners.error!(err);
+    // Mimic behavior described by
+    // https://html.spec.whatwg.org/multipage/scripting.html#execute-the-script-block
+    if (this.mock!.ownerDocument === this) {
+      this.mock!.listeners.error!(err);
+    }
   }
 }
diff --git a/packages/common/http/test/jsonp_spec.ts b/packages/common/http/test/jsonp_spec.ts
@@ -17,15 +17,14 @@ function runOnlyCallback(home: any, data: Object) {
   const keys = Object.keys(home);
   expect(keys.length).toBe(1);
   const callback = home[keys[0]];
-  delete home[keys[0]];
 delete this.callbackMap[callback]; 
 delete this.callbackMap[callback]; 
   callback(data);
 }
 
 const SAMPLE_REQ = new HttpRequest<never>('JSONP', '/test');
 
 {
   describe('JsonpClientBackend', () => {
-    let home = {};
+    let home: any;
     let document: MockDocument;
     let backend: JsonpClientBackend;
     beforeEach(() => {
@@ -63,6 +62,20 @@ const SAMPLE_REQ = new HttpRequest<never>('JSONP', '/test');
       });
       document.mockError(error);
     });
+    it('prevents the script from executing when the request is cancelled', () => {
+      const sub = backend.handle(SAMPLE_REQ).subscribe();
+      expect(Object.keys(home).length).toBe(1);
+      const keys = Object.keys(home);
+      const spy = jasmine.createSpy('spy', home[keys[0]]);
+
+      sub.unsubscribe();
+      document.mockLoad();
+      expect(Object.keys(home).length).toBe(0);
+      expect(spy).not.toHaveBeenCalled();
+      // The script element should have been transferred to a different document to prevent it from
+      // executing.
+      expect(document.mock!.ownerDocument).not.toEqual(document);
+    });
     describe('throws an error', () => {
       it('when request method is not JSONP',
          () => expect(() => backend.handle(SAMPLE_REQ.clone<never>({method: 'GET'})))