paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 47 Packages Projects Releases Wiki Activity

Merge pull request #4169 from backstage/orkohunter/url-reader-no-guessing-archve-name

Browse Source 
fix(backend): URL Reader should not hardcode the archive filename, use response header instead
This commit is contained in:
Patrik Oldsberg
2021-01-20 19:48:02 +01:00
committed by GitHub
parent 08f3d53434 0ea0327634
commit 443416b875
8 changed files with 107 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/purple-olives-destroy.md
+5
View File
@@ -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.
packages/backend-common/src/reading/AzureUrlReader.ts
+2
View File
@@ -76,6 +76,8 @@ export class AzureUrlReader implements UrlReader {
url: string,
options?: ReadTreeOptions,
): Promise<ReadTreeResponse> {
// TODO: Support filepath based reading tree feature like other providers
// Get latest commit SHA
const commitsAzureResponse = await fetch(
packages/backend-common/src/reading/BitbucketUrlReader.test.ts
+8
View File
@@ -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),
),
),
packages/backend-common/src/reading/BitbucketUrlReader.ts
+22 -9
View File
@@ -101,17 +101,13 @@ export class BitbucketUrlReader implements UrlReader {
url: string,
options?: ReadTreeOptions,
): Promise<ReadTreeResponse> {
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,
@@ -125,14 +121,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=(?<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,
});
packages/backend-common/src/reading/GithubUrlReader.test.ts
+12
View File
@@ -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),
);
},
packages/backend-common/src/reading/GithubUrlReader.ts
+22 -9
View File
@@ -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=(?<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
packages/backend-common/src/reading/GitlabUrlReader.test.ts
+12
View File
@@ -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),
),
),
packages/backend-common/src/reading/GitlabUrlReader.ts
+24 -4
View File
@@ -78,7 +78,7 @@ export class GitlabUrlReader implements UrlReader {
url: string,
options?: ReadTreeOptions,
): Promise<ReadTreeResponse> {
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
@@ -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="(?<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,