fix(common): treat an empty body as null when parsing JSON in HttpClient (#19958)

Previously, XhrBackend would call JSON.parse('') if the response body was
empty (a 200 status code with content-length 0). This changes the XhrBackend
to attempt the JSON parse only if the response body is non-empty. Otherwise,
the body is left as null.

Fixes #18680.
Fixes #19413.
Fixes #19502.
Fixes #19555.

PR Close #19958

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) {
+            // 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;
             }
-        } 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.
           }
         }
 

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');