fix(network): make allHeaders wait until all headers are available (#15094)

fix(network): make allHeaders wait until all header are available

Before, calling `allHeaders()` from `page.on('request')` would yield
provisional headers instead.

With these changes:
- In Firefox, all headers are available immediately.
- In Chromium, all headers are available upon requestWillBeSentExtraInfo.
- In WebKit, all headers are available upon responseReceived.
- In all browsers, intercepted requests use "provisional" headers
  as all headers, since there is no network stack to change the headers.

Drive-by: migrated Chromium to `hasExtraInfo` flags that simplifies
the logic quite a bit.

packages/playwright-core/src/server/chromium/crNetworkManager.ts

@@ -54,7 +54,6 @@ export class CRNetworkManager {
       eventsHelper.addEventListener(session, 'Fetch.authRequired', this._onAuthRequired.bind(this)),
       eventsHelper.addEventListener(session, 'Network.requestWillBeSent', this._onRequestWillBeSent.bind(this, workerFrame)),
       eventsHelper.addEventListener(session, 'Network.requestWillBeSentExtraInfo', this._onRequestWillBeSentExtraInfo.bind(this)),
-      eventsHelper.addEventListener(session, 'Network.requestServedFromCache', this._onRequestServedFromCache.bind(this)),
       eventsHelper.addEventListener(session, 'Network.responseReceived', this._onResponseReceived.bind(this)),
       eventsHelper.addEventListener(session, 'Network.responseReceivedExtraInfo', this._onResponseReceivedExtraInfo.bind(this)),
       eventsHelper.addEventListener(session, 'Network.loadingFinished', this._onLoadingFinished.bind(this)),
@@ -119,8 +118,6 @@ export class CRNetworkManager {
   }
 
   _onRequestWillBeSent(workerFrame: frames.Frame | undefined, event: Protocol.Network.requestWillBeSentPayload) {
-    this._responseExtraInfoTracker.requestWillBeSent(event);
-
     // Request interception doesn't happen for data URLs with Network Service.
     if (this._protocolRequestInterceptionEnabled && !event.request.url.startsWith('data:')) {
       const requestId = event.requestId;
@@ -136,10 +133,6 @@ export class CRNetworkManager {
     }
   }
 
-  _onRequestServedFromCache(event: Protocol.Network.requestServedFromCachePayload) {
-    this._responseExtraInfoTracker.requestServedFromCache(event);
-  }
-
   _onRequestWillBeSentExtraInfo(event: Protocol.Network.requestWillBeSentExtraInfoPayload) {
     this._responseExtraInfoTracker.requestWillBeSentExtraInfo(event);
   }
@@ -160,13 +153,6 @@ export class CRNetworkManager {
   }
 
   _onRequestPaused(workerFrame: frames.Frame | undefined, event: Protocol.Fetch.requestPausedPayload) {
-    if (!event.responseStatusCode && !event.responseErrorReason) {
-      // Request intercepted, deliver signal to the tracker.
-      const request = this._requestIdToRequest.get(event.networkId!);
-      if (request)
-        this._responseExtraInfoTracker.requestPaused(request.request, event);
-    }
-
     if (!event.networkId) {
       // Fetch without networkId means that request was not recongnized by inspector, and
       // it will never receive Network.requestWillBeSent. Most likely, this is an internal request
@@ -198,7 +184,7 @@ export class CRNetworkManager {
       const request = this._requestIdToRequest.get(requestWillBeSentEvent.requestId);
       // If we connect late to the target, we could have missed the requestWillBeSent event.
       if (request) {
-        this._handleRequestRedirect(request, requestWillBeSentEvent.redirectResponse, requestWillBeSentEvent.timestamp);
+        this._handleRequestRedirect(request, requestWillBeSentEvent.redirectResponse, requestWillBeSentEvent.timestamp, requestWillBeSentEvent.redirectHasExtraInfo);
         redirectedFrom = request;
       }
     }
@@ -266,10 +252,16 @@ export class CRNetworkManager {
       redirectedFrom
     });
     this._requestIdToRequest.set(requestWillBeSentEvent.requestId, request);
+    if (requestPausedEvent && !requestPausedEvent.responseStatusCode && !requestPausedEvent.responseErrorReason) {
+      // We will not receive extra info when intercepting the request.
+      // Use the headers from the Fetch.requestPausedPayload and release the allHeaders()
+      // right away, so that client can call it from the route handler.
+      request.request.setRawRequestHeaders(headersObjectToArray(requestPausedEvent.request.headers, '\n'));
+    }
     this._page._frameManager.requestStarted(request.request, route || undefined);
   }
 
-  _createResponse(request: InterceptableRequest, responsePayload: Protocol.Network.Response): network.Response {
+  _createResponse(request: InterceptableRequest, responsePayload: Protocol.Network.Response, hasExtraInfo: boolean): network.Response {
     const getResponseBody = async () => {
       const contentLengthHeader = Object.entries(responsePayload.headers).find(header => header[0].toLowerCase() === 'content-length');
       const expectedLength = contentLengthHeader ? +contentLengthHeader[1] : undefined;
@@ -332,12 +324,18 @@ export class CRNetworkManager {
       validFrom: responsePayload?.securityDetails?.validFrom,
       validTo: responsePayload?.securityDetails?.validTo,
     });
-    this._responseExtraInfoTracker.processResponse(request._requestId, response, request.wasFulfilled());
+    if (hasExtraInfo) {
+      this._responseExtraInfoTracker.processResponse(request._requestId, response);
+    } else {
+      // Use "provisional" headers as "raw" ones.
+      response.request().setRawRequestHeaders(null);
+      response.setRawResponseHeaders(null);
+    }
     return response;
   }
 
-  _handleRequestRedirect(request: InterceptableRequest, responsePayload: Protocol.Network.Response, timestamp: number) {
+  _handleRequestRedirect(request: InterceptableRequest, responsePayload: Protocol.Network.Response, timestamp: number, hasExtraInfo: boolean) {
-    const response = this._createResponse(request, responsePayload);
+    const response = this._createResponse(request, responsePayload, hasExtraInfo);
     response._requestFinished((timestamp - request._timestamp) * 1000);
     this._requestIdToRequest.delete(request._requestId);
     if (request._interceptionId)
@@ -351,13 +349,11 @@ export class CRNetworkManager {
   }
 
   _onResponseReceived(event: Protocol.Network.responseReceivedPayload) {
-    this._responseExtraInfoTracker.responseReceived(event);
-
     const request = this._requestIdToRequest.get(event.requestId);
     // FileUpload sends a response without a matching request.
     if (!request)
       return;
-    const response = this._createResponse(request, event.response);
+    const response = this._createResponse(request, event.response, event.hasExtraInfo);
     this._page._frameManager.requestReceivedResponse(response);
   }
 
@@ -484,16 +480,11 @@ class InterceptableRequest {
       request = request._redirectedFrom;
     return request._route;
   }
-
-  wasFulfilled() {
-    return this._routeForRedirectChain()?._wasFulfilled || false;
-  }
 }
 
 class RouteImpl implements network.RouteDelegate {
   private readonly _client: CRSession;
   private _interceptionId: string;
-  _wasFulfilled = false;
 
   constructor(client: CRSession, interceptionId: string) {
     this._client = client;
@@ -513,7 +504,6 @@ class RouteImpl implements network.RouteDelegate {
   }
 
   async fulfill(response: types.NormalizedFulfillResponse) {
-    this._wasFulfilled = true;
     const body = response.isBase64 ? response.body : Buffer.from(response.body).toString('base64');
 
     const responseHeaders = splitSetCookieHeader(response.headers);
@@ -573,70 +563,39 @@ const errorReasons: { [reason: string]: Protocol.Network.ErrorReason } = {
 
 type RequestInfo = {
   requestId: string,
-  requestWillBeSentExtraInfo: Protocol.Network.requestWillBeSentExtraInfoPayload[],
+  // Events are replaced with "undefined" to avoid updating the same headers twice.
-  responseReceivedExtraInfo: Protocol.Network.responseReceivedExtraInfoPayload[],
+  requestWillBeSentExtraInfo: (Protocol.Network.requestWillBeSentExtraInfoPayload | undefined)[],
+  responseReceivedExtraInfo: (Protocol.Network.responseReceivedExtraInfoPayload | undefined)[],
+  // Note: we only put the responses that expect extra info in this list.
+  // Since the order of responses and extraInfo events is the same, each response
+  // will get a pair of matching request/response extraInfo events in this list.
   responses: network.Response[],
   loadingFinished?: Protocol.Network.loadingFinishedPayload,
   loadingFailed?: Protocol.Network.loadingFailedPayload,
-  sawResponseWithoutConnectionId: boolean
-  requestServedFromCache: boolean;
 };
 
 // This class aligns responses with response headers from extra info:
 //   - Network.requestWillBeSent, Network.responseReceived, Network.loadingFinished/loadingFailed are
 //     dispatched using one channel.
-//   - Network.requestWillBeSentExtraInfo and Network.responseReceivedExtraInfo are dispatches on
+//   - Network.requestWillBeSentExtraInfo and Network.responseReceivedExtraInfo are dispatched on
 //     another channel. Those channels are not associated, so events come in random order.
 //
 // This class will associate responses with the new headers. These extra info headers will become
 // available to client reliably upon requestfinished event only. It consumes CDP
 // signals on one end and processResponse(network.Response) signals on the other hands. It then makes
-// sure that responses have all the extra headers in place by the time request finises.
+// sure that responses have all the extra headers in place by the time request finishes.
 //
 // The shape of the instrumentation API is deliberately following the CDP, so that it
-// what clear what is called when and what this means to the tracker without extra
+// is clear what is called when and what this means to the tracker without extra
 // documentation.
 class ResponseExtraInfoTracker {
   private _requests = new Map<string, RequestInfo>();
 
-  requestWillBeSent(event: Protocol.Network.requestWillBeSentPayload) {
-    const info = this._requests.get(event.requestId);
-    if (info && event.redirectResponse)
-      this._innerResponseReceived(info, event.redirectResponse);
-    else
-      this._getOrCreateEntry(event.requestId);
-  }
-
   requestWillBeSentExtraInfo(event: Protocol.Network.requestWillBeSentExtraInfoPayload) {
     const info = this._getOrCreateEntry(event.requestId);
     info.requestWillBeSentExtraInfo.push(event);
     this._patchHeaders(info, info.requestWillBeSentExtraInfo.length - 1);
-  }
+    this._checkFinished(info);
-
-  requestServedFromCache(event: Protocol.Network.requestServedFromCachePayload) {
-    const info = this._getOrCreateEntry(event.requestId);
-    info.requestServedFromCache = true;
-  }
-
-  responseReceived(event: Protocol.Network.responseReceivedPayload) {
-    const info = this._requests.get(event.requestId);
-    if (!info)
-      return;
-    this._innerResponseReceived(info, event.response);
-  }
-
-  requestPaused(request: network.Request, event: Protocol.Fetch.requestPausedPayload) {
-    // requestWillBeSentExtraInfo is not being called when interception
-    // is enabled. But interception is mutually exclusive with the redirects.
-    // So we can use the headers from the Fetch.requestPausedPayload immediately.
-    request.setRawRequestHeaders(headersObjectToArray(event.request.headers, '\n'));
-  }
-
-  private _innerResponseReceived(info: RequestInfo, response: Protocol.Network.Response) {
-    if (!response.connectionId) {
-      // Starting with this response we no longer can guarantee that response and extra info correspond to the same index.
-      info.sawResponseWithoutConnectionId = true;
-    }
   }
 
   responseReceivedExtraInfo(event: Protocol.Network.responseReceivedExtraInfoPayload) {
@@ -646,19 +605,8 @@ class ResponseExtraInfoTracker {
     this._checkFinished(info);
   }
 
-  processResponse(requestId: string, response: network.Response, wasFulfilled: boolean) {
+  processResponse(requestId: string, response: network.Response) {
-    // We are not interested in ExtraInfo tracking for fulfilled requests, our Blink
+    const info = this._getOrCreateEntry(requestId);
-    // headers are the ones that contain fulfilled headers.
-    if (wasFulfilled) {
-      this._stopTracking(requestId);
-      return;
-    }
-
-    const info = this._requests.get(requestId);
-    if (!info || info.sawResponseWithoutConnectionId)
-      return;
-    if (!info.requestServedFromCache)
-      response.setWillReceiveExtraHeaders();
     info.responses.push(response);
     this._patchHeaders(info, info.responses.length - 1);
   }
@@ -687,8 +635,6 @@ class ResponseExtraInfoTracker {
         requestWillBeSentExtraInfo: [],
         responseReceivedExtraInfo: [],
         responses: [],
-        sawResponseWithoutConnectionId: false,
-        requestServedFromCache: false
       };
       this._requests.set(requestId, info);
     }
@@ -698,12 +644,15 @@ class ResponseExtraInfoTracker {
   private _patchHeaders(info: RequestInfo, index: number) {
     const response = info.responses[index];
     const requestExtraInfo = info.requestWillBeSentExtraInfo[index];
-    if (response && requestExtraInfo)
+    if (response && requestExtraInfo) {
       response.request().setRawRequestHeaders(headersObjectToArray(requestExtraInfo.headers, '\n'));
+      info.requestWillBeSentExtraInfo[index] = undefined;
+    }
     const responseExtraInfo = info.responseReceivedExtraInfo[index];
     if (response && responseExtraInfo) {
       response.setRawResponseHeaders(headersObjectToArray(responseExtraInfo.headers, '\n'));
       response.request().responseSize.responseHeadersSize = responseExtraInfo.headersText?.length || 0;
+      info.responseReceivedExtraInfo[index] = undefined;
     }
   }
 
@@ -713,7 +662,6 @@ class ResponseExtraInfoTracker {
 
     if (info.responses.length <= info.responseReceivedExtraInfo.length) {
       // We have extra info for each response.
-      // We could have more extra infos because we stopped collecting responses at some point.
       this._stopTracking(info.requestId);
       return;
     }

packages/playwright-core/src/server/firefox/ffNetworkManager.ts

@@ -113,6 +113,8 @@ export class FFNetworkManager {
       validFrom: event?.securityDetails?.validFrom,
       validTo: event?.securityDetails?.validTo,
     });
+    // "raw" headers are the same as "provisional" headers in Firefox.
+    response.setRawResponseHeaders(null);
     this._page._frameManager.requestReceivedResponse(response);
   }
 
@@ -194,6 +196,8 @@ class InterceptableRequest {
       postDataBuffer = Buffer.from(payload.postData, 'base64');
     this.request = new network.Request(frame, redirectedFrom ? redirectedFrom.request : null, payload.navigationId,
         payload.url, internalCauseToResourceType[payload.internalCause] || causeToResourceType[payload.cause] || 'other', payload.method, postDataBuffer, payload.headers);
+    // "raw" headers are the same as "provisional" headers in Firefox.
+    this.request.setRawRequestHeaders(null);
   }
 
   _finalRequest(): InterceptableRequest {

packages/playwright-core/src/server/network.ts

@@ -103,7 +103,7 @@ export class Request extends SdkObject {
   private _postData: Buffer | null;
   readonly _headers: types.HeadersArray;
   private _headersMap = new Map<string, string>();
-  private _rawRequestHeadersPromise: ManualPromise<types.HeadersArray> | undefined;
+  private _rawRequestHeadersPromise = new ManualPromise<types.HeadersArray>();
   private _frame: frames.Frame;
   private _waitForResponsePromise = new ManualPromise<Response | null>();
   _responseEndTiming = -1;
@@ -131,6 +131,8 @@ export class Request extends SdkObject {
   _setFailureText(failureText: string) {
     this._failureText = failureText;
     this._waitForResponsePromise.resolve(null);
+    // If we didn't get raw headers, declare them equal to provisional.
+    this.setRawRequestHeaders(null);
   }
 
   url(): string {
@@ -157,22 +159,13 @@ export class Request extends SdkObject {
     return this._headersMap.get(name);
   }
 
-  setWillReceiveExtraHeaders() {
+  // "null" means no raw headers available - we'll use provisional headers as raw headers.
-    if (!this._rawRequestHeadersPromise)
+  setRawRequestHeaders(headers: types.HeadersArray | null) {
-      this._rawRequestHeadersPromise = new ManualPromise();
+    if (!this._rawRequestHeadersPromise.isDone())
-  }
+      this._rawRequestHeadersPromise.resolve(headers || this._headers);
-
-  setRawRequestHeaders(headers: types.HeadersArray) {
-    if (!this._rawRequestHeadersPromise)
-      this._rawRequestHeadersPromise = new ManualPromise();
-    this._rawRequestHeadersPromise!.resolve(headers);
   }
 
   async rawRequestHeaders(): Promise<NameValue[]> {
-    return this._rawRequestHeadersPromise || Promise.resolve(this._headers);
-  }
-
-  rawRequestHeadersPromise(): Promise<types.HeadersArray> | undefined {
     return this._rawRequestHeadersPromise;
   }
 
@@ -222,7 +215,7 @@ export class Request extends SdkObject {
     headersSize += this.method().length;
     headersSize += (new URL(this.url())).pathname.length;
     headersSize += 8; // httpVersion
-    const headers = this.rawRequestHeadersPromise() ? await this.rawRequestHeadersPromise()! : this._headers;
+    const headers = await this.rawRequestHeaders();
     for (const header of headers)
       headersSize += header.name.length + header.value.length + 4; // 4 = ': ' + '\r\n'
     return headersSize;
@@ -344,11 +337,11 @@ export type RemoteAddr = {
 };
 
 export type SecurityDetails = {
-    protocol?: string;
+  protocol?: string;
-    subjectName?: string;
+  subjectName?: string;
-    issuer?: string;
+  issuer?: string;
-    validFrom?: number;
+  validFrom?: number;
-    validTo?: number;
+  validTo?: number;
 };
 
 export class Response extends SdkObject {
@@ -364,7 +357,7 @@ export class Response extends SdkObject {
   private _timing: ResourceTiming;
   private _serverAddrPromise = new ManualPromise<RemoteAddr | undefined>();
   private _securityDetailsPromise = new ManualPromise<SecurityDetails | undefined>();
-  private _rawResponseHeadersPromise: ManualPromise<types.HeadersArray> | undefined;
+  private _rawResponseHeadersPromise = new ManualPromise<types.HeadersArray>();
   private _httpVersion: string | undefined;
   private _fromServiceWorker: boolean;
 
@@ -393,6 +386,9 @@ export class Response extends SdkObject {
   }
 
   _requestFinished(responseEndTiming: number) {
+    // If we didn't get raw headers, declare them equal to provisional.
+    this.setRawResponseHeaders(null);
+    this._request.setRawRequestHeaders(null);
     this._request._responseEndTiming = Math.max(responseEndTiming, this._timing.responseStart);
     this._finishedPromise.resolve();
   }
@@ -422,18 +418,13 @@ export class Response extends SdkObject {
   }
 
   async rawResponseHeaders(): Promise<NameValue[]> {
-    return this._rawResponseHeadersPromise || Promise.resolve(this._headers);
+    return this._rawResponseHeadersPromise;
   }
 
-  setWillReceiveExtraHeaders() {
+  // "null" means no raw headers available - we'll use provisional headers as raw headers.
-    this._request.setWillReceiveExtraHeaders();
+  setRawResponseHeaders(headers: types.HeadersArray | null) {
-    this._rawResponseHeadersPromise = new ManualPromise();
+    if (!this._rawResponseHeadersPromise.isDone())
-  }
+      this._rawResponseHeadersPromise.resolve(headers || this._headers);
-
-  setRawResponseHeaders(headers: types.HeadersArray) {
-    if (!this._rawResponseHeadersPromise)
-      this._rawResponseHeadersPromise = new ManualPromise();
-    this._rawResponseHeadersPromise!.resolve(headers);
   }
 
   timing(): ResourceTiming {

packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts

@@ -88,7 +88,10 @@ export class WKInterceptableRequest {
       responseStart: timingPayload ? wkMillisToRoundishMillis(timingPayload.responseStart) : -1,
     };
     const setCookieSeparator = process.platform === 'darwin' ? ',' : '\n';
-    return new network.Response(this.request, responsePayload.status, responsePayload.statusText, headersObjectToArray(responsePayload.headers, ',', setCookieSeparator), timing, getResponseBody, responsePayload.source === 'service-worker');
+    const response = new network.Response(this.request, responsePayload.status, responsePayload.statusText, headersObjectToArray(responsePayload.headers, ',', setCookieSeparator), timing, getResponseBody, responsePayload.source === 'service-worker');
+    // No raw response headers in WebKit, use "provisional" ones.
+    response.setRawResponseHeaders(null);
+    return response;
   }
 }
 

packages/playwright-core/src/server/webkit/wkPage.ts

@@ -1034,6 +1034,9 @@ export class WKPage implements PageDelegate {
       session.sendMayFail('Network.interceptRequestWithError', { errorType: 'Cancellation', requestId: event.requestId });
       return;
     }
+    // There is no point in waiting for the raw headers in Network.responseReceived when intercepting.
+    // Use provisional headers as raw headers, so that client can call allHeaders() from the route handler.
+    request.request.setRawRequestHeaders(null);
     if (!request._route) {
       // Intercepted, although we do not intend to allow interception.
       // Just continue.
@@ -1055,6 +1058,9 @@ export class WKPage implements PageDelegate {
       if (!headers['host'])
         headers['Host'] = new URL(request.request.url()).host;
       request.request.setRawRequestHeaders(headersObjectToArray(headers));
+    } else {
+      // No raw headers avaialable, use provisional ones.
+      request.request.setRawRequestHeaders(null);
     }
     this._page._frameManager.requestReceivedResponse(response);
 

tests/page/page-network-request.spec.ts

@@ -93,6 +93,30 @@ it('should get the same headers as the server', async ({ page, server, browserNa
   expect(headers).toEqual(serverRequest.headers);
 });
 
+it('should not return allHeaders() until they are available', async ({ page, server, browserName, platform }) => {
+  it.fail(browserName === 'webkit' && platform === 'win32', 'Curl does not show accept-encoding and accept-language');
+
+  let requestHeadersPromise;
+  page.on('request', request => requestHeadersPromise = request.allHeaders());
+  let responseHeadersPromise;
+  page.on('response', response => responseHeadersPromise = response.allHeaders());
+
+  let serverRequest;
+  server.setRoute('/empty.html', async (request, response) => {
+    serverRequest = request;
+    response.writeHead(200, { 'foo': 'bar' });
+    await new Promise(f => setTimeout(f, 3000));
+    response.end('done');
+  });
+
+  await page.goto(server.PREFIX + '/empty.html');
+  const requestHeaders = await requestHeadersPromise;
+  expect(requestHeaders).toEqual(serverRequest.headers);
+
+  const responseHeaders = await responseHeadersPromise;
+  expect(responseHeaders['foo']).toBe('bar');
+});
+
 it('should get the same headers as the server CORS', async ({ page, server, browserName, platform }) => {
   it.fail(browserName === 'webkit' && platform === 'win32', 'Curl does not show accept-encoding and accept-language');