Adds support for downloading attachments as a .tar (#1495)

Currently only Zip is supported for downloading attachments. In the
future, we want to be able to support re-uploading attachments. Zip will
make this difficult, as the structure of the zip file is stored at the
end of file, requiring the entire file to be uploaded before it can be
parsed.

This commit adds downloading attachments as a .tar, which uses inline
headers for each file it contains. This means it can be easily parsed
and processed through node's streams, minimizing server disk and memory
usage.

## Related issues

#1021

Author: Spoffy

app/gen-server/lib/DocApiForwarder.ts

@@ -59,6 +59,11 @@ export class DocApiForwarder {
     app.use('/api/docs/:docId/create-fork', withDoc);
     app.use('/api/docs/:docId/apply', withDoc);
     app.use('/api/docs/:docId/attachments', withDoc);
+    app.use('/api/docs/:docId/attachments/download', withDoc);
+    app.use('/api/docs/:docId/attachments/transferStatus', withDoc);
+    app.use('/api/docs/:docId/attachments/transferAll', withDoc);
+    app.use('/api/docs/:docId/attachments/store', withDoc);
+    app.use('/api/docs/:docId/attachments/stores', withDoc);
     app.use('/api/docs/:docId/snapshots', withDoc);
     app.use('/api/docs/:docId/usersForViewAs', withDoc);
     app.use('/api/docs/:docId/replace', withDoc);
@@ -74,10 +79,6 @@ export class DocApiForwarder {
     app.use('/api/docs/:docId/timing/start', withDoc);
     app.use('/api/docs/:docId/timing/stop', withDoc);
     app.use('/api/docs/:docId/forms/:vsId', withDoc);
-    app.use('/api/docs/:docId/attachments/transferStatus', withDoc);
-    app.use('/api/docs/:docId/attachments/transferAll', withDoc);
-    app.use('/api/docs/:docId/attachments/store', withDoc);
-    app.use('/api/docs/:docId/attachments/stores', withDoc);
 
     app.use('^/api/docs$', withoutDoc);
   }

app/server/lib/ActiveDoc.ts

@@ -90,7 +90,12 @@ import {Share} from 'app/gen-server/entity/Share';
 import {RecordWithStringId} from 'app/plugin/DocApiTypes';
 import {ParseFileResult, ParseOptions} from 'app/plugin/FileParserAPI';
 import {AccessTokenOptions, AccessTokenResult, GristDocAPI, UIRowId} from 'app/plugin/GristAPI';
-import {Archive, ArchiveEntry, create_zip_archive} from 'app/server/lib/Archive';
+import {
+  Archive,
+  ArchiveEntry, CreatableArchiveFormats,
+  create_tar_archive,
+  create_zip_archive
+} from 'app/server/lib/Archive';
 import {AssistanceSchemaPromptV1Context} from 'app/server/lib/Assistance';
 import {AssistanceContext} from 'app/common/AssistancePrompts';
 import {AuditEventAction} from 'app/server/lib/AuditEvent';
@@ -960,7 +965,8 @@ export class ActiveDoc extends EventEmitter {
     return data;
   }
 
-  public async getAttachmentsArchive(docSession: OptDocSession): Promise<Archive> {
+  public async getAttachmentsArchive(docSession: OptDocSession,
+                                     format: CreatableArchiveFormats = 'zip'): Promise<Archive> {
     if (
       !await this._granularAccess.canReadEverything(docSession) &&
       !await this.canDownload(docSession)
@@ -990,12 +996,20 @@ export class ActiveDoc extends EventEmitter {
         filesAdded.add(name);
         yield({
           name,
+          size: file.metadata.size,
           data: file.contentStream,
         });
       }
     }
 
-    return create_zip_archive({ store: true }, fileGenerator());
+    if (format == 'tar') {
+      return create_tar_archive(fileGenerator());
+    }
+    if (format == 'zip') {
+      return create_zip_archive({ store: true }, fileGenerator());
+    }
+    // Generally this won't happen, as long as the above is exhaustive over the type of `format`
+    throw new ApiError(`Unsupported archive format ${format}`, 400);
   }
 
   @ActiveDoc.keepDocOpen

app/server/lib/Archive.ts

@@ -1,17 +1,25 @@
+import {StringUnion} from 'app/common/StringUnion';
 import {ZipArchiveEntry} from 'compress-commons';
 import stream from 'node:stream';
+import * as tar from 'tar-stream';
 import ZipStream, {ZipStreamOptions} from 'zip-stream';
 
 export interface ArchiveEntry {
   name: string;
+  size: number;
   data: stream.Readable | Buffer;
 }
 
 export interface Archive {
+  mimeType: string;
+  fileExtension: string;
   dataStream: stream.Readable;
   completed: Promise<void>;
 }
 
+export const CreatableArchiveFormats = StringUnion('zip', 'tar');
+export type CreatableArchiveFormats = typeof CreatableArchiveFormats.type;
+
 /**
  *
  * Creates a streamable zip archive, reading files on-demand from the entries iterator.
@@ -27,6 +35,8 @@ export async function create_zip_archive(
   const archive = new ZipStream(options);
 
   return {
+    mimeType: "application/zip",
+    fileExtension: "zip",
     dataStream: archive,
     // Caller is responsible for error handling/awaiting on this promise.
     completed: (async () => {
@@ -57,3 +67,39 @@ function addEntryToZipArchive(archive: ZipStream, file: ArchiveEntry): Promise<Z
     });
   });
 }
+
+/**
+ *
+ * Creates a streamable tar archive, reading files on-demand from the entries iterator.
+ * Entries are provided as an async iterable, to ensure the archive is constructed
+ * correctly. A generator can be used for convenience.
+ * @param {AsyncIterable<ArchiveEntry>} entries - Entries to add.
+ * @returns {Archive}
+ */
+export async function create_tar_archive(
+  entries: AsyncIterable<ArchiveEntry>
+): Promise<Archive> {
+  const archive = tar.pack();
+
+  return {
+    mimeType: "application/x-tar",
+    fileExtension: "tar",
+    dataStream: archive,
+    // Caller is responsible for error handling/awaiting on this promise.
+    completed: (async () => {
+      try {
+        for await (const entry of entries) {
+          const entryStream = archive.entry({ name: entry.name, size: entry.size });
+          await stream.promises.pipeline(entry.data, entryStream);
+        }
+        archive.finalize();
+      } catch (error) {
+        archive.destroy(error);
+      } finally {
+        // If the stream was destroyed with an error, this will re-throw the error we caught above.
+        // Without this, node will see the stream as having an uncaught error, and complain.
+        await stream.promises.finished(archive);
+      }
+    })()
+  };
+}

app/server/lib/DocApi.ts

@@ -45,6 +45,7 @@ import {
 } from 'app/plugin/TableOperationsImpl';
 import {ActiveDoc, colIdToRef as colIdToReference, getRealTableId, tableIdToRef} from "app/server/lib/ActiveDoc";
 import {appSettings} from "app/server/lib/AppSettings";
+import {CreatableArchiveFormats} from 'app/server/lib/Archive';
 import {sendForCompletion} from 'app/server/lib/Assistance';
 import {getDocPoolIdFromDocInfo} from 'app/server/lib/AttachmentStore';
 import {
@@ -575,12 +576,19 @@ export class DocWorkerApi {
 
     // Responds with an archive of all attachment contents, with suitable Content-Type and Content-Disposition.
     this._app.get('/api/docs/:docId/attachments/download', canView, withDoc(async (activeDoc, req, res) => {
-      const archive = await activeDoc.getAttachmentsArchive(docSessionFromRequest(req));
+      const archiveFormatStr = optStringParam(req.query.format, 'format', {
+        allowed: CreatableArchiveFormats.values,
+        allowEmpty: true,
+      });
+
+      const archiveFormat = CreatableArchiveFormats.parse(archiveFormatStr) || 'zip';
+      const archive = await activeDoc.getAttachmentsArchive(docSessionFromRequest(req), archiveFormat);
       const docName = await this._getDownloadFilename(req, "Attachments", activeDoc.doc);
       res.status(200)
-        .type("application/zip")
+        .type(archive.mimeType)
         // Construct a content-disposition header of the form 'attachment; filename="NAME"'
-        .set('Content-Disposition', contentDisposition(`${docName}.zip`, {type: 'attachment'}))
+        .set('Content-Disposition',
+          contentDisposition(`${docName}.${archive.fileExtension}`, {type: 'attachment'}))
         // Avoid storing because this could be huge.
         .set('Cache-Control', 'no-store');
 

package.json

@@ -128,6 +128,7 @@
     "@gristlabs/pidusage": "2.0.17",
     "@gristlabs/sqlite3": "5.1.4-grist.8",
     "@popperjs/core": "2.11.8",
+    "@types/tar-stream": "^3.1.3",
     "@types/zip-stream": "^7.0.0",
     "accept-language-parser": "1.5.0",
     "ace-builds": "1.23.3",
@@ -205,6 +206,7 @@
     "short-uuid": "5.2.0",
     "slugify": "1.6.6",
     "swagger-ui-dist": "5.11.0",
+    "tar-stream": "^3.1.7",
     "tmp": "0.0.33",
     "tmp-promise": "1.0.5",
     "ts-interface-checker": "1.0.2",

test/server/lib/ActiveDoc.ts

@@ -6,6 +6,7 @@ import * as gristTypes from 'app/common/gristTypes';
 import {GristObjCode} from 'app/plugin/GristData';
 import {TableData} from 'app/common/TableData';
 import {ActiveDoc} from 'app/server/lib/ActiveDoc';
+import {CreatableArchiveFormats} from 'app/server/lib/Archive';
 import {AttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider';
 import {DummyAuthorizer} from 'app/server/lib/Authorizer';
 import {Client} from 'app/server/lib/Client';
@@ -1193,15 +1194,19 @@ describe('ActiveDoc', async function() {
       await doc.addAttachments(fakeTransferSession, uploadId);
     }
 
-    async function assertArchiveContents(archive: string | Buffer, expectedFiles: { name: string; contents?: string }[])
-    {
+    async function assertArchiveContents(
+      archive: string | Buffer,
+      archiveType: string,
+      expectedFiles: { name: string; contents?: string }[],
+    ) {
       const getFileName = (filePath: string) => filePath.substring(filePath.indexOf("_") + 1);
       const files = await decompress(archive);
       for (const expectedFile of expectedFiles) {
         const file = files.find((file) => getFileName(file.path) === expectedFile.name);
         assert(file, "file not found in archive");
         if (expectedFile.contents) {
-          assert.equal(file?.data.toString(), expectedFile.contents, "file contents in archive don't match");
+          assert.equal(
+            file?.data.toString(), expectedFile.contents, `file contents in ${archiveType} archive don't match`);
         }
       }
     }
@@ -1212,11 +1217,14 @@ describe('ActiveDoc', async function() {
 
       await uploadAttachments(activeDoc1, testAttachments);
 
-      const archive = await activeDoc1.getAttachmentsArchive(fakeTransferSession);
-      const archiveMemoryStream = new MemoryWritableStream();
-      await stream.promises.pipeline(archive.dataStream, archiveMemoryStream);
+      for (const archiveType of CreatableArchiveFormats.values) {
+        const archive = await activeDoc1.getAttachmentsArchive(fakeTransferSession, archiveType);
+        const archiveMemoryStream = new MemoryWritableStream();
+        await stream.promises.pipeline(archive.dataStream, archiveMemoryStream);
+
+        await assertArchiveContents(archiveMemoryStream.getBuffer(), archiveType, testAttachments);
+      }
 
-      await assertArchiveContents(archiveMemoryStream.getBuffer(), testAttachments);
     });
 
     /*

test/server/lib/Archive.ts

@@ -1,4 +1,9 @@
-import {Archive, create_zip_archive} from 'app/server/lib/Archive';
+import {
+  Archive,
+  ArchiveEntry,
+  create_tar_archive,
+  create_zip_archive
+} from 'app/server/lib/Archive';
 import {MemoryWritableStream} from 'app/server/utils/MemoryWritableStream';
 import decompress from 'decompress';
 import {assert} from 'chai';
@@ -16,24 +21,24 @@ const testFiles = [
 ];
 
 async function* generateTestArchiveContents() {
-  yield {
-    name: testFiles[0].name,
-    data: stream.Readable.from(Buffer.from(testFiles[0].contents))
-  };
-  yield {
-    name: testFiles[1].name,
-    data: Buffer.from(testFiles[1].contents)
-  };
+  for (const file of testFiles) {
+    const buffer = Buffer.from(file.contents);
+    yield {
+      name: file.name,
+      size: buffer.length,
+      data: stream.Readable.from(buffer),
+    };
+  }
 }
 
 async function assertArchiveCompletedSuccessfully(archive: Archive) {
   await assert.isFulfilled(archive.completed);
-  assert.isTrue(archive.dataStream.closed, "archive data stream was not closed");
+  assert.isTrue(archive.dataStream.destroyed, "archive data stream was not finished");
 }
 
 async function assertArchiveFailedWithError(archive: Archive) {
   await assert.isRejected(archive.completed);
-  assert.isTrue(archive.dataStream.closed, "archive data stream was not closed");
+  assert.isTrue(archive.dataStream.destroyed, "archive data stream was not finished");
 }
 
 async function assertArchiveContainsTestFiles(archiveData: Buffer) {
@@ -46,30 +51,41 @@ async function assertArchiveContainsTestFiles(archiveData: Buffer) {
     assert.deepEqual(zippedFile?.data.toString(), testFile.contents);
   }
 }
-describe('Archive', function () {
-  describe('create_zip_archive', function () {
-    it('should create a zip archive', async function () {
-      const archive = await create_zip_archive({store: true}, generateTestArchiveContents());
-      const output = new MemoryWritableStream();
-      archive.dataStream.pipe(output);
-      await assertArchiveCompletedSuccessfully(archive);
 
-      const archiveData = output.getBuffer();
+type ArchiveCreator = (entries: AsyncIterable<ArchiveEntry>) => Promise<Archive>;
 
-      await assertArchiveContainsTestFiles(archiveData);
-    });
+function testArchive(type: string, makeArchive: ArchiveCreator) {
+  it(`should create a ${type} archive`, async function () {
+    const archive = await makeArchive(generateTestArchiveContents());
+    const output = new MemoryWritableStream();
+    archive.dataStream.pipe(output);
+    await assertArchiveCompletedSuccessfully(archive);
 
-    it('errors in archive.completed if the generator errors', async function () {
-      async function* throwErrorGenerator() {
-        throw new Error("Test error");
-        yield {name: testFiles[0].name, data: Buffer.from(testFiles[0].contents)};
-      }
+    const archiveData = output.getBuffer();
 
+    await assertArchiveContainsTestFiles(archiveData);
+  });
+
+  it('errors in archive.completed if the generator errors', async function () {
+    async function* throwErrorGenerator() {
+      throw new Error("Test error");
+      yield {name: 'Test', size: 0, data: Buffer.from([])};
+    }
 
-      // Shouldn't error here - as this just starts the packing.
-      const archive = await create_zip_archive({store: true}, throwErrorGenerator());
-      await assert.isRejected(archive.completed, "Test error");
-      await assertArchiveFailedWithError(archive);
-    });
+    // Shouldn't error here - as this just starts the packing.
+    const archive = await makeArchive(throwErrorGenerator());
+    await assert.isRejected(archive.completed, "Test error");
+    await assertArchiveFailedWithError(archive);
+  });
+}
+
+describe('Archive', function () {
+  describe('create_zip_archive', function () {
+    const creator: ArchiveCreator = (entries) => create_zip_archive({store: true}, entries);
+    testArchive('zip', creator);
+  });
+  describe('create_tar_archive', function () {
+    const creator: ArchiveCreator = (entries) => create_tar_archive(entries);
+    testArchive('tar', creator);
   });
 });