From 23daf84cdd6d2041e7c93877e43217249e931b8b Mon Sep 17 00:00:00 2001 From: Chad Sheets Date: Thu, 2 Sep 2021 08:31:25 -0700 Subject: [PATCH] fix(har): favicon redirect handling (#8176) --- src/server/network.ts | 2 +- tests/browsercontext-network-event.spec.ts | 43 +++++++++++++++++++ tests/har.spec.ts | 48 ++++++++++++++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/server/network.ts b/src/server/network.ts index 57b8b77bd6..a45ba3aba2 100644 --- a/src/server/network.ts +++ b/src/server/network.ts @@ -119,7 +119,7 @@ export class Request extends SdkObject { this._headers = headers; for (const { name, value } of this._headers) this._headersMap.set(name.toLowerCase(), value); - this._isFavicon = url.endsWith('/favicon.ico'); + this._isFavicon = url.endsWith('/favicon.ico') || !!redirectedFrom?._isFavicon; } _setFailureText(failureText: string) { diff --git a/tests/browsercontext-network-event.spec.ts b/tests/browsercontext-network-event.spec.ts index 770f1d17d8..ab6616cac2 100644 --- a/tests/browsercontext-network-event.spec.ts +++ b/tests/browsercontext-network-event.spec.ts @@ -107,3 +107,46 @@ it('should fire events in proper order', async ({browser, server}) => { 'requestfinished', ]); }); + +it('should not fire events for favicon or favicon redirects', async ({browser, server, browserName, channel, headless}) => { + it.skip(headless && browserName !== 'firefox', 'headless browsers, except firefox, do not request favicons'); + it.skip(!headless && browserName === 'webkit' && !channel, 'headed webkit does not have a favicon feature'); + const context = await browser.newContext(); + const page = await context.newPage(); + const favicon = `/favicon.ico`; + const hashedFaviconUrl = `/favicon-hashed.ico`; + const imagePath = `/fakeimage.png`; + const pagePath = `/page.html`; + server.setRedirect(favicon, hashedFaviconUrl); + server.setRoute(pagePath, (_, res) => { + res.end(` + + + + + + SVG Favicon Test + + + my fake image + + +`); + }); + + const events = []; + context.on('request', req => events.push(req.url())); + context.on('response', res => events.push(res.url())); + context.on('requestfinished', req => events.push(req.url())); + + await Promise.all([ + server.waitForRequest(favicon), + server.waitForRequest(hashedFaviconUrl), + page.goto(server.PREFIX + '/page.html'), + ]); + + expect(events).toEqual(expect.arrayContaining([expect.stringContaining(pagePath)])); + expect(events).toEqual(expect.arrayContaining([expect.stringContaining(imagePath)])); + expect(events).not.toEqual(expect.arrayContaining([expect.stringContaining(favicon)])); + expect(events).not.toEqual(expect.arrayContaining([expect.stringContaining(hashedFaviconUrl)])); +}); diff --git a/tests/har.spec.ts b/tests/har.spec.ts index 1e752a7536..f25b674216 100644 --- a/tests/har.spec.ts +++ b/tests/har.spec.ts @@ -511,6 +511,54 @@ it('should contain http2 for http2 requests', async ({ contextFactory, browserNa server.close(); }); +it('should filter favicon and favicon redirects', async ({server, browserName, channel, headless, asset, contextFactory}, testInfo) => { + it.skip(headless && browserName !== 'firefox', 'headless browsers, except firefox, do not request favicons'); + it.skip(!headless && browserName === 'webkit' && !channel, 'headed webkit does not have a favicon feature'); + + const { page, getLog } = await pageWithHar(contextFactory, testInfo); + + // Browsers aggresively cache favicons, so force bust with the + // `d` parameter to make iterating on this test more predictable and isolated. + const favicon = `/favicon.ico`; + const hashedFaviconUrl = `/favicon-hashed.ico`; + server.setRedirect(favicon, hashedFaviconUrl); + server.setRoute(hashedFaviconUrl, (req, res) => { + server.serveFile(req, res, asset('media-query-prefers-color-scheme.svg')); + }); + + server.setRoute('/page.html', (_, res) => { + res.end(` + + + + + + SVG Favicon Test + + + favicons + + +`); + }); + + await Promise.all([ + server.waitForRequest(favicon), + server.waitForRequest(hashedFaviconUrl), + page.goto(server.PREFIX + '/page.html'), + ]); + + await page.waitForTimeout(500); + // Text still being around ensures we haven't actually lost our browser to a crash. + await page.waitForSelector('text=favicons'); + + // favicon and 302 redirects to favicons should be filtered out of request logs + const log = await getLog(); + expect(log.entries.length).toBe(1); + const entry = log.entries[0]; + expect(entry.request.url).toBe(server.PREFIX + '/page.html'); +}); + it('should have different hars for concurrent contexts', async ({ contextFactory }, testInfo) => { const session0 = await pageWithHar(contextFactory, testInfo, 'test-0.har'); await session0.page.goto('data:text/html,Zero');