diff --git a/packages/common/http/src/xhr.ts b/packages/common/http/src/xhr.ts index 88ad2e2658..c7123ae792 100644 --- a/packages/common/http/src/xhr.ts +++ b/packages/common/http/src/xhr.ts @@ -107,7 +107,14 @@ export class HttpXhrBackend implements HttpBackend { // Set the responseType if one was requested. if (req.responseType) { - xhr.responseType = req.responseType.toLowerCase() as any; + const responseType = req.responseType.toLowerCase(); + + // JSON responses need to be processed as text. This is because if the server + // returns an XSSI-prefixed JSON response, the browser will fail to parse it, + // xhr.response will be null, and xhr.responseText cannot be accessed to + // retrieve the prefixed JSON data in order to strip the prefix. Thus, all JSON + // is parsed by first requesting text and then applying JSON.parse. + xhr.responseType = ((responseType !== 'json') ? responseType : 'text') as any; } // Serialize the request body if one is present. If not, this will be set to null. @@ -158,12 +165,6 @@ export class HttpXhrBackend implements HttpBackend { if (status !== 204) { // Use XMLHttpRequest.response if set, responseText otherwise. body = (typeof xhr.response === 'undefined') ? xhr.responseText : xhr.response; - - // Strip a common XSSI prefix from string responses. - // TODO: determine if this behavior should be optional and moved to an interceptor. - if (typeof body === 'string') { - body = body.replace(XSSI_PREFIX, ''); - } } // Normalize another potential bug (this one comes from CORS). @@ -179,8 +180,9 @@ export class HttpXhrBackend implements HttpBackend { // Check whether the body needs to be parsed as JSON (in many cases the browser // will have done that already). - if (ok && typeof body === 'string' && req.responseType === 'json') { + if (ok && req.responseType === 'json' && typeof body === 'string') { // Attempt the parse. If it fails, a parse error should be delivered to the user. + body = body.replace(XSSI_PREFIX, ''); try { body = JSON.parse(body); } catch (error) { diff --git a/packages/common/http/test/xhr_mock.ts b/packages/common/http/test/xhr_mock.ts index 743f3526a0..eceb729ebf 100644 --- a/packages/common/http/test/xhr_mock.ts +++ b/packages/common/http/test/xhr_mock.ts @@ -79,8 +79,8 @@ export class MockXMLHttpRequest { return new HttpHeaders(this.mockResponseHeaders).get(header); } - mockFlush(status: number, statusText: string, body: any|null) { - if (this.responseType === 'text') { + mockFlush(status: number, statusText: string, body?: string) { + if (typeof body === 'string') { this.responseText = body; } else { this.response = body; diff --git a/packages/common/http/test/xhr_spec.ts b/packages/common/http/test/xhr_spec.ts index fc9dff6422..6c37619333 100644 --- a/packages/common/http/test/xhr_spec.ts +++ b/packages/common/http/test/xhr_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ddescribe, describe, it} from '@angular/core/testing/src/testing_internal'; +import {ddescribe, describe, iit, it} from '@angular/core/testing/src/testing_internal'; import {Observable} from 'rxjs/Observable'; import {HttpRequest} from '../src/request'; @@ -87,14 +87,22 @@ export function main() { }); it('handles a json response', () => { const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'}))); - factory.mock.mockFlush(200, 'OK', {data: 'some data'}); + factory.mock.mockFlush(200, 'OK', JSON.stringify({data: 'some data'})); expect(events.length).toBe(2); const res = events[1] as HttpResponse<{data: string}>; expect(res.body !.data).toBe('some data'); }); - it('handles a json response that comes via responseText', () => { + it('handles a json string response', () => { const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'}))); - factory.mock.mockFlush(200, 'OK', JSON.stringify({data: 'some data'})); + expect(factory.mock.responseType).toEqual('text'); + factory.mock.mockFlush(200, 'OK', JSON.stringify('this is a string')); + expect(events.length).toBe(2); + const res = events[1] as HttpResponse; + expect(res.body).toEqual('this is a string'); + }); + it('handles a json response with an XSSI prefix', () => { + const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'}))); + factory.mock.mockFlush(200, 'OK', ')]}\'\n' + JSON.stringify({data: 'some data'})); expect(events.length).toBe(2); const res = events[1] as HttpResponse<{data: string}>; expect(res.body !.data).toBe('some data'); @@ -299,7 +307,7 @@ export function main() { expect(error.status).toBe(0); done(); }); - factory.mock.mockFlush(0, 'CORS 0 status', null); + factory.mock.mockFlush(0, 'CORS 0 status'); }); }); });