cherry-pick(1.19): always return non-empty body regardless of request…

… method (#12102) (#12121)
microsoft · Feb 15, 2022 · d8bc6db · d8bc6db
1 parent 03501cf
commit d8bc6db
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 10 deletions.
diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts
@@ -18,7 +18,7 @@ import * as http from 'http';
 import * as https from 'https';
 import { HttpsProxyAgent } from 'https-proxy-agent';
 import { SocksProxyAgent } from 'socks-proxy-agent';
-import { pipeline, Readable, Transform } from 'stream';
+import { pipeline, Readable, Transform, TransformCallback } from 'stream';
 import url from 'url';
 import zlib from 'zlib';
 import { HTTPCredentials } from '../../types/types';
@@ -341,13 +341,6 @@ export abstract class APIRequestContext extends SdkObject {
           });
         };
 
-        // These requests don't have response body.
-        if (['HEAD', 'PUT', 'TRACE'].includes(options.method!)) {
-          notifyBodyFinished();
-          request.destroy();
-          return;
-        }
-
         let body: Readable = response;
         let transform: Transform | undefined;
         const encoding = response.headers['content-encoding'];
@@ -362,7 +355,9 @@ export abstract class APIRequestContext extends SdkObject {
           transform = zlib.createInflate();
         }
         if (transform) {
-          body = pipeline(response, transform, e => {
+          // Brotli and deflate decompressors throw if the input stream is empty.
+          const emptyStreamTransform = new SafeEmptyStreamTransform(notifyBodyFinished);
+          body = pipeline(response, emptyStreamTransform, transform, e => {
             if (e)
               reject(new Error(`failed to decompress '${encoding}' encoding: ${e}`));
           });
@@ -407,6 +402,26 @@ export abstract class APIRequestContext extends SdkObject {
   }
 }
 
+class SafeEmptyStreamTransform extends Transform {
+  private _receivedSomeData: boolean = false;
+  private _onEmptyStreamCallback: () => void;
+
+  constructor(onEmptyStreamCallback: () => void) {
+    super();
+    this._onEmptyStreamCallback = onEmptyStreamCallback;
+  }
+  override _transform(chunk: any, encoding: BufferEncoding, callback: TransformCallback): void {
+    this._receivedSomeData = true;
+    callback(null, chunk);
+  }
+  override _flush(callback: TransformCallback): void {
+    if (this._receivedSomeData)
+      callback(null);
+    else
+      this._onEmptyStreamCallback();
+  }
+}
+
 export class BrowserContextAPIRequestContext extends APIRequestContext {
   private readonly _context: BrowserContext;
 

diff --git a/tests/global-fetch.spec.ts b/tests/global-fetch.spec.ts
@@ -50,7 +50,7 @@ for (const method of ['fetch', 'delete', 'get', 'head', 'patch', 'post', 'put']
     expect(response.ok()).toBeTruthy();
     expect(response.headers()['content-type']).toBe('application/json; charset=utf-8');
     expect(response.headersArray()).toContainEqual({ name: 'Content-Type', value: 'application/json; charset=utf-8' });
-    expect(await response.text()).toBe(['head', 'put'].includes(method) ? '' : '{"foo": "bar"}\n');
+    expect(await response.text()).toBe('head' === method ? '' : '{"foo": "bar"}\n');
   });
 }
 
@@ -363,3 +363,18 @@ it('should not fail on empty body with encoding', async ({ playwright, server })
   }
   await request.dispose();
 });
+
+it('should return body for failing requests', async ({ playwright, server }) => {
+  const request = await playwright.request.newContext();
+  for (const method of ['head', 'put', 'trace']) {
+    server.setRoute('/empty.html', (req, res) => {
+      res.writeHead(404, { 'Content-Length': 10, 'Content-Type': 'text/plain' });
+      res.end('Not found.');
+    });
+    const response = await request.fetch(server.EMPTY_PAGE, { method });
+    expect(response.status()).toBe(404);
+    // HEAD response returns empty body in node http module.
+    expect(await response.text()).toBe(method === 'head' ? '' : 'Not found.');
+  }
+  await request.dispose();
+});