From 4916a6e23d8b4f8549134320995129f809bd1a86 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 7 May 2021 14:51:09 +0200 Subject: [PATCH 1/6] Decode req.path before loading object from GCS Signed-off-by: Eric Peterson --- .../__mocks__/@google-cloud/storage.ts | 14 ++++--- packages/techdocs-common/package.json | 4 +- .../src/stages/publish/googleStorage.test.ts | 41 +++++++++++++++++++ .../src/stages/publish/googleStorage.ts | 4 +- 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts index f8d0368f27..11fbc32de2 100644 --- a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts +++ b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { EventEmitter } from 'events'; +import { Readable, Writable } from 'stream'; import fs from 'fs-extra'; import os from 'os'; import path from 'path'; @@ -58,19 +58,21 @@ class GCSFile { } createReadStream() { - const emitter = new EventEmitter(); + const readable = new Readable(); + readable._read = () => {}; + process.nextTick(() => { if (fs.existsSync(this.localFilePath)) { - emitter.emit('data', Buffer.from(fs.readFileSync(this.localFilePath))); - emitter.emit('end'); + readable.emit('data', fs.readFileSync(this.localFilePath)); + readable.emit('end'); } else { - emitter.emit( + readable.emit( 'error', new Error(`The file ${this.localFilePath} does not exist !`), ); } }); - return emitter; + return readable; } } diff --git a/packages/techdocs-common/package.json b/packages/techdocs-common/package.json index f2d19ba8c5..7c12f6f675 100644 --- a/packages/techdocs-common/package.json +++ b/packages/techdocs-common/package.json @@ -67,7 +67,9 @@ "@types/mime-types": "^2.1.0", "@types/mock-fs": "^4.13.0", "@types/pkgcloud": "^1.7.4", - "@types/recursive-readdir": "^2.2.0" + "@types/recursive-readdir": "^2.2.0", + "@types/supertest": "^2.0.8", + "supertest": "^6.1.3" }, "jest": { "roots": [ diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index 19e4938763..9a966e6a60 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -21,6 +21,8 @@ import { EntityName, } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; +import express from 'express'; +import request from 'supertest'; import mockFs from 'mock-fs'; import os from 'os'; import path from 'path'; @@ -271,4 +273,43 @@ describe('GoogleGCSPublish', () => { }); }); }); + + describe('docsRouter', () => { + let app: express.Express; + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + beforeEach(() => { + app = express().use(publisher.docsRouter()); + + mockFs.restore(); + mockFs({ + [entityRootDir]: { + img: { + 'with spaces.png': 'found it', + }, + 'some folder': { + 'also with spaces.js': 'found it too', + }, + }, + }); + }); + + it('should pass expected object path to bucket', async () => { + const { + kind, + metadata: { namespace, name }, + } = entity; + + // Ensures leading slash is trimmed and encoded path is decoded. + const pngResponse = await request(app).get( + `/${namespace}/${kind}/${name}/img/with%20spaces.png`, + ); + expect(pngResponse.text).toEqual('found it'); + const jsResponse = await request(app).get( + `/${namespace}/${kind}/${name}/some%20folder/also%20with%20spaces.js`, + ); + expect(jsResponse.text).toEqual('found it too'); + }); + }); }); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index 8adafc361a..e5e3d6c25a 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -187,9 +187,9 @@ export class GoogleGCSPublish implements PublisherBase { */ docsRouter(): express.Handler { return (req, res) => { - // Trim the leading forward slash + // Decode and trim the leading forward slash // filePath example - /default/Component/documented-component/index.html - const filePath = req.path.replace(/^\//, ''); + const filePath = decodeURI(req.path.replace(/^\//, '')); // Files with different extensions (CSS, HTML) need to be served with different headers const fileExtension = path.extname(filePath); From 3ebfa8f189e7a029f4743e6e5ed923b24a8bdd16 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 7 May 2021 14:57:19 +0200 Subject: [PATCH 2/6] Decode req.path before loading object from S3 bucket. Signed-off-by: Eric Peterson --- .../src/stages/publish/awsS3.test.ts | 43 +++++++++++++++++++ .../src/stages/publish/awsS3.ts | 4 +- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index f094457346..fe0fbd556a 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -21,6 +21,8 @@ import { ENTITY_DEFAULT_NAMESPACE, } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; +import express from 'express'; +import request from 'supertest'; import mockFs from 'mock-fs'; import os from 'os'; import path from 'path'; @@ -272,4 +274,45 @@ describe('AwsS3Publish', () => { }); }); }); + + describe('docsRouter', () => { + let app: express.Express; + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + beforeEach(() => { + app = express().use(publisher.docsRouter()); + + mockFs.restore(); + mockFs({ + [entityRootDir]: { + img: { + 'with spaces.png': 'found it', + }, + 'some folder': { + 'also with spaces.js': 'found it too', + }, + }, + }); + }); + + it('should pass expected object path to bucket', async () => { + const { + kind, + metadata: { namespace, name }, + } = entity; + + // Ensures leading slash is trimmed and encoded path is decoded. + const pngResponse = await request(app).get( + `/${namespace}/${kind}/${name}/img/with%20spaces.png`, + ); + expect(Buffer.from(pngResponse.body).toString('utf8')).toEqual( + 'found it', + ); + const jsResponse = await request(app).get( + `/${namespace}/${kind}/${name}/some%20folder/also%20with%20spaces.js`, + ); + expect(jsResponse.text).toEqual('found it too'); + }); + }); }); diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index d628c860fd..e1ffb107cc 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -263,9 +263,9 @@ export class AwsS3Publish implements PublisherBase { */ docsRouter(): express.Handler { return async (req, res) => { - // Trim the leading forward slash + // Decode and trim the leading forward slash // filePath example - /default/Component/documented-component/index.html - const filePath = req.path.replace(/^\//, ''); + const filePath = decodeURI(req.path.replace(/^\//, '')); // Files with different extensions (CSS, HTML) need to be served with different headers const fileExtension = path.extname(filePath); From 9570335f241d88b4bb826f68867a01d9d74aa4c6 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 7 May 2021 15:18:05 +0200 Subject: [PATCH 3/6] Decode req.path before loading object from Azure Signed-off-by: Eric Peterson --- .../__mocks__/@azure/storage-blob.ts | 6 +-- .../stages/publish/azureBlobStorage.test.ts | 43 +++++++++++++++++++ .../src/stages/publish/azureBlobStorage.ts | 4 +- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index 0d480a7fa9..2fad447788 100644 --- a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts +++ b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts @@ -71,9 +71,9 @@ export class BlockBlobClient { download() { const filePath = path.join(rootDir, this.blobName); const emitter = new EventEmitter(); - process.nextTick(() => { + setTimeout(() => { if (fs.existsSync(filePath)) { - emitter.emit('data', Buffer.from(fs.readFileSync(filePath))); + emitter.emit('data', fs.readFileSync(filePath)); emitter.emit('end'); } else { emitter.emit( @@ -81,7 +81,7 @@ export class BlockBlobClient { new Error(`The file ${filePath} does not exist !`), ); } - }); + }, 0); return Promise.resolve({ readableStreamBody: emitter, }); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 1160ffe49d..797ab5435c 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -21,6 +21,8 @@ import { ENTITY_DEFAULT_NAMESPACE, } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; +import express from 'express'; +import request from 'supertest'; import mockFs from 'mock-fs'; import os from 'os'; import path from 'path'; @@ -329,4 +331,45 @@ describe('publishing with valid credentials', () => { ); }); }); + + describe('docsRouter', () => { + let app: express.Express; + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + beforeEach(() => { + app = express().use(publisher.docsRouter()); + + mockFs.restore(); + mockFs({ + [entityRootDir]: { + img: { + 'with spaces.png': 'found it', + }, + 'some folder': { + 'also with spaces.js': 'found it too', + }, + }, + }); + }); + + it('should pass expected object path to bucket', async () => { + const { + kind, + metadata: { namespace, name }, + } = entity; + + // Ensures leading slash is trimmed and encoded path is decoded. + const pngResponse = await request(app).get( + `/${namespace}/${kind}/${name}/img/with%20spaces.png`, + ); + expect(Buffer.from(pngResponse.body).toString('utf8')).toEqual( + 'found it', + ); + const jsResponse = await request(app).get( + `/${namespace}/${kind}/${name}/some%20folder/also%20with%20spaces.js`, + ); + expect(jsResponse.text).toEqual('found it too'); + }); + }); }); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index f6567abe49..60640c374d 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -254,9 +254,9 @@ export class AzureBlobStoragePublish implements PublisherBase { */ docsRouter(): express.Handler { return (req, res) => { - // Trim the leading forward slash + // Decode and trim the leading forward slash // filePath example - /default/Component/documented-component/index.html - const filePath = req.path.replace(/^\//, ''); + const filePath = decodeURI(req.path.replace(/^\//, '')); // Files with different extensions (CSS, HTML) need to be served with different headers const fileExtension = platformPath.extname(filePath); const responseHeaders = getHeadersForFileExtension(fileExtension); From 4042b6e140fe74f56e939fbfeeff534afa59d96e Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 7 May 2021 15:20:55 +0200 Subject: [PATCH 4/6] Decode req.path before loading object from OpenStack Swift Signed-off-by: Eric Peterson --- .../src/stages/publish/openStackSwift.test.ts | 43 +++++++++++++++++++ .../src/stages/publish/openStackSwift.ts | 5 +-- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts index e179aeb9a7..a75c8b75ec 100644 --- a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts +++ b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts @@ -21,6 +21,8 @@ import { ENTITY_DEFAULT_NAMESPACE, } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; +import express from 'express'; +import request from 'supertest'; import mockFs from 'mock-fs'; import os from 'os'; import path from 'path'; @@ -277,4 +279,45 @@ describe('OpenStackSwiftPublish', () => { }); }); }); + + describe('docsRouter', () => { + let app: express.Express; + const entity = createMockEntity(); + const entityRootDir = getEntityRootDir(entity); + + beforeEach(() => { + app = express().use(publisher.docsRouter()); + + mockFs.restore(); + mockFs({ + [entityRootDir]: { + img: { + 'with spaces.png': 'found it', + }, + 'some folder': { + 'also with spaces.js': 'found it too', + }, + }, + }); + }); + + it('should pass expected object path to bucket', async () => { + const { + kind, + metadata: { namespace, name }, + } = entity; + + // Ensures leading slash is trimmed and encoded path is decoded. + const pngResponse = await request(app).get( + `/${namespace}/${kind}/${name}/img/with%20spaces.png`, + ); + expect(Buffer.from(pngResponse.body).toString('utf8')).toEqual( + 'found it', + ); + const jsResponse = await request(app).get( + `/${namespace}/${kind}/${name}/some%20folder/also%20with%20spaces.js`, + ); + expect(jsResponse.text).toEqual('found it too'); + }); + }); }); diff --git a/packages/techdocs-common/src/stages/publish/openStackSwift.ts b/packages/techdocs-common/src/stages/publish/openStackSwift.ts index bff0055402..2321aa33bd 100644 --- a/packages/techdocs-common/src/stages/publish/openStackSwift.ts +++ b/packages/techdocs-common/src/stages/publish/openStackSwift.ts @@ -221,10 +221,9 @@ export class OpenStackSwiftPublish implements PublisherBase { */ docsRouter(): express.Handler { return async (req, res) => { - // Trim the leading forward slash + // Decode and trim the leading forward slash // filePath example - /default/Component/documented-component/index.html - - const filePath = req.path.replace(/^\//, ''); + const filePath = decodeURI(req.path.replace(/^\//, '')); // Files with different extensions (CSS, HTML) need to be served with different headers const fileExtension = path.extname(filePath); From e04f1ccfbebcd4636dc79b31c2fa18b62b7001bd Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 7 May 2021 16:43:28 +0200 Subject: [PATCH 5/6] Changeset Signed-off-by: Eric Peterson --- .changeset/techdocs-spaces-in-objects.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/techdocs-spaces-in-objects.md diff --git a/.changeset/techdocs-spaces-in-objects.md b/.changeset/techdocs-spaces-in-objects.md new file mode 100644 index 0000000000..7bbac5d2a8 --- /dev/null +++ b/.changeset/techdocs-spaces-in-objects.md @@ -0,0 +1,5 @@ +--- +'@backstage/techdocs-common': patch +--- + +Fixed a bug that prevented loading static assets from GCS, S3, Azure, and OpenStackSwift whose keys contain spaces or other special characters. From 0c491a1c5ed6123ecdc49f753fc6353478e8f1fb Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 10 May 2021 11:25:11 +0200 Subject: [PATCH 6/6] Clean up tests and imports Signed-off-by: Eric Peterson --- packages/techdocs-common/__mocks__/@google-cloud/storage.ts | 2 +- packages/techdocs-common/src/stages/publish/awsS3.test.ts | 4 ++++ .../src/stages/publish/azureBlobStorage.test.ts | 4 ++++ .../techdocs-common/src/stages/publish/googleStorage.test.ts | 4 ++++ .../techdocs-common/src/stages/publish/openStackSwift.test.ts | 4 ++++ 5 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts index 11fbc32de2..5e9890cd98 100644 --- a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts +++ b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Readable, Writable } from 'stream'; +import { Readable } from 'stream'; import fs from 'fs-extra'; import os from 'os'; import path from 'path'; diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index fe0fbd556a..b2954f7b2e 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -296,6 +296,10 @@ describe('AwsS3Publish', () => { }); }); + afterEach(() => { + mockFs.restore(); + }); + it('should pass expected object path to bucket', async () => { const { kind, diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 797ab5435c..568393662b 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -353,6 +353,10 @@ describe('publishing with valid credentials', () => { }); }); + afterEach(() => { + mockFs.restore(); + }); + it('should pass expected object path to bucket', async () => { const { kind, diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index 9a966e6a60..f29e1f10be 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -295,6 +295,10 @@ describe('GoogleGCSPublish', () => { }); }); + afterEach(() => { + mockFs.restore(); + }); + it('should pass expected object path to bucket', async () => { const { kind, diff --git a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts index a75c8b75ec..aeaceeaaa6 100644 --- a/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts +++ b/packages/techdocs-common/src/stages/publish/openStackSwift.test.ts @@ -301,6 +301,10 @@ describe('OpenStackSwiftPublish', () => { }); }); + afterEach(() => { + mockFs.restore(); + }); + it('should pass expected object path to bucket', async () => { const { kind,