From 0209ee48c2239ef34ea533d0a25da542d0788d4f Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Thu, 4 Aug 2022 14:46:48 -0400 Subject: [PATCH 1/6] read name from metadata as backup for gcs upload --- scripts/storage-emulator-integration/tests.ts | 40 +++++++++++++++++++ src/emulator/storage/apis/gcloud.ts | 28 ++++++++----- src/emulator/storage/metadata.ts | 1 + 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/scripts/storage-emulator-integration/tests.ts b/scripts/storage-emulator-integration/tests.ts index f4ccffd0298..9b40f7a2bea 100644 --- a/scripts/storage-emulator-integration/tests.ts +++ b/scripts/storage-emulator-integration/tests.ts @@ -246,6 +246,46 @@ describe("Storage emulator", () => { expect(fileMetadata).to.deep.include(metadata); }); + it("should handle resumable upload with name only in metadata", async () => { + const fileName = "test_upload.jpg"; + const uploadURL = await supertest(STORAGE_EMULATOR_HOST) + .post(`/upload/storage/v1/b/${storageBucket}/o?uploadType=resumable`) + .send({ name: fileName }) + .set({ + Authorization: "Bearer owner", + }) + .expect(200) + .then((res) => new URL(res.header["location"])); + expect(uploadURL.searchParams?.get("name")).to.equal(fileName); + }); + + it.only("should handle multipart upload with name only in metadata", async () => { + const contentTypeHeader = + "multipart/related; boundary=b1d5b2e3-1845-4338-9400-6ac07ce53c1e"; + const body = Buffer.from(`--b1d5b2e3-1845-4338-9400-6ac07ce53c1e\r +content-type: application/json\r +\r +{"name":"test_upload.jpg"}\r +--b1d5b2e3-1845-4338-9400-6ac07ce53c1e\r +content-type: text/plain\r +\r +hello there! +\r +--b1d5b2e3-1845-4338-9400-6ac07ce53c1e--\r +`); + const fileName = "test_upload.jpg"; + const responseName = await supertest(STORAGE_EMULATOR_HOST) + .post(`/upload/storage/v1/b/${storageBucket}/o?uploadType=multipart`) + .send(body) + .set({ + Authorization: "Bearer owner", + "content-type": contentTypeHeader, + }) + .expect(200) + .then((res) => res.body.name); + expect(responseName).to.equal(fileName); + }); + it("should return an error message when uploading a file with invalid metadata", async () => { const fileName = "test_upload.jpg"; const errorMessage = await supertest(STORAGE_EMULATOR_HOST) diff --git a/src/emulator/storage/apis/gcloud.ts b/src/emulator/storage/apis/gcloud.ts index 0da8c1f675e..971d8c69826 100644 --- a/src/emulator/storage/apis/gcloud.ts +++ b/src/emulator/storage/apis/gcloud.ts @@ -17,6 +17,7 @@ import { parseObjectUploadMultipartRequest } from "../multipart"; import { Upload, UploadNotActiveError } from "../upload"; import { ForbiddenError, NotFoundError } from "../errors"; import { reqBodyToBuffer } from "../../shared/request"; +import { Query } from "express-serve-static-core"; export function createCloudEndpoints(emulator: StorageEmulator): Router { // eslint-disable-next-line new-cap @@ -199,18 +200,7 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { }); gcloudStorageAPI.post("/upload/storage/v1/b/:bucketId/o", async (req, res) => { - if (!req.query.name) { - res.sendStatus(400); - return; - } - let name = req.query.name.toString(); - - if (name.startsWith("/")) { - name = name.slice(1); - } - const contentTypeHeader = req.header("content-type") || req.header("x-upload-content-type"); - if (!contentTypeHeader) { return res.sendStatus(400); } @@ -219,6 +209,11 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { if (emulatorInfo === undefined) { return res.sendStatus(500); } + const name = getValidName(req.query, req.body); + if (name == null) { + res.sendStatus(400); + return; + } const upload = uploadService.startResumableUpload({ bucketId: req.params.bucketId, objectId: name, @@ -237,11 +232,17 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { // Multipart upload let metadataRaw: string; let dataRaw: Buffer; + let name: string | undefined; try { ({ metadataRaw, dataRaw } = parseObjectUploadMultipartRequest( contentTypeHeader!, await reqBodyToBuffer(req) )); + name = getValidName(req.query, JSON.parse(metadataRaw)); + if (name == null) { + res.sendStatus(400); + return; + } } catch (err) { if (err instanceof Error) { return res.status(400).json({ @@ -411,3 +412,8 @@ function sendObjectNotFound(req: Request, res: Response): void { }); } } + +function getValidName(query: Query, metadata: IncomingMetadata): string | undefined { + const name = query?.name?.toString() || metadata?.name; + return name?.startsWith("/") ? name.slice(1) : name; +} diff --git a/src/emulator/storage/metadata.ts b/src/emulator/storage/metadata.ts index 6b58b72c7be..ef40ea6bc67 100644 --- a/src/emulator/storage/metadata.ts +++ b/src/emulator/storage/metadata.ts @@ -287,6 +287,7 @@ export interface RulesResourceMetadata { } export interface IncomingMetadata { + name?: string; contentType?: string; contentLanguage?: string; contentEncoding?: string; From 19aefed990a1709d5649f786a93878d10368c811 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Thu, 4 Aug 2022 15:12:35 -0400 Subject: [PATCH 2/6] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e43618fa5fd..68cb83d3456 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,3 +3,4 @@ - Make storage emulator content type case insensitive. (#3953) - Add storage emulator support to init.js useEmulator flag. (#4805) - Populate resource correctly in storage rules evaluation. (#4329) +- Read name from metadata as backup for gcs upload into storage emulator. (#3953) From aa8b54c4bb4b42b5f1b4af8055d88f1e1443d6f8 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Thu, 4 Aug 2022 15:40:17 -0400 Subject: [PATCH 3/6] address comments --- scripts/storage-emulator-integration/tests.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scripts/storage-emulator-integration/tests.ts b/scripts/storage-emulator-integration/tests.ts index 4a1064fa5f4..5de7589c035 100644 --- a/scripts/storage-emulator-integration/tests.ts +++ b/scripts/storage-emulator-integration/tests.ts @@ -239,9 +239,7 @@ describe("Storage emulator", () => { expect(uploadURL.searchParams?.get("name")).to.equal(fileName); }); - it.only("should handle multipart upload with name only in metadata", async () => { - const contentTypeHeader = - "multipart/related; boundary=b1d5b2e3-1845-4338-9400-6ac07ce53c1e"; + it("should handle multipart upload with name only in metadata", async () => { const body = Buffer.from(`--b1d5b2e3-1845-4338-9400-6ac07ce53c1e\r content-type: application/json\r \r @@ -259,7 +257,7 @@ hello there! .send(body) .set({ Authorization: "Bearer owner", - "content-type": contentTypeHeader, + "content-type": "multipart/related; boundary=b1d5b2e3-1845-4338-9400-6ac07ce53c1e", }) .expect(200) .then((res) => res.body.name); From 5a52834e5c598d9fe4fa3d9e213b478be3983bf5 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Thu, 4 Aug 2022 16:51:51 -0400 Subject: [PATCH 4/6] address comments --- scripts/storage-emulator-integration/tests.ts | 14 ++++++++++--- src/emulator/storage/apis/gcloud.ts | 21 +++++++++++-------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/scripts/storage-emulator-integration/tests.ts b/scripts/storage-emulator-integration/tests.ts index 5de7589c035..2c525136a2d 100644 --- a/scripts/storage-emulator-integration/tests.ts +++ b/scripts/storage-emulator-integration/tests.ts @@ -225,10 +225,14 @@ describe("Storage emulator", () => { expect(fileMetadata).to.deep.include(metadata); }); - + // TODO(b/241151246): Fix conformance tests. it("should handle resumable upload with name only in metadata", async () => { + const expectedHost = TEST_CONFIG.useProductionServers + ? "https://firebasestorage.googleapis.com" + : STORAGE_EMULATOR_HOST; + const fileName = "test_upload.jpg"; - const uploadURL = await supertest(STORAGE_EMULATOR_HOST) + const uploadURL = await supertest(expectedHost) .post(`/upload/storage/v1/b/${storageBucket}/o?uploadType=resumable`) .send({ name: fileName }) .set({ @@ -240,6 +244,10 @@ describe("Storage emulator", () => { }); it("should handle multipart upload with name only in metadata", async () => { + const expectedHost = TEST_CONFIG.useProductionServers + ? "https://firebasestorage.googleapis.com" + : STORAGE_EMULATOR_HOST; + const body = Buffer.from(`--b1d5b2e3-1845-4338-9400-6ac07ce53c1e\r content-type: application/json\r \r @@ -252,7 +260,7 @@ hello there! --b1d5b2e3-1845-4338-9400-6ac07ce53c1e--\r `); const fileName = "test_upload.jpg"; - const responseName = await supertest(STORAGE_EMULATOR_HOST) + const responseName = await supertest(expectedHost) .post(`/upload/storage/v1/b/${storageBucket}/o?uploadType=multipart`) .send(body) .set({ diff --git a/src/emulator/storage/apis/gcloud.ts b/src/emulator/storage/apis/gcloud.ts index 971d8c69826..fda8d0d4fbf 100644 --- a/src/emulator/storage/apis/gcloud.ts +++ b/src/emulator/storage/apis/gcloud.ts @@ -209,8 +209,8 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { if (emulatorInfo === undefined) { return res.sendStatus(500); } - const name = getValidName(req.query, req.body); - if (name == null) { + const name = getIncomingFileNameFromRequest(req.query, req.body); + if (name === undefined) { res.sendStatus(400); return; } @@ -232,17 +232,11 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { // Multipart upload let metadataRaw: string; let dataRaw: Buffer; - let name: string | undefined; try { ({ metadataRaw, dataRaw } = parseObjectUploadMultipartRequest( contentTypeHeader!, await reqBodyToBuffer(req) )); - name = getValidName(req.query, JSON.parse(metadataRaw)); - if (name == null) { - res.sendStatus(400); - return; - } } catch (err) { if (err instanceof Error) { return res.status(400).json({ @@ -255,6 +249,12 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { throw err; } + const name = getIncomingFileNameFromRequest(req.query, JSON.parse(metadataRaw)); + if (name === undefined) { + res.sendStatus(400); + return; + } + const upload = uploadService.multipartUpload({ bucketId: req.params.bucketId, objectId: name, @@ -413,7 +413,10 @@ function sendObjectNotFound(req: Request, res: Response): void { } } -function getValidName(query: Query, metadata: IncomingMetadata): string | undefined { +function getIncomingFileNameFromRequest( + query: Query, + metadata: IncomingMetadata +): string | undefined { const name = query?.name?.toString() || metadata?.name; return name?.startsWith("/") ? name.slice(1) : name; } From b2c9eeddc2a35c1c24d65041f8ab5020ddfdf51f Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Thu, 4 Aug 2022 17:02:54 -0400 Subject: [PATCH 5/6] move host deciding logic to top of test file --- scripts/storage-emulator-integration/tests.ts | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/scripts/storage-emulator-integration/tests.ts b/scripts/storage-emulator-integration/tests.ts index 2c525136a2d..1a4047d476e 100644 --- a/scripts/storage-emulator-integration/tests.ts +++ b/scripts/storage-emulator-integration/tests.ts @@ -71,6 +71,10 @@ describe("Storage emulator", () => { const STORAGE_EMULATOR_HOST = getStorageEmulatorHost(emulatorConfig); const AUTH_EMULATOR_HOST = getAuthEmulatorHost(emulatorConfig); + const expectedHost = TEST_CONFIG.useProductionServers + ? "https://firebasestorage.googleapis.com" + : STORAGE_EMULATOR_HOST; + const emulatorSpecificDescribe = TEST_CONFIG.useProductionServers ? describe.skip : describe; async function resetEmulatorState(): Promise { @@ -227,10 +231,6 @@ describe("Storage emulator", () => { }); // TODO(b/241151246): Fix conformance tests. it("should handle resumable upload with name only in metadata", async () => { - const expectedHost = TEST_CONFIG.useProductionServers - ? "https://firebasestorage.googleapis.com" - : STORAGE_EMULATOR_HOST; - const fileName = "test_upload.jpg"; const uploadURL = await supertest(expectedHost) .post(`/upload/storage/v1/b/${storageBucket}/o?uploadType=resumable`) @@ -244,10 +244,6 @@ describe("Storage emulator", () => { }); it("should handle multipart upload with name only in metadata", async () => { - const expectedHost = TEST_CONFIG.useProductionServers - ? "https://firebasestorage.googleapis.com" - : STORAGE_EMULATOR_HOST; - const body = Buffer.from(`--b1d5b2e3-1845-4338-9400-6ac07ce53c1e\r content-type: application/json\r \r @@ -1609,10 +1605,6 @@ hello there! const downloadUrl: string = await page.evaluate((filename) => { return firebase.storage().ref(filename).getDownloadURL(); }, filename); - const expectedHost = TEST_CONFIG.useProductionServers - ? "https://firebasestorage.googleapis.com" - : STORAGE_EMULATOR_HOST; - expect(downloadUrl).to.contain( `${expectedHost}/v0/b/${storageBucket}/o/testing%2Fstorage_ref%2Fimage.png?alt=media&token=` ); From ca9660afb9d9e1de1433c7ec2299b1c718a051ee Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Thu, 4 Aug 2022 17:07:21 -0400 Subject: [PATCH 6/6] lint --- scripts/storage-emulator-integration/tests.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/storage-emulator-integration/tests.ts b/scripts/storage-emulator-integration/tests.ts index 1a4047d476e..2709f18b641 100644 --- a/scripts/storage-emulator-integration/tests.ts +++ b/scripts/storage-emulator-integration/tests.ts @@ -72,9 +72,9 @@ describe("Storage emulator", () => { const AUTH_EMULATOR_HOST = getAuthEmulatorHost(emulatorConfig); const expectedHost = TEST_CONFIG.useProductionServers - ? "https://firebasestorage.googleapis.com" - : STORAGE_EMULATOR_HOST; - + ? "https://firebasestorage.googleapis.com" + : STORAGE_EMULATOR_HOST; + const emulatorSpecificDescribe = TEST_CONFIG.useProductionServers ? describe.skip : describe; async function resetEmulatorState(): Promise {