diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index f31d4b7fc..95e6cb39a 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -460,8 +460,30 @@ function readWarnThresholdBytes() { // the actual coldest entry if needed. DISK_SPILL_KEY is only set when the // ByteLRU tier is active (see createDiscoveryQueue 'start' handler), so the // cache here is guaranteed to be a ByteLRU when we enter this branch. -export function lookupCacheResource(percy, snapshotResources, cache, url, width) { - let resource = snapshotResources.get(url) || cache.get(url); +export function lookupCacheResource(percy, snapshotResources, cache, url, width, rootUrl) { + let resource = snapshotResources.get(url); + + // @percy/dom keys the resources it fabricates by a root-relative URL + // (/__serialized__/.) so no host or scheme is baked into the + // snapshot; the discovery browser requests the resolved absolute form. + // Match on the origin-less form (path + query), but only for requests from + // the snapshot's own origin: a root-relative href can resolve nowhere else, + // and an allowed cross-origin request must never be answered with the + // snapshot's bytes. Both snapshot lookups run before the shared cache so a + // snapshot's own provided bytes always win over whatever an earlier fetch + // of the same URL left cached. + if (!resource && rootUrl) { + try { + let { origin, pathname, search } = new URL(url); + if (origin === new URL(rootUrl).origin) { + resource = snapshotResources.get(pathname + search); + } + } catch (e) { + // url is not absolute -- nothing further to try + } + } + if (!resource) resource = cache.get(url); + const disk = percy[DISK_SPILL_KEY]; if (!resource && disk) { resource = disk.get(url); @@ -652,7 +674,7 @@ export function createDiscoveryQueue(percy) { allowedHostnames: snapshot.discovery.allowedHostnames, disallowedHostnames: snapshot.discovery.disallowedHostnames, getResource: (u, width = null) => ( - lookupCacheResource(percy, snapshot.resources, cache, u, width) + lookupCacheResource(percy, snapshot.resources, cache, u, width, snapshot.url) ), saveResource: r => { const limitResources = process.env.LIMIT_SNAPSHOT_RESOURCES || false; diff --git a/packages/core/test/unit/byte-lru.test.js b/packages/core/test/unit/byte-lru.test.js index fd782f63b..fc4055e11 100644 --- a/packages/core/test/unit/byte-lru.test.js +++ b/packages/core/test/unit/byte-lru.test.js @@ -616,6 +616,57 @@ describe('Unit / lookupCacheResource', () => { expect(lookupCacheResource(percy, snapshotResources, cache, 'a')).toBe(local); }); + it('matches an absolute request against a root-relative snapshot key', () => { + // @percy/dom keys fabricated resources by /__serialized__/.; + // the discovery browser asks for the resolved absolute URL. + const { percy } = makePercy(undefined); + const provided = { url: '/__serialized__/_abc.css', provided: true, content: Buffer.from('P') }; + const snapshotResources = new Map([['/__serialized__/_abc.css', provided]]); + expect(lookupCacheResource(percy, snapshotResources, new ByteLRU(), 'http://localhost:3000/__serialized__/_abc.css', undefined, 'http://localhost:3000/')).toBe(provided); + }); + + it('prefers a root-relative provided snapshot resource over a cached entry for the absolute URL', () => { + const { percy } = makePercy(undefined); + const provided = { url: '/__serialized__/_abc.css', provided: true, content: Buffer.from('SNAPSHOT') }; + const snapshotResources = new Map([['/__serialized__/_abc.css', provided]]); + const cache = new ByteLRU(); + cache.set('http://localhost:3000/__serialized__/_abc.css', { url: 'http://localhost:3000/__serialized__/_abc.css', content: Buffer.from('CACHED') }, 100); + expect(lookupCacheResource(percy, snapshotResources, cache, 'http://localhost:3000/__serialized__/_abc.css', undefined, 'http://localhost:3000/')).toBe(provided); + }); + + it('does not match a bare root-relative key when the absolute request carries a query string', () => { + const { percy } = makePercy(undefined); + const snapshotResources = new Map([['/__serialized__/_abc.css', { url: '/__serialized__/_abc.css', provided: true }]]); + expect(lookupCacheResource(percy, snapshotResources, new ByteLRU(), 'http://localhost:3000/__serialized__/_abc.css?theme=dark', undefined, 'http://localhost:3000/')).toBeUndefined(); + }); + + it('matches a root-relative key that includes a query string when the request carries the same one', () => { + const { percy } = makePercy(undefined); + const provided = { url: '/asset.css?theme=dark', provided: true, content: Buffer.from('DARK') }; + const snapshotResources = new Map([['/asset.css?theme=dark', provided]]); + expect(lookupCacheResource(percy, snapshotResources, new ByteLRU(), 'http://localhost:3000/asset.css?theme=dark', undefined, 'http://localhost:3000/')).toBe(provided); + }); + + it('does not apply the relative-key fallback to a request from another origin', () => { + // An allowed cross-origin request that happens to share the path must be + // served its own origin's bytes, never the snapshot's. + const { percy } = makePercy(undefined); + const snapshotResources = new Map([['/__serialized__/_abc.css', { url: '/__serialized__/_abc.css', provided: true }]]); + expect(lookupCacheResource(percy, snapshotResources, new ByteLRU(), 'https://cdn.example.com/__serialized__/_abc.css', undefined, 'http://localhost:3000/')).toBeUndefined(); + }); + + it('does not apply the relative-key fallback when no snapshot url is given', () => { + const { percy } = makePercy(undefined); + const snapshotResources = new Map([['/__serialized__/_abc.css', { url: '/__serialized__/_abc.css', provided: true }]]); + expect(lookupCacheResource(percy, snapshotResources, new ByteLRU(), 'http://localhost:3000/__serialized__/_abc.css')).toBeUndefined(); + }); + + it('does not match an absolute request whose pathname is not a snapshot key', () => { + const { percy } = makePercy(undefined); + const snapshotResources = new Map([['/__serialized__/_abc.css', { url: '/__serialized__/_abc.css' }]]); + expect(lookupCacheResource(percy, snapshotResources, new ByteLRU(), 'http://localhost:3000/__serialized__/_other.css', undefined, 'http://localhost:3000/')).toBeUndefined(); + }); + it('falls through to RAM cache when snapshot has no entry', () => { const { percy } = makePercy(undefined); const cache = new ByteLRU(); diff --git a/packages/dom/src/utils.js b/packages/dom/src/utils.js index ee9ee9cad..6e4566426 100644 --- a/packages/dom/src/utils.js +++ b/packages/dom/src/utils.js @@ -14,20 +14,24 @@ export function resourceFromDataURL(uid, dataURL) { let [, mimetype] = data.split(':'); [mimetype] = mimetype.split(';'); - // build a URL for the serialized asset + // build a root-relative URL for the serialized asset. It is same-origin by + // construction (served from wherever the captured page itself ends up), so + // it never needs a host or scheme baked in -- PPLT-6109: an absolute URL + // here previously carried over whatever scheme the local capture host used + // (typically http), which survives every downstream hostname rewrite and + // becomes a mixed-content block on any https render. let [, ext] = mimetype.split('/'); - let path = `/__serialized__/${uid}.${ext}`; - let url = rewriteLocalhostURL(new URL(path, document.URL).toString()); + let url = `/__serialized__/${uid}.${ext}`; // return the url, base64 content, and mimetype return { url, content, mimetype }; } export function resourceFromText(uid, mimetype, data) { - // build a URL for the serialized asset + // build a root-relative URL for the serialized asset -- see + // resourceFromDataURL above for why this is relative, not absolute. let [, ext] = mimetype.split('/'); - let path = `/__serialized__/${uid}.${ext}`; - let url = rewriteLocalhostURL(new URL(path, document.URL).toString()); + let url = `/__serialized__/${uid}.${ext}`; // return the url, text content, and mimetype return { url, content: data, mimetype }; } @@ -53,10 +57,6 @@ export function styleSheetFromNode(node) { } } -export function rewriteLocalhostURL(url) { - return url.replace(/(http[s]{0,1}:\/\/)(localhost|127.0.0.1)[:\d+]*/, '$1render.percy.local'); -} - // Utility function to handle errors export function handleErrors(error, prefixMessage, element = null, additionalData = {}) { let elementData = {}; diff --git a/packages/dom/test/serialize-dom.test.js b/packages/dom/test/serialize-dom.test.js index f498526ba..e3d25099a 100644 --- a/packages/dom/test/serialize-dom.test.js +++ b/packages/dom/test/serialize-dom.test.js @@ -654,17 +654,22 @@ describe('serializeDOM', () => { describe('error handling', () => { it('adds node details in error message and rethrow it', () => { - let oldURL = window.URL; - window.URL = undefined; withExample(` Example Image `); + // Fail inside the img's own serialization step (the base64 serializer is + // the only thing that sets this attribute) so the decorated error names it. + let setAttribute = window.Element.prototype.setAttribute; + spyOn(window.Element.prototype, 'setAttribute').and.callFake(function(name, value) { + if (name === 'data-percy-serialized-attribute-src') throw new Error('boom'); + return setAttribute.call(this, name, value); + }); + expect(() => serializeDOM()).toThrowMatching((error) => { return error.message.includes('Error cloning node:') && error.message.includes('{"nodeName":"IMG","classNames":"test1 test2","id":"test"}'); }); - window.URL = oldURL; }); it('ignores canvas serialization errors when flag is enabled', () => { diff --git a/packages/dom/test/utils.test.js b/packages/dom/test/utils.test.js index 128e402dd..bda9e7332 100644 --- a/packages/dom/test/utils.test.js +++ b/packages/dom/test/utils.test.js @@ -1,4 +1,4 @@ -import { resourceFromDataURL, resourceFromText, rewriteLocalhostURL, styleSheetFromNode } from '../src/utils'; +import { resourceFromDataURL, resourceFromText, styleSheetFromNode } from '../src/utils'; describe('utils', () => { describe('styleSheetFromNode', () => { it('creates stylesheet properly', () => { @@ -47,38 +47,28 @@ describe('utils', () => { describe('resourceFromDataURL', () => { const uid = (Math.random() + 1).toString(36).substring(10); const dataURL = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAHgAAAB4CAYAAAA5ZDbSAAAAAXNSR0IArs4c6QAACbVJREFUeF7tXAWoFVEQnW+'; - it('If URL is localhost, replace it to render.percy.local', () => { + // PPLT-6109: the URL is root-relative regardless of document.URL -- it is + // same-origin by construction, so it never needs a host or scheme baked in. + it('builds a root-relative URL regardless of document.URL', () => { Object.defineProperty(window.document, 'URL', { writable: true, value: 'http://localhost' }); const result = resourceFromDataURL(uid, dataURL); expect(result).toEqual({ - url: `http://render.percy.local/__serialized__/${uid}.png`, + url: `/__serialized__/${uid}.png`, content: 'iVBORw0KGgoAAAANSUhEUgAAAHgAAAB4CAYAAAA5ZDbSAAAAAXNSR0IArs4c6QAACbVJREFUeF7tXAWoFVEQnW+', mimetype: 'image/png' }); }); - it('If URL is 127.0.0.1, replace it to render.percy.local', () => { - Object.defineProperty(window.document, 'URL', { - writable: true, - value: 'http://127.0.0.1' - }); - const result = resourceFromDataURL(uid, dataURL); - expect(result).toEqual({ - url: `http://render.percy.local/__serialized__/${uid}.png`, - content: 'iVBORw0KGgoAAAANSUhEUgAAAHgAAAB4CAYAAAA5ZDbSAAAAAXNSR0IArs4c6QAACbVJREFUeF7tXAWoFVEQnW+', - mimetype: 'image/png' - }); - }); - it('If URL is not localhost, return as is', () => { + it('builds the same root-relative URL for a non-localhost document.URL', () => { Object.defineProperty(window.document, 'URL', { writable: true, value: 'http://example.com' }); const result = resourceFromDataURL(uid, dataURL); expect(result).toEqual({ - url: `http://example.com/__serialized__/${uid}.png`, + url: `/__serialized__/${uid}.png`, content: 'iVBORw0KGgoAAAANSUhEUgAAAHgAAAB4CAYAAAA5ZDbSAAAAAXNSR0IArs4c6QAACbVJREFUeF7tXAWoFVEQnW+', mimetype: 'image/png' }); @@ -87,63 +77,29 @@ describe('utils', () => { describe('resourceFromText', () => { const uid = (Math.random() + 1).toString(36).substring(10); const dataURL = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAHgAAAB4CAYAAAA5ZDbSAAAAAXNSR0IArs4c6QAACbVJREFUeF7tXAWoFVEQnW+'; - it('Replace localhost to render.percy.local', () => { + it('builds a root-relative URL regardless of document.URL', () => { Object.defineProperty(window.document, 'URL', { writable: true, value: 'http://localhost' }); const result = resourceFromText(uid, 'image/png', dataURL); expect(result).toEqual({ - url: `http://render.percy.local/__serialized__/${uid}.png`, + url: `/__serialized__/${uid}.png`, content: dataURL, mimetype: 'image/png' }); }); - it('Replace 127.0.0.1 to render.percy.local', () => { - Object.defineProperty(window.document, 'URL', { - writable: true, - value: 'http://127.0.0.1' - }); - const result = resourceFromText(uid, 'image/png', dataURL); - expect(result).toEqual({ - url: `http://render.percy.local/__serialized__/${uid}.png`, - content: dataURL, - mimetype: 'image/png' - }); - }); - it('If URL is not localhost, return as is', () => { + it('builds the same root-relative URL for a non-localhost document.URL', () => { Object.defineProperty(window.document, 'URL', { writable: true, value: 'http://example.com' }); const result = resourceFromText(uid, 'image/png', dataURL); expect(result).toEqual({ - url: `http://example.com/__serialized__/${uid}.png`, + url: `/__serialized__/${uid}.png`, content: dataURL, mimetype: 'image/png' }); }); }); - describe('rewriteLocalhostURL', () => { - it('should replace with render.percy.local', () => { - const case1 = rewriteLocalhostURL('https://localhost/hello'); - expect(case1).toEqual('https://render.percy.local/hello'); - const case2 = rewriteLocalhostURL('http://localhost:4000/hello'); - expect(case2).toEqual('http://render.percy.local/hello'); - const case3 = rewriteLocalhostURL('http://localhost/hello'); - expect(case3).toEqual('http://render.percy.local/hello'); - const case4 = rewriteLocalhostURL('https://localhost:4000/hello'); - expect(case4).toEqual('https://render.percy.local/hello'); - }); - it('Should not replace url', () => { - const case1 = rewriteLocalhostURL('http://hello.com/localhost/'); - expect(case1).toEqual('http://hello.com/localhost/'); - const case2 = rewriteLocalhostURL('http://hello/world'); - expect(case2).toEqual('http://hello/world'); - const case3 = rewriteLocalhostURL('http://hellolocalhost:2000/world'); - expect(case3).toEqual('http://hellolocalhost:2000/world'); - const case4 = rewriteLocalhostURL('https://hellolocalhost:2000/world'); - expect(case4).toEqual('https://hellolocalhost:2000/world'); - }); - }); });