From d552ba727f62184b39a5d774f801f6923a7f0903 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Wed, 20 Jan 2021 17:17:51 +0100
Subject: [PATCH] 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=(?<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