feat: support v4 signed url endpoint by 7hokerz · Pull Request #10490 · firebase/firebase-tools

7hokerz · 2026-05-12T14:10:41Z

Description

Issue: [Emulator] Support generateSignedPostPolicyV4 in Storage Emulator #10488

I added the endpoint and also added test code for the endpoint.

Additional Context

Actually, the latest version of the package has been released, but the existing method,
npm i @google-cloud/storage@latest, failed to download the package.
@google-cloud/storage v7.20.0 release

gemini-code-assist

Code Review

This pull request implements support for signed post policy uploads in the GCS emulator by adding a new POST endpoint and a custom multipart/form-data parser. The feedback identifies a bug where the parser could truncate data if a trailing line separator is missing and suggests adding a verification check. Additionally, the reviewer recommends against trimming custom metadata values to maintain parity with GCS behavior and suggests making the boundary extraction logic more robust to handle extra Content-Type parameters.

gemini-code-assist · 2026-05-12T14:12:44Z

+  const headerBuffer = sections[0];
+  const bodyBuffer = sections[1];
+
+  const dataRaw = bodyBuffer.slice(0, bodyBuffer.byteLength - LINE_SEPARATOR.length);


The parser assumes that every multipart part body ends with a line separator (\r\n), but it doesn't verify this. If the separator is missing, it will silently truncate the last two bytes of the actual data. Furthermore, the included tests (line 231 in multipart.spec.ts) expect an error to be thrown in this case, which currently won't happen, causing the tests to fail.

Suggested change

const dataRaw = bodyBuffer.slice(0, bodyBuffer.byteLength - LINE_SEPARATOR.length);

if (!bodyBuffer.slice(-LINE_SEPARATOR.length).equals(Buffer.from(LINE_SEPARATOR))) {

throw new Error("Failed to parse multipart part: Missing trailing line separator.");

}

const dataRaw = bodyBuffer.slice(0, bodyBuffer.byteLength - LINE_SEPARATOR.length);

gemini-code-assist · 2026-05-12T14:12:44Z

+          if (part.type === "file" || part.name === "key") continue;
+
+          const key = part.name.toLowerCase();
+          const value = part.value.trim();


Trimming the value of form fields can lead to data corruption for custom metadata (x-goog-meta-*). Google Cloud Storage preserves leading and trailing spaces in custom metadata values. Since the multipart parser already handles the trailing line separator of the part, additional trimming here is likely unnecessary and potentially incorrect.

Suggested change

const value = part.value.trim();

const value = part.value;

gemini-code-assist · 2026-05-12T14:12:44Z

+  if (!contentTypeHeader.startsWith("multipart/form-data")) {
+    throw new Error(`Bad content type. ${contentTypeHeader}`);
+  }
+  const boundaryId = contentTypeHeader.split("boundary=")[1];


Extracting the boundary by simply splitting on boundary= and taking the rest of the string is fragile. If the Content-Type header contains additional parameters after the boundary (e.g., ; charset=utf-8), they will be incorrectly included in the boundaryId, causing the parser to fail to find the boundary markers in the body.

Suggested change

const boundaryId = contentTypeHeader.split("boundary=")[1];

const boundaryId = contentTypeHeader.split("boundary=")[1]?.split(";")[0].trim();

adjustment to gemini solution:

contentTypeHeader.split("boundary=")[1]?.split(";")[0]?.trim();

The gemini recommended solution assumes "boundary=" is present.

7hokerz · 2026-05-13T22:57:10Z

@aalej @cabljac , Could you please take a look at this when you have a moment?

aalej · 2026-05-14T12:42:51Z

Thanks for submitting this PR @7hokerz! I don't have a good understanding of the Storage emulator, but let me try checking with our engineering team to see if anyone more familiar with the Storage emulator would be able to take a look.

Just to set expectations, we're going on a code freeze, so it might take some time for this PR to get merged.

7hokerz · 2026-05-14T14:37:45Z

Thanks for submitting this PR @7hokerz! I don't have a good understanding of the Storage emulator, but let me try checking with our engineering team to see if anyone more familiar with the Storage emulator would be able to take a look.

Just to set expectations, we're going on a code freeze, so it might take some time for this PR to get merged.

@aalej , Thank you for your response. I will gladly wait until a storage emulator expert is available. I appreciate your assistance!

annajowang

(i am also not a "storage emulator expert". I don't think there currently is one. but if bugs come up after this is merged they can always be fixed)

Left some comments, please address them and the gemini ones as well.
Thanks for your contribution!

annajowang · 2026-05-14T16:01:46Z

+  if (!contentTypeHeader.startsWith("multipart/form-data")) {
+    throw new Error(`Bad content type. ${contentTypeHeader}`);
+  }
+  const boundaryId = contentTypeHeader.split("boundary=")[1];


adjustment to gemini solution:

contentTypeHeader.split("boundary=")[1]?.split(";")[0]?.trim();

The gemini recommended solution assumes "boundary=" is present.

annajowang · 2026-05-14T16:07:46Z

+        for (const part of formData) {
+          if (part.type === "file" || part.name === "key") continue;
+
+          const key = part.name.toLowerCase();
+          const value = part.value.trim();
+
+          if (key.startsWith("x-goog-meta-")) {
+            const metaKey = key.replace("x-goog-meta-", "");
+            metadata.metadata![metaKey] = value;
+          } else {
+            switch (key) {
+              case "content-type":
+                metadata.contentType = value;
+                break;
+              case "cache-control":
+                metadata.cacheControl = value;
+                break;
+              case "content-disposition":
+                metadata.contentDisposition = value;
+                break;
+              case "content-encoding":
+                metadata.contentEncoding = value;
+                break;
+              case "content-language":
+                metadata.contentLanguage = value;
+                break;
+            }
+          }
+        }


opinion nit: this seems a little easier to read to me

const HEADER_MAP: Record<string, keyof IncomingMetadata> = { "content-type": "contentType", "cache-control": "cacheControl", "content-disposition": "contentDisposition", "content-encoding": "contentEncoding", "content-language": "contentLanguage", }; for (const part of formData) { if (part.type === "file" || part.name === "key") continue; const key = part.name.toLowerCase(); if (key.startsWith("x-goog-meta-")) { metadata.metadata![key.substring(12)] = part.value; } else if (HEADER_MAP[key]) { metadata[HEADER_MAP[key]] = part.value.trim(); } }

annajowang · 2026-05-14T16:19:33Z

+      const contentDispositionParams = h.split(";").slice(1);
+      for (const param of contentDispositionParams) {
+        const [paramName, ...valueParts] = param.trim().split("=");
+        const value = valueParts.join("=").replace(/^["'](.+(?=["']$))["']$/, "$1");
+        if (paramName === "name") {
+          name = value;
+        } else if (paramName === "filename") {
+          filename = value;
+        }
+      }


I believe this is all just to extract the name and filename values?

could this just be simplified to:

name = extractParam(h, "name") ?? name; filename = extractParam(h, "filename") ?? filename;

with some descriptively-named helper function like:

function extractParam(header: string, param: string): string | undefined { const match = header.match(new RegExp(`\\b${param}=["']?([^"';]+)["']?`, "i")); return match ? match[1] : undefined; }

7hokerz · 2026-05-14T16:30:12Z

(i am also not a "storage emulator expert". I don't think there currently is one. but if bugs come up after this is merged they can always be fixed)

Left some comments, please address them and the gemini ones as well. Thanks for your contribution!

@annajowang , Thank you for your feedback! I will take the time to review it and make improvements soon.

feat: support v4 signed url endpoint

38cd91f

gemini-code-assist Bot reviewed May 12, 2026

View reviewed changes

annajowang reviewed May 14, 2026

View reviewed changes

	const boundaryId = contentTypeHeader.split("boundary=")[1];
	const boundaryId = contentTypeHeader.split("boundary=")[1]?.split(";")[0].trim();

Conversation

7hokerz commented May 12, 2026

Description

Additional Context

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

annajowang May 14, 2026

Choose a reason for hiding this comment

Uh oh!

7hokerz commented May 13, 2026

Uh oh!

aalej commented May 14, 2026

Uh oh!

7hokerz commented May 14, 2026

Uh oh!

annajowang left a comment

Choose a reason for hiding this comment

Uh oh!

annajowang May 14, 2026

Choose a reason for hiding this comment

Uh oh!

annajowang May 14, 2026

Choose a reason for hiding this comment

Uh oh!

annajowang May 14, 2026

Choose a reason for hiding this comment

Uh oh!

7hokerz commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants