fix(fetch): handle cookies on redirects (#8519)

src/server/fetch.ts

@@ -28,22 +28,11 @@ export async function playwrightFetch(context: BrowserContext, params: types.Fet
       for (const [name, value] of Object.entries(params.headers))
         headers[name.toLowerCase()] = value;
     }
-    if (headers['user-agent'] === undefined)
+    headers['user-agent'] ??= context._options.userAgent || context._browser.userAgent();
-      headers['user-agent'] = context._options.userAgent || context._browser.userAgent();
+    headers['accept'] ??= '*/*';
-    if (headers['accept'] === undefined)
+    headers['accept-encoding'] ??= 'gzip,deflate';
-      headers['accept'] = '*/*';
-    if (headers['accept-encoding'] === undefined)
-      headers['accept-encoding'] = 'gzip,deflate';
 
-    if (headers['cookie'] === undefined) {
+    const method = params.method?.toUpperCase() || 'GET';
-      const cookies = await context.cookies(params.url);
-      if (cookies.length) {
-        const valueArray = cookies.map(c => `${c.name}=${c.value}`);
-        headers['cookie'] = valueArray.join('; ');
-      }
-    }
-    if (!params.method)
-      params.method = 'GET';
     let agent;
     if (context._options.proxy) {
       // TODO: support bypass proxy
@@ -54,14 +43,12 @@ export async function playwrightFetch(context: BrowserContext, params: types.Fet
     }
 
     // TODO(https://github.com/microsoft/playwright/issues/8381): set user agent
-    const {fetchResponse, setCookie} = await sendRequest(new URL(params.url), {
+    const fetchResponse = await sendRequest(context, new URL(params.url), {
-      method: params.method,
+      method,
-      headers: headers,
+      headers,
       agent,
       maxRedirects: 20
     }, params.postData);
-    if (setCookie)
-      await updateCookiesFromHeader(context, fetchResponse.url, setCookie);
     return { fetchResponse };
   } catch (e) {
     return { error: String(e) };
@@ -71,7 +58,7 @@ export async function playwrightFetch(context: BrowserContext, params: types.Fet
 async function updateCookiesFromHeader(context: BrowserContext, responseUrl: string, setCookie: string[]) {
   const url = new URL(responseUrl);
   // https://datatracker.ietf.org/doc/html/rfc6265#section-5.1.4
-  const defaultPath = '/' + url.pathname.split('/').slice(0, -1).join('/');
+  const defaultPath = '/' + url.pathname.substr(1).split('/').slice(0, -1).join('/');
   const cookies: types.SetNetworkCookieParam[] = [];
   for (const header of setCookie) {
     // Decode cookie value?
@@ -91,48 +78,57 @@ async function updateCookiesFromHeader(context: BrowserContext, responseUrl: str
     await context.addCookies(cookies);
 }
 
-type Response = {
+async function updateRequestCookieHeader(context: BrowserContext, url: URL, options: http.RequestOptions) {
-  fetchResponse: types.FetchResponse,
+  if (options.headers!['cookie'] !== undefined)
-  setCookie?: string[]
+    return;
-};
+  const cookies = await context.cookies(url.toString());
+  if (cookies.length) {
+    const valueArray = cookies.map(c => `${c.name}=${c.value}`);
+    options.headers!['cookie'] = valueArray.join('; ');
+  }
+}
 
-async function sendRequest(url: URL, options: http.RequestOptions & { maxRedirects: number }, postData?: Buffer): Promise<Response>{
+async function sendRequest(context: BrowserContext, url: URL, options: http.RequestOptions & { maxRedirects: number }, postData?: Buffer): Promise<types.FetchResponse>{
-  return new Promise<Response>((fulfill, reject) => {
+  await updateRequestCookieHeader(context, url, options);
+  return new Promise<types.FetchResponse>((fulfill, reject) => {
     const requestConstructor: ((url: URL, options: http.RequestOptions, callback?: (res: http.IncomingMessage) => void) => http.ClientRequest)
       = (url.protocol === 'https:' ? https : http).request;
-    const request = requestConstructor(url, options, response => {
+    const request = requestConstructor(url, options, async response => {
+      if (response.headers['set-cookie'])
+        await updateCookiesFromHeader(context, response.url || url.toString(), response.headers['set-cookie']);
       if (redirectStatus.includes(response.statusCode!)) {
         if (!options.maxRedirects) {
           reject(new Error('Max redirect count exceeded'));
           request.abort();
           return;
         }
+        const headers = { ...options.headers };
+        delete headers[`cookie`];
+
+        // HTTP-redirect fetch step 13 (https://fetch.spec.whatwg.org/#http-redirect-fetch)
+        const status = response.statusCode!;
+        let method = options.method!;
+        if ((status === 301 || status === 302) && method === 'POST' ||
+             status === 303 && !['GET', 'HEAD'].includes(method)) {
+          method = 'GET';
+          postData = undefined;
+          delete headers[`content-encoding`];
+          delete headers[`content-language`];
+          delete headers[`content-location`];
+          delete headers[`content-type`];
+        }
+
         const redirectOptions: http.RequestOptions & { maxRedirects: number } = {
-          method: options.method,
+          method,
-          headers: { ...options.headers },
+          headers,
           agent: options.agent,
           maxRedirects: options.maxRedirects - 1,
         };
 
-        // HTTP-redirect fetch step 13 (https://fetch.spec.whatwg.org/#http-redirect-fetch)
-        const status = response.statusCode!;
-        const method = redirectOptions.method!;
-        if ((status === 301 || status === 302) && method === 'POST' ||
-             status === 303 && !['GET', 'HEAD'].includes(method)) {
-          redirectOptions.method = 'GET';
-          postData = undefined;
-          delete redirectOptions.headers?.[`content-encoding`];
-          delete redirectOptions.headers?.[`content-language`];
-          delete redirectOptions.headers?.[`content-location`];
-          delete redirectOptions.headers?.[`content-type`];
-        }
-
-        // TODO: set-cookie from response, add cookie from the context.
-
         // HTTP-redirect fetch step 4: If locationURL is null, then return response.
         if (response.headers.location) {
           const locationURL = new URL(response.headers.location, url);
-          fulfill(sendRequest(locationURL, redirectOptions, postData));
+          fulfill(sendRequest(context, locationURL, redirectOptions, postData));
           request.abort();
           return;
         }
@@ -142,14 +138,11 @@ async function sendRequest(url: URL, options: http.RequestOptions & { maxRedirec
       response.on('end', () => {
         const body = Buffer.concat(chunks);
         fulfill({
-          fetchResponse: {
+          url: response.url || url.toString(),
-            url: response.url || url.toString(),
+          status: response.statusCode || 0,
-            status: response.statusCode || 0,
+          statusText: response.statusMessage || '',
-            statusText: response.statusMessage || '',
+          headers: flattenHeaders(response.headers),
-            headers: flattenHeaders(response.headers),
+          body
-            body
-          },
-          setCookie: response.headers['set-cookie']
         });
       });
       response.on('error',reject);

tests/browsercontext-fetch.spec.ts

@@ -136,6 +136,66 @@ it('should add cookies from Set-Cookie header', async ({context, page, server})
   expect((await page.evaluate(() => document.cookie)).split(';').map(s => s.trim()).sort()).toEqual(['foo=bar', 'session=value']);
 });
 
+it('should handle cookies on redirects', async ({context, server}) => {
+  server.setRoute('/redirect1', (req, res) => {
+    res.setHeader('Set-Cookie', 'r1=v1;SameSite=Lax');
+    res.writeHead(301, { location: '/a/b/redirect2' });
+    res.end();
+  });
+  server.setRoute('/a/b/redirect2', (req, res) => {
+    res.setHeader('Set-Cookie', 'r2=v2;SameSite=Lax');
+    res.writeHead(302, { location: '/title.html' });
+    res.end();
+  });
+  {
+    const [req1, req2, req3] = await Promise.all([
+      server.waitForRequest('/redirect1'),
+      server.waitForRequest('/a/b/redirect2'),
+      server.waitForRequest('/title.html'),
+      // @ts-expect-error
+      context._fetch(`${server.PREFIX}/redirect1`),
+    ]);
+    expect(req1.headers.cookie).toBeFalsy();
+    expect(req2.headers.cookie).toBe('r1=v1');
+    expect(req3.headers.cookie).toBe('r1=v1');
+  }
+  {
+    const [req1, req2, req3] = await Promise.all([
+      server.waitForRequest('/redirect1'),
+      server.waitForRequest('/a/b/redirect2'),
+      server.waitForRequest('/title.html'),
+      // @ts-expect-error
+      context._fetch(`${server.PREFIX}/redirect1`),
+    ]);
+    expect(req1.headers.cookie).toBe('r1=v1');
+    expect(req2.headers.cookie.split(';').map(s => s.trim()).sort()).toEqual(['r1=v1', 'r2=v2']);
+    expect(req3.headers.cookie).toBe('r1=v1');
+  }
+  const cookies = await context.cookies();
+  expect(new Set(cookies)).toEqual(new Set([
+    {
+      'sameSite': 'Lax',
+      'name': 'r2',
+      'value': 'v2',
+      'domain': 'localhost',
+      'path': '/a/b',
+      'expires': -1,
+      'httpOnly': false,
+      'secure': false
+    },
+    {
+      'sameSite': 'Lax',
+      'name': 'r1',
+      'value': 'v1',
+      'domain': 'localhost',
+      'path': '/',
+      'expires': -1,
+      'httpOnly': false,
+      'secure': false
+    }
+  ]));
+});
+
 it('should work with context level proxy', async ({browserOptions, browserType, contextOptions, server, proxyServer}) => {
   server.setRoute('/target.html', async (req, res) => {
     res.end('<title>Served by the proxy</title>');
@@ -208,6 +268,19 @@ it('should add default headers', async ({context, server, page}) => {
   expect(request.headers['accept-encoding']).toBe('gzip,deflate');
 });
 
+it('should add default headers to redirects', async ({context, server, page}) => {
+  server.setRedirect('/redirect', '/empty.html');
+  const [request] = await Promise.all([
+    server.waitForRequest('/empty.html'),
+    // @ts-expect-error
+    context._fetch(`${server.PREFIX}/redirect`)
+  ]);
+  expect(request.headers['accept']).toBe('*/*');
+  const userAgent = await page.evaluate(() => navigator.userAgent);
+  expect(request.headers['user-agent']).toBe(userAgent);
+  expect(request.headers['accept-encoding']).toBe('gzip,deflate');
+});
+
 it('should allow to override default headers', async ({context, server, page}) => {
   const [request] = await Promise.all([
     server.waitForRequest('/empty.html'),
@@ -224,3 +297,19 @@ it('should allow to override default headers', async ({context, server, page}) =
   expect(request.headers['user-agent']).toBe('Playwright');
   expect(request.headers['accept-encoding']).toBe('br');
 });
+
+it('should propagate custom headers with redirects', async ({context, server}) => {
+  server.setRedirect('/a/redirect1', '/b/c/redirect2');
+  server.setRedirect('/b/c/redirect2', '/simple.json');
+  const [req1, req2, req3] = await Promise.all([
+    server.waitForRequest('/a/redirect1'),
+    server.waitForRequest('/b/c/redirect2'),
+    server.waitForRequest('/simple.json'),
+    // @ts-expect-error
+    context._fetch(`${server.PREFIX}/a/redirect1`, {headers: {'foo': 'bar'}}),
+  ]);
+  expect(req1.headers['foo']).toBe('bar');
+  expect(req2.headers['foo']).toBe('bar');
+  expect(req3.headers['foo']).toBe('bar');
+});
+