diff --git a/packages/common/http/src/xhr.ts b/packages/common/http/src/xhr.ts index 4c299d9a06..02ba1cb4d6 100644 --- a/packages/common/http/src/xhr.ts +++ b/packages/common/http/src/xhr.ts @@ -180,24 +180,20 @@ 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 && req.responseType === 'json' && typeof body === 'string') { + if (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); + body = body !== '' ? JSON.parse(body) : null; } catch (error) { - // Even though the response status was 2xx, this is still an error. - ok = false; - // The parse error contains the text of the body that failed to parse. - body = { error, text: body } as HttpJsonParseError; - } - } else if (!ok && req.responseType === 'json' && typeof body === 'string') { - try { - // Attempt to parse the body as JSON. - body = JSON.parse(body); - } catch (error) { - // Cannot be certain that the body was meant to be parsed as JSON. - // Leave the body as a string. + // If this was an error request to begin with, leave it as a string, it probably + // just isn't JSON. Otherwise, deliver the parsing error to the user. + if (ok) { + // Even though the response status was 2xx, this is still an error. + ok = false; + // The parse error contains the text of the body that failed to parse. + body = { error, text: body } as HttpJsonParseError; + } } } diff --git a/packages/common/http/test/xhr_spec.ts b/packages/common/http/test/xhr_spec.ts index 4c76dce6ad..f6e3a9ab5d 100644 --- a/packages/common/http/test/xhr_spec.ts +++ b/packages/common/http/test/xhr_spec.ts @@ -25,6 +25,8 @@ const TEST_POST = new HttpRequest('POST', '/test', 'some body', { responseType: 'text', }); +const XSSI_PREFIX = ')]}\'\n'; + export function main() { describe('XhrBackend', () => { let factory: MockXhrFactory = null !; @@ -92,6 +94,13 @@ export function main() { const res = events[1] as HttpResponse<{data: string}>; expect(res.body !.data).toBe('some data'); }); + it('handles a blank json response', () => { + const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'}))); + factory.mock.mockFlush(200, 'OK', ''); + expect(events.length).toBe(2); + const res = events[1] as HttpResponse<{data: string}>; + expect(res.body).toBeNull(); + }); it('handles a json error response', () => { const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'}))); factory.mock.mockFlush(500, 'Error', JSON.stringify({data: 'some data'})); @@ -99,6 +108,13 @@ export function main() { const res = events[1] as any as HttpErrorResponse; expect(res.error !.data).toBe('some data'); }); + it('handles a json error response with XSSI prefix', () => { + const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'}))); + factory.mock.mockFlush(500, 'Error', XSSI_PREFIX + JSON.stringify({data: 'some data'})); + expect(events.length).toBe(2); + const res = events[1] as any as HttpErrorResponse; + expect(res.error !.data).toBe('some data'); + }); it('handles a json string response', () => { const events = trackEvents(backend.handle(TEST_POST.clone({responseType: 'json'}))); expect(factory.mock.responseType).toEqual('text'); @@ -109,7 +125,7 @@ export function main() { }); 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'})); + factory.mock.mockFlush(200, 'OK', XSSI_PREFIX + 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');