Fix Storage Emulator crash on iOS auth error for resumable uploads (#…

…4210)

* Fix Storage Emulator crash on iOS 403

* Add CHANGELOG entry

* Refactor finalizeUpload to call Cloud Functions after auth check

* Add createMetadata method and refactor finalizeUpload to take upload, not id

CHANGELOG.md

@@ -1,5 +1,7 @@
 - Updates reserved environment variables for CF3 to include 'EVENTARC_CLOUD_EVENT_SOURCE' (#4196).
 - Fixes arg order for `firebase emulators:start --only storage` (#4195).
+- Fixes iOS auth for resumable uploads in Storage Emulator (#4184).
+- Fixes Storage Emulator crash on iOS auth error for resumable uploads (#4210).
 - Fixes bug where environment variable for gen 2 functions weren't updated on deploy (#4209).
 - Fixes an issue in the storage emulator where a file upload would trigger functions with a metadata update handler (#4213).
 - Fixes Storage Emulator rules resource evaluation (#4214).

scripts/storage-emulator-integration/tests.ts

@@ -1356,6 +1356,144 @@ describe("Storage emulator", () => {
             "X-Goog-Upload-Command": "upload, finalize",
           })
           .expect(200);
+
+        await supertest(STORAGE_EMULATOR_HOST)
+          .get(`/v0/b/${storageBucket}/o/test_upload.jpg`)
+          .set({ Authorization: "Bearer owner" })
+          .expect(200);
+      });
+
+      it("should return 403 when resumable upload is unauthenticated", async () => {
+        const uploadURL = await supertest(STORAGE_EMULATOR_HOST)
+          .post(
+            `/v0/b/${storageBucket}/o/test_upload.jpg?uploadType=resumable&name=test_upload.jpg`
+          )
+          .set({
+            // Authorization missing
+            "X-Goog-Upload-Protocol": "resumable",
+            "X-Goog-Upload-Command": "start",
+          })
+          .expect(200)
+          .then((res) => new URL(res.header["x-goog-upload-url"]));
+
+        await supertest(STORAGE_EMULATOR_HOST)
+          .put(uploadURL.pathname + uploadURL.search)
+          .set({
+            "X-Goog-Upload-Protocol": "resumable",
+            "X-Goog-Upload-Command": "upload, finalize",
+          })
+          .expect(403);
+      });
+
+      describe("cancels upload", () => {
+        it("should cancel upload successfully", async () => {
+          const uploadURL = await supertest(STORAGE_EMULATOR_HOST)
+            .post(
+              `/v0/b/${storageBucket}/o/test_upload.jpg?uploadType=resumable&name=test_upload.jpg`
+            )
+            .set({
+              Authorization: "Bearer owner",
+              "X-Goog-Upload-Protocol": "resumable",
+              "X-Goog-Upload-Command": "start",
+            })
+            .expect(200)
+            .then((res) => new URL(res.header["x-goog-upload-url"]));
+
+          await supertest(STORAGE_EMULATOR_HOST)
+            .put(uploadURL.pathname + uploadURL.search)
+            .set({
+              "X-Goog-Upload-Protocol": "resumable",
+              "X-Goog-Upload-Command": "cancel",
+            })
+            .expect(200);
+
+          await supertest(STORAGE_EMULATOR_HOST)
+            .get(`/v0/b/${storageBucket}/o/test_upload.jpg`)
+            .set({ Authorization: "Bearer owner" })
+            .expect(404);
+        });
+
+        it("should return 200 when cancelling already cancelled upload", async () => {
+          const uploadURL = await supertest(STORAGE_EMULATOR_HOST)
+            .post(
+              `/v0/b/${storageBucket}/o/test_upload.jpg?uploadType=resumable&name=test_upload.jpg`
+            )
+            .set({
+              Authorization: "Bearer owner",
+              "X-Goog-Upload-Protocol": "resumable",
+              "X-Goog-Upload-Command": "start",
+            })
+            .expect(200)
+            .then((res) => new URL(res.header["x-goog-upload-url"]));
+
+          await supertest(STORAGE_EMULATOR_HOST)
+            .put(uploadURL.pathname + uploadURL.search)
+            .set({
+              "X-Goog-Upload-Protocol": "resumable",
+              "X-Goog-Upload-Command": "cancel",
+            })
+            .expect(200);
+
+          await supertest(STORAGE_EMULATOR_HOST)
+            .put(uploadURL.pathname + uploadURL.search)
+            .set({
+              "X-Goog-Upload-Protocol": "resumable",
+              "X-Goog-Upload-Command": "cancel",
+            })
+            .expect(200);
+        });
+
+        it("should return 400 when cancelling finalized resumable upload", async () => {
+          const uploadURL = await supertest(STORAGE_EMULATOR_HOST)
+            .post(
+              `/v0/b/${storageBucket}/o/test_upload.jpg?uploadType=resumable&name=test_upload.jpg`
+            )
+            .set({
+              Authorization: "Bearer owner",
+              "X-Goog-Upload-Protocol": "resumable",
+              "X-Goog-Upload-Command": "start",
+            })
+            .expect(200)
+            .then((res) => new URL(res.header["x-goog-upload-url"]));
+
+          await supertest(STORAGE_EMULATOR_HOST)
+            .put(uploadURL.pathname + uploadURL.search)
+            .set({
+              "X-Goog-Upload-Protocol": "resumable",
+              "X-Goog-Upload-Command": "upload, finalize",
+            })
+            .expect(200);
+
+          await supertest(STORAGE_EMULATOR_HOST)
+            .put(uploadURL.pathname + uploadURL.search)
+            .set({
+              "X-Goog-Upload-Protocol": "resumable",
+              "X-Goog-Upload-Command": "cancel",
+            })
+            .expect(400);
+        });
+
+        it("should return 404 when cancelling non-existent upload", async () => {
+          const uploadURL = await supertest(STORAGE_EMULATOR_HOST)
+            .post(
+              `/v0/b/${storageBucket}/o/test_upload.jpg?uploadType=resumable&name=test_upload.jpg`
+            )
+            .set({
+              Authorization: "Bearer owner",
+              "X-Goog-Upload-Protocol": "resumable",
+              "X-Goog-Upload-Command": "start",
+            })
+            .expect(200)
+            .then((res) => new URL(res.header["x-goog-upload-url"]));
+
+          await supertest(STORAGE_EMULATOR_HOST)
+            .put(uploadURL.pathname + uploadURL.search.replace(/(upload_id=).*?(&)/, "$1foo$2"))
+            .set({
+              "X-Goog-Upload-Protocol": "resumable",
+              "X-Goog-Upload-Command": "cancel",
+            })
+            .expect(404);
+        });
       });
     });
 

src/emulator/storage/apis/firebase.ts

@@ -493,12 +493,13 @@ export function createFirebaseEndpoints(emulator: StorageEmulator): Router {
       }
 
       if (uploadCommand == "cancel") {
-        const upload = storageLayer.cancelUpload(uploadId);
-        if (!upload) {
-          res.sendStatus(400);
-          return;
+        const upload = storageLayer.queryUpload(uploadId);
+        if (upload) {
+          const cancelled = storageLayer.cancelUpload(upload);
+          res.sendStatus(cancelled ? 200 : 400);
+        } else {
+          res.sendStatus(404);
         }
-        res.sendStatus(200);
         return;
       }
 
@@ -530,14 +531,11 @@ export function createFirebaseEndpoints(emulator: StorageEmulator): Router {
       }
 
       if (uploadCommand.includes("finalize")) {
-        const finalizedUpload = storageLayer.finalizeUpload(uploadId);
-        if (!finalizedUpload) {
+        upload = storageLayer.queryUpload(uploadId);
+        if (!upload) {
           res.sendStatus(400);
           return;
         }
-        upload = finalizedUpload.upload;
-
-        res.header("x-goog-upload-status", "final");
 
         // For resumable uploads, we check auth on finalization in case of byte-dependant rules
         if (
@@ -548,7 +546,7 @@ export function createFirebaseEndpoints(emulator: StorageEmulator): Router {
             path: operationPath,
             authorization: upload.authorization,
             file: {
-              after: storageLayer.getMetadata(req.params.bucketId, name)?.asRulesResource(),
+              after: storageLayer.createMetadata(upload).asRulesResource(),
             },
           }))
         ) {
@@ -561,12 +559,15 @@ export function createFirebaseEndpoints(emulator: StorageEmulator): Router {
           });
         }
 
-        const md = finalizedUpload.file.metadata;
+        res.header("x-goog-upload-status", "final");
+        const uploadedFile = storageLayer.finalizeUpload(upload);
+
+        const md = uploadedFile.metadata;
         if (md.downloadTokens.length == 0) {
           md.addDownloadToken();
         }
 
-        res.json(new OutgoingFirebaseMetadata(finalizedUpload.file.metadata));
+        res.json(new OutgoingFirebaseMetadata(uploadedFile.metadata));
       } else if (!upload) {
         res.sendStatus(400);
         return;

src/emulator/storage/apis/gcloud.ts

@@ -123,20 +123,14 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router {
       });
     });
 
-    let upload = storageLayer.uploadBytes(uploadId, req.body);
-
+    const upload = storageLayer.uploadBytes(uploadId, req.body);
     if (!upload) {
       res.sendStatus(400);
       return;
     }
 
-    const finalizedUpload = storageLayer.finalizeUpload(uploadId);
-    if (!finalizedUpload) {
-      res.sendStatus(400);
-      return;
-    }
-    upload = finalizedUpload.upload;
-    res.status(200).json(new CloudStorageObjectMetadata(finalizedUpload.file.metadata)).send();
+    const uploadedFile = storageLayer.finalizeUpload(upload);
+    res.status(200).json(new CloudStorageObjectMetadata(uploadedFile.metadata)).send();
   });
 
   gcloudStorageAPI.post("/b/:bucketId/o/:objectId/acl", (req, res) => {

src/emulator/storage/files.ts

@@ -120,11 +120,6 @@ export enum UploadStatus {
   FINISHED,
 }
 
-export type FinalizedUpload = {
-  upload: ResumableUpload;
-  file: StoredFile;
-};
-
 export class StorageLayer {
   private _files!: Map<string, StoredFile>;
   private _uploads!: Map<string, ResumableUpload>;
@@ -173,6 +168,27 @@ export class StorageLayer {
     return;
   }
 
+  /**
+   * Generates metadata for an uploaded file. Generally, this should only be used for finalized
+   * uploads, unless needed for security rule checks.
+   * @param upload The upload corresponding to the file for which to generate metadata.
+   * @returns Metadata for uploaded file.
+   */
+  public createMetadata(upload: ResumableUpload): StoredFileMetadata {
+    const bytes = this._persistence.readBytes(upload.fileLocation, upload.currentBytesUploaded);
+    return new StoredFileMetadata(
+      {
+        name: upload.objectId,
+        bucket: upload.bucketId,
+        contentType: "",
+        contentEncoding: upload.metadata.contentEncoding,
+        customMetadata: upload.metadata.metadata,
+      },
+      this._cloudFunctions,
+      bytes
+    );
+  }
+
   public getBytes(
     bucket: string,
     object: string,
@@ -216,13 +232,18 @@ export class StorageLayer {
     return this._uploads.get(uploadId);
   }
 
-  public cancelUpload(uploadId: string): ResumableUpload | undefined {
-    const upload = this._uploads.get(uploadId);
-    if (!upload) {
-      return undefined;
+  /**
+   * Deletes partially uploaded file from persistence layer and updates its status. Cancelling
+   * an upload is idempotent.
+   * @param upload The upload to be cancelled.
+   * @returns Whether the upload was cancelled (i.e. its initial status was ACTIVE or CANCELLED).
+   */
+  public cancelUpload(upload: ResumableUpload): boolean {
+    if (upload.status === UploadStatus.ACTIVE) {
+      this._persistence.deleteFile(upload.fileLocation);
+      upload.status = UploadStatus.CANCELLED;
     }
-    upload.status = UploadStatus.CANCELLED;
-    this._persistence.deleteFile(upload.fileLocation);
+    return upload.status === UploadStatus.CANCELLED;
   }
 
   public uploadBytes(uploadId: string, bytes: Buffer): ResumableUpload | undefined {
@@ -266,36 +287,26 @@ export class StorageLayer {
     return this._persistence.deleteAll();
   }
 
-  public finalizeUpload(uploadId: string): FinalizedUpload | undefined {
-    const upload = this._uploads.get(uploadId);
-
-    if (!upload) {
-      return undefined;
-    }
-
+  /**
+   * Stores the uploaded file with generated metadata and triggers Object Finalize Cloud Functions.
+   * @param upload The upload to finalize.
+   * @returns The stored file.
+   */
+  public finalizeUpload(upload: ResumableUpload): StoredFile {
     upload.status = UploadStatus.FINISHED;
+
+    const metadata = this.createMetadata(upload);
     const filePath = this.path(upload.bucketId, upload.objectId);
+    const file = new StoredFile(metadata, filePath);
 
-    const bytes = this._persistence.readBytes(upload.fileLocation, upload.currentBytesUploaded);
-    const finalMetadata = new StoredFileMetadata(
-      {
-        name: upload.objectId,
-        bucket: upload.bucketId,
-        contentType: "",
-        contentEncoding: upload.metadata.contentEncoding,
-        customMetadata: upload.metadata.metadata,
-      },
-      this._cloudFunctions,
-      bytes
-    );
-    const file = new StoredFile(finalMetadata, filePath);
     this._files.set(filePath, file);
 
-    this._persistence.deleteFile(filePath, true);
+    this._persistence.deleteFile(filePath, /* failSilently = */ true);
     this._persistence.renameFile(upload.fileLocation, filePath);
 
     this._cloudFunctions.dispatch("finalize", new CloudStorageObjectMetadata(file.metadata));
-    return { upload: upload, file: file };
+
+    return file;
   }
 
   public oneShotUpload(

src/test/emulators/storage/files.spec.ts

@@ -1,6 +1,7 @@
 import { expect } from "chai";
 import { StoredFileMetadata } from "../../../emulator/storage/metadata";
 import { StorageCloudFunctions } from "../../../emulator/storage/cloudFunctions";
+import { StorageLayer } from "../../../emulator/storage/files";
 
 describe("files", () => {
   it("can serialize and deserialize metadata", () => {
@@ -23,4 +24,30 @@ describe("files", () => {
     const deserialized = StoredFileMetadata.fromJSON(json, cf);
     expect(deserialized).to.deep.equal(metadata);
   });
+
+  it("should store file in memory when upload is finalized", () => {
+    const storageLayer = new StorageLayer("project");
+    const bytesToWrite = "Hello, World!";
+
+    const upload = storageLayer.startUpload("bucket", "object", "mime/type", {
+      contentType: "mime/type",
+    });
+    storageLayer.uploadBytes(upload.uploadId, Buffer.from(bytesToWrite));
+    storageLayer.finalizeUpload(upload);
+
+    expect(storageLayer.getBytes("bucket", "object")?.includes(bytesToWrite));
+    expect(storageLayer.getMetadata("bucket", "object")?.size).equals(bytesToWrite.length);
+  });
+
+  it("should delete file from persistence layer when upload is cancelled", () => {
+    const storageLayer = new StorageLayer("project");
+
+    const upload = storageLayer.startUpload("bucket", "object", "mime/type", {
+      contentType: "mime/type",
+    });
+    storageLayer.uploadBytes(upload.uploadId, Buffer.alloc(0));
+    storageLayer.cancelUpload(upload);
+
+    expect(storageLayer.getMetadata("bucket", "object")).to.equal(undefined);
+  });
 });