From 86fc1c39301ede302485da28d7ab39277a8bdcea Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 11 Jun 2021 10:35:01 +0200 Subject: [PATCH 1/3] Carve out exception for external SVGs Signed-off-by: Eric Peterson --- .../reader/transformers/addBaseUrl.test.ts | 65 ++++++++++++++++++- .../src/reader/transformers/addBaseUrl.ts | 19 +++++- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/plugins/techdocs/src/reader/transformers/addBaseUrl.test.ts b/plugins/techdocs/src/reader/transformers/addBaseUrl.test.ts index 2b17d04144..5a3205f886 100644 --- a/plugins/techdocs/src/reader/transformers/addBaseUrl.test.ts +++ b/plugins/techdocs/src/reader/transformers/addBaseUrl.test.ts @@ -19,12 +19,15 @@ import { addBaseUrl } from '../transformers'; import { TechDocsStorageApi } from '../../api'; const DOC_STORAGE_URL = 'https://example-host.storage.googleapis.com'; +const API_ORIGIN_URL = 'https://backstage.example.com/api/techdocs'; const techdocsStorageApi: TechDocsStorageApi = { - getBaseUrl: jest.fn(() => Promise.resolve(DOC_STORAGE_URL)), + getBaseUrl: jest.fn(o => + Promise.resolve(new URL(o, DOC_STORAGE_URL).toString()), + ), getEntityDocs: () => new Promise(resolve => resolve('yes!')), syncEntityDocs: () => new Promise(resolve => resolve(true)), - getApiOrigin: jest.fn(), + getApiOrigin: jest.fn(() => new Promise(resolve => resolve(API_ORIGIN_URL))), getBuilder: jest.fn(), getStorageUrl: jest.fn(), }; @@ -96,7 +99,7 @@ describe('addBaseUrl', () => { ); }); - it('transforms svg img src to data uri', async () => { + it('inlines svg img src to data uri', async () => { const svgContent = ''; const expectedSrc = `data:image/svg+xml;base64,${Buffer.from( svgContent, @@ -125,4 +128,60 @@ describe('addBaseUrl', () => { }); }); }); + + it('inlines absolute url svgs pointed at our backend', async () => { + const svgContent = ''; + const expectedSrc = `data:image/svg+xml;base64,${Buffer.from( + svgContent, + ).toString('base64')}`; + + (global.fetch as jest.Mock).mockReturnValue({ + text: jest.fn().mockResolvedValue(svgContent), + }); + + const root = createTestShadowDom( + ``, + { + preTransformers: [ + addBaseUrl({ + techdocsStorageApi, + entityId: mockEntityId, + path: '', + }), + ], + postTransformers: [], + }, + ); + + await new Promise(done => { + process.nextTick(() => { + const actualSrc = root.getElementById('x')?.getAttribute('src'); + expect(expectedSrc).toEqual(actualSrc); + done(); + }); + }); + }); + + it('does not inline external svgs', async () => { + const expectedSrc = 'https://example.com/test.svg'; + const root = createTestShadowDom(``, { + preTransformers: [ + addBaseUrl({ + techdocsStorageApi, + entityId: mockEntityId, + path: '', + }), + ], + postTransformers: [], + }); + + await new Promise(done => { + process.nextTick(() => { + const actualElem = root.getElementById('x'); + expect(actualElem?.getAttribute('src')).toEqual(expectedSrc); + expect(actualElem?.getAttribute('alt')).toEqual(null); + done(); + }); + }); + }); }); diff --git a/plugins/techdocs/src/reader/transformers/addBaseUrl.ts b/plugins/techdocs/src/reader/transformers/addBaseUrl.ts index 98db162b46..dc4eecde16 100644 --- a/plugins/techdocs/src/reader/transformers/addBaseUrl.ts +++ b/plugins/techdocs/src/reader/transformers/addBaseUrl.ts @@ -23,6 +23,22 @@ type AddBaseUrlOptions = { path: string; }; +/** + * TechDocs backend serves SVGs with text/plain content-type for security. This + * helper determines if an SVG is being loaded from the backend, and thus needs + * inlining to be displayed properly. + */ +const isSvgNeedingInlining = ( + attrName: string, + attrVal: string, + apiOrigin: string, +) => { + const isSrcToSvg = attrName === 'src' && attrVal.endsWith('.svg'); + const isRelativeUrl = !attrVal.match(/^([a-z]*:)?\/\//i); + const pointsToOurBackend = attrVal.startsWith(apiOrigin); + return isSrcToSvg && (isRelativeUrl || pointsToOurBackend); +}; + export const addBaseUrl = ({ techdocsStorageApi, entityId, @@ -45,7 +61,8 @@ export const addBaseUrl = ({ entityId, path, ); - if (attributeName === 'src' && elemAttribute.endsWith('.svg')) { + const apiOrigin = await techdocsStorageApi.getApiOrigin(); + if (isSvgNeedingInlining(attributeName, elemAttribute, apiOrigin)) { try { const svg = await fetch(newValue); const svgContent = await svg.text(); From 9b57fda8b2191b2222242fecdab77c33931d0575 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 11 Jun 2021 10:41:19 +0200 Subject: [PATCH 2/3] Changeset Signed-off-by: Eric Peterson --- .changeset/techdocs-a-primeira-vez.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/techdocs-a-primeira-vez.md diff --git a/.changeset/techdocs-a-primeira-vez.md b/.changeset/techdocs-a-primeira-vez.md new file mode 100644 index 0000000000..efc626091c --- /dev/null +++ b/.changeset/techdocs-a-primeira-vez.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-techdocs': patch +--- + +Fixes a bug that could prevent some externally hosted images (like icons or +build badges) from rendering within TechDocs documentation. From 0606f9b416fcdeb674b295bdd89d8cd87be02d0d Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Thu, 17 Jun 2021 09:30:36 +0200 Subject: [PATCH 3/3] Clean up tests. Signed-off-by: Eric Peterson --- .../reader/transformers/addBaseUrl.test.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/plugins/techdocs/src/reader/transformers/addBaseUrl.test.ts b/plugins/techdocs/src/reader/transformers/addBaseUrl.test.ts index 5a3205f886..6b6eae4b0d 100644 --- a/plugins/techdocs/src/reader/transformers/addBaseUrl.test.ts +++ b/plugins/techdocs/src/reader/transformers/addBaseUrl.test.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { waitFor } from '@testing-library/react'; import { createTestShadowDom } from '../../test-utils'; import { addBaseUrl } from '../transformers'; import { TechDocsStorageApi } from '../../api'; @@ -120,12 +121,9 @@ describe('addBaseUrl', () => { postTransformers: [], }); - await new Promise(done => { - process.nextTick(() => { - const actualSrc = root.getElementById('x')?.getAttribute('src'); - expect(expectedSrc).toEqual(actualSrc); - done(); - }); + await waitFor(() => { + const actualSrc = root.getElementById('x')?.getAttribute('src'); + expect(expectedSrc).toEqual(actualSrc); }); }); @@ -153,12 +151,9 @@ describe('addBaseUrl', () => { }, ); - await new Promise(done => { - process.nextTick(() => { - const actualSrc = root.getElementById('x')?.getAttribute('src'); - expect(expectedSrc).toEqual(actualSrc); - done(); - }); + await waitFor(() => { + const actualSrc = root.getElementById('x')?.getAttribute('src'); + expect(expectedSrc).toEqual(actualSrc); }); });