Skip to content

Commit

Permalink
fix(http): queue jsonp <script> tag onLoad event handler in microtask (
Browse files Browse the repository at this point in the history
…#39512)

Before this change, when trying to load a JSONP script that calls the JSONP callback inside a
microtask, it will fail in Internet Explorer 11 and EdgeHTML. This commit changes the onLoad cleanup
to be queued after the loaded endpoint executed any potential microtask itself. This ensures that
the aforementioned browsers will first evaluate the loaded script calling the JSONP callback and
only then run the cleanup inside onLoad.

Fixes #39496

PR Close #39512
  • Loading branch information
sebastianhaeni authored and atscott committed Nov 17, 2020
1 parent a88bc64 commit 10e4ac0
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 24 deletions.
58 changes: 34 additions & 24 deletions packages/common/http/src/jsonp.ts
Expand Up @@ -50,6 +50,11 @@ export abstract class JsonpCallbackContext {
*/
@Injectable()
export class JsonpClientBackend implements HttpBackend {
/**
* A resolved promise that can be used to schedule microtasks in the event handlers.
*/
private readonly resolvedPromise = Promise.resolve();

constructor(private callbackMap: JsonpCallbackContext, @Inject(DOCUMENT) private document: any) {}

/**
Expand Down Expand Up @@ -140,33 +145,38 @@ export class JsonpClientBackend implements HttpBackend {
return;
}

// Cleanup the page.
cleanup();

// Check whether the response callback has run.
if (!finished) {
// It hasn't, something went wrong with the request. Return an error via
// the Observable error path. All JSONP errors have status 0.
observer.error(new HttpErrorResponse({
// 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
this.resolvedPromise.then(() => {
// Cleanup the page.
cleanup();

// Check whether the response callback has run.
if (!finished) {
// It hasn't, something went wrong with the request. Return an error via
// the Observable error path. All JSONP errors have status 0.
observer.error(new HttpErrorResponse({
url,
status: 0,
statusText: 'JSONP Error',
error: new Error(JSONP_ERR_NO_CALLBACK),
}));
return;
}

// Success. body either contains the response body or null if none was
// returned.
observer.next(new HttpResponse({
body,
status: 200,
statusText: 'OK',
url,
status: 0,
statusText: 'JSONP Error',
error: new Error(JSONP_ERR_NO_CALLBACK),
}));
return;
}

// Success. body either contains the response body or null if none was
// returned.
observer.next(new HttpResponse({
body,
status: 200,
statusText: 'OK',
url,
}));

// Complete the stream, the response is over.
observer.complete();
// Complete the stream, the response is over.
observer.complete();
});
};

// onError() is the error callback, which runs if the script returned generates
Expand Down
10 changes: 10 additions & 0 deletions packages/common/http/test/jsonp_spec.ts
Expand Up @@ -45,6 +45,16 @@ const SAMPLE_REQ = new HttpRequest<never>('JSONP', '/test');
runOnlyCallback(home, {data: 'This is a test'});
document.mockLoad();
});
// Issue #39496
it('handles a request with callback call wrapped in promise', done => {
backend.handle(SAMPLE_REQ).subscribe(() => {
done();
});
Promise.resolve().then(() => {
runOnlyCallback(home, {data: 'This is a test'});
});
document.mockLoad();
});
it('handles an error response properly', done => {
const error = new Error('This is a test error');
backend.handle(SAMPLE_REQ).pipe(toArray()).subscribe(undefined, (err: HttpErrorResponse) => {
Expand Down

0 comments on commit 10e4ac0

Please sign in to comment.