From d552ba727f62184b39a5d774f801f6923a7f0903 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 20 Jan 2021 17:17:51 +0100 Subject: [PATCH 1/5] backend: (GitHub) URL Reader should get filename from header instead of guessing --- .../src/reading/GithubUrlReader.test.ts | 12 +++++++ .../src/reading/GithubUrlReader.ts | 31 +++++++++++++------ 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 0b7d56480f..080e8b1d5b 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -165,6 +165,10 @@ describe('GithubUrlReader', () => { res( ctx.status(200), ctx.set('Content-Type', 'application/x-gzip'), + ctx.set( + 'content-disposition', + 'attachment; filename=backstage-mock-etag123.tar.gz', + ), ctx.body(repoBuffer), ), ), @@ -178,6 +182,10 @@ describe('GithubUrlReader', () => { res( ctx.status(200), ctx.set('Content-Type', 'application/x-gzip'), + ctx.set( + 'content-disposition', + 'attachment; filename=backstage-mock-etag123.tar.gz', + ), ctx.body(repoBuffer), ), ), @@ -244,6 +252,10 @@ describe('GithubUrlReader', () => { return res( ctx.status(200), ctx.set('Content-Type', 'application/x-gzip'), + ctx.set( + 'content-disposition', + 'attachment; filename=backstage-mock-etag123.tar.gz', + ), ctx.body(repoBuffer), ); }, diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index b5b4e9c1a7..6c7cefe2ef 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -166,18 +166,31 @@ export class GithubUrlReader implements UrlReader { throw new Error(message); } - // Note that repoResponseJson.full_name must be used over full_name because the path - // is case sensitive and full_name may not be inq the correct case. - // TODO(OrkoHunter): The directory name inside the tarball should be retrieved from the tar - // instead of being constructed here. Same goes for GitLab, Bitbucket and Azure. - const extractedDirName = `${repoResponseJson.full_name.replace( - '/', - '-', - )}-${commitSha.substr(0, 7)}`; + // Get the filename of archive from the header of the response + const contentDispositionHeader = archive.headers.get( + 'content-disposition', + ) as string; + if (!contentDispositionHeader) { + throw new Error( + `Failed to read tree from ${url}. ` + + 'GitHub API response for downloading archive does not contain content-disposition header ', + ); + } + const fileNameRegEx = new RegExp( + /^attachment; filename=(?.*).tar.gz$/, + ); + const archiveFileName = contentDispositionHeader.match(fileNameRegEx) + ?.groups?.fileName; + if (!archiveFileName) { + throw new Error( + `Failed to read tree from ${url}. GitHub API response for downloading archive has an unexpected ` + + `format of content-disposition header ${contentDispositionHeader} `, + ); + } // The path includes the name of the directory inside the tarball and a sub path // if requested in readTree. - const path = `${extractedDirName}/${filepath}`; + const path = `${archiveFileName}/${filepath}`; return await this.deps.treeResponseFactory.fromTarArchive({ // TODO(Rugvip): Underlying implementation of fetch will be node-fetch, we probably want From 4c8b47c5cc8381c39ee7e0de9b40291734b470e3 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 20 Jan 2021 17:43:04 +0100 Subject: [PATCH 2/5] backend: (GitLab) URL Reader uses content-disposition header for filename --- .../src/reading/GitlabUrlReader.test.ts | 12 +++++++++ .../src/reading/GitlabUrlReader.ts | 26 ++++++++++++++++--- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 2e66794397..c0736f769d 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -176,6 +176,10 @@ describe('GitlabUrlReader', () => { res( ctx.status(200), ctx.set('Content-Type', 'application/zip'), + ctx.set( + 'content-disposition', + 'attachment; filename="mock-main-sha123abc.zip"', + ), ctx.body(archiveBuffer), ), ), @@ -225,6 +229,10 @@ describe('GitlabUrlReader', () => { res( ctx.status(200), ctx.set('Content-Type', 'application/zip'), + ctx.set( + 'content-disposition', + 'attachment; filename="mock-main-sha123abc.zip"', + ), ctx.body(archiveBuffer), ), ), @@ -254,6 +262,10 @@ describe('GitlabUrlReader', () => { res( ctx.status(200), ctx.set('Content-Type', 'application/zip'), + ctx.set( + 'content-disposition', + 'attachment; filename="mock-main-sha123abc.zip"', + ), ctx.body(archiveBuffer), ), ), diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index 72db901ac1..24fc640784 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -140,9 +140,29 @@ export class GitlabUrlReader implements UrlReader { throw new Error(message); } - const path = filepath - ? `${repoName}-${branch}-${commitSha}/${filepath}/` - : ''; + // Get the filename of archive from the header of the response + const contentDispositionHeader = archiveGitLabResponse.headers.get( + 'content-disposition', + ) as string; + if (!contentDispositionHeader) { + throw new Error( + `Failed to read tree from ${url}. ` + + 'GitLab API response for downloading archive does not contain content-disposition header ', + ); + } + const fileNameRegEx = new RegExp( + /^attachment; filename="(?.*).zip"$/, + ); + const archiveFileName = contentDispositionHeader.match(fileNameRegEx) + ?.groups?.fileName; + if (!archiveFileName) { + throw new Error( + `Failed to read tree from ${url}. GitLab API response for downloading archive has an unexpected ` + + `format of content-disposition header ${contentDispositionHeader} `, + ); + } + + const path = filepath ? `${archiveFileName}/${filepath}/` : ''; return await this.treeResponseFactory.fromZipArchive({ stream: (archiveGitLabResponse.body as unknown) as Readable, From 40f8847c802e91b873b89466fccfe26f4d140269 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 20 Jan 2021 17:56:11 +0100 Subject: [PATCH 3/5] backend: (Bitbucket) URL Reader uses content-disposition header for filename --- .../src/reading/BitbucketUrlReader.test.ts | 8 ++++++ .../src/reading/BitbucketUrlReader.ts | 25 ++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index 3542e822e1..9661368b5e 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -95,6 +95,10 @@ describe('BitbucketUrlReader', () => { res( ctx.status(200), ctx.set('Content-Type', 'application/zip'), + ctx.set( + 'content-disposition', + 'attachment; filename=backstage-mock-12ab34cd56ef.zip', + ), ctx.body(repoBuffer), ), ), @@ -114,6 +118,10 @@ describe('BitbucketUrlReader', () => { res( ctx.status(200), ctx.set('Content-Type', 'application/zip'), + ctx.set( + 'content-disposition', + 'attachment; filename=backstage-mock.zip', + ), ctx.body(privateBitbucketRepoBuffer), ), ), diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index 869870f248..5b9dc7f333 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -125,14 +125,31 @@ export class BitbucketUrlReader implements UrlReader { throw new Error(message); } - let folderPath = `${project}-${repoName}`; - if (isHosted) { - folderPath = `${project}-${repoName}-${lastCommitShortHash}`; + // Get the filename of archive from the header of the response + const contentDispositionHeader = archiveBitbucketResponse.headers.get( + 'content-disposition', + ) as string; + if (!contentDispositionHeader) { + throw new Error( + `Failed to read tree from ${url}. ` + + 'Bitbucket API response for downloading archive does not contain content-disposition header ', + ); + } + const fileNameRegEx = new RegExp( + /^attachment; filename=(?.*).zip$/, + ); + const archiveFileName = contentDispositionHeader.match(fileNameRegEx) + ?.groups?.fileName; + if (!archiveFileName) { + throw new Error( + `Failed to read tree from ${url}. Bitbucket API response for downloading archive has an unexpected ` + + `format of content-disposition header ${contentDispositionHeader} `, + ); } return await this.treeResponseFactory.fromZipArchive({ stream: (archiveBitbucketResponse.body as unknown) as Readable, - path: `${folderPath}/${filepath}`, + path: `${archiveFileName}/${filepath}`, etag: lastCommitShortHash, filter: options?.filter, }); From 3c2ec77f8b747c303c882ee39371ebea705c553f Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 20 Jan 2021 18:04:42 +0100 Subject: [PATCH 4/5] backend: Azure URL Reader does not yet support file path based readTree --- packages/backend-common/src/reading/AzureUrlReader.ts | 2 ++ packages/backend-common/src/reading/BitbucketUrlReader.ts | 6 +----- packages/backend-common/src/reading/GitlabUrlReader.ts | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts index 59e437607f..578db2ac92 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.ts @@ -76,6 +76,8 @@ export class AzureUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { + // TODO: Support filepath based reading tree feature like other providers + // Get latest commit SHA const commitsAzureResponse = await fetch( diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index 5b9dc7f333..e9727e04cf 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -101,17 +101,13 @@ export class BitbucketUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { - const { name: repoName, owner: project, resource, filepath } = parseGitUrl( - url, - ); + const { filepath } = parseGitUrl(url); const lastCommitShortHash = await this.getLastCommitShortHash(url); if (options?.etag && options.etag === lastCommitShortHash) { throw new NotModifiedError(); } - const isHosted = resource === 'bitbucket.org'; - const downloadUrl = await getBitbucketDownloadUrl(url, this.config); const archiveBitbucketResponse = await fetch( downloadUrl, diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index 24fc640784..654f4f9a85 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -78,7 +78,7 @@ export class GitlabUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { - const { name: repoName, ref, full_name, filepath } = parseGitUrl(url); + const { ref, full_name, filepath } = parseGitUrl(url); // Use GitLab API to get the default branch // encodeURIComponent is required for GitLab API From 0ea03276349b0d6caf97b2cc7f90647b150e6352 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 20 Jan 2021 18:08:10 +0100 Subject: [PATCH 5/5] Add changeset for URL Reader readTree fix of getting archive filename from API response header --- .changeset/purple-olives-destroy.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/purple-olives-destroy.md diff --git a/.changeset/purple-olives-destroy.md b/.changeset/purple-olives-destroy.md new file mode 100644 index 0000000000..3578f5fdf5 --- /dev/null +++ b/.changeset/purple-olives-destroy.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +URL Reader: Use API response headers for archive filename in readTree. Fixes bug for users with hosted Bitbucket.