Skip to content

fix!: Update storage to use newer versions of gaxios, teeny-request, and google-auth-library#8827

Draft
pearigee wants to merge 1 commit into
mainfrom
pearigee-storage-upgrade
Draft

fix!: Update storage to use newer versions of gaxios, teeny-request, and google-auth-library#8827
pearigee wants to merge 1 commit into
mainfrom
pearigee-storage-upgrade

Conversation

@pearigee

@pearigee pearigee commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

These newer versions required code changes to be compatible, specifically changes to how headers are handled.

@pearigee pearigee requested a review from a team as a code owner July 7, 2026 17:52

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates dependency versions and refactors header handling in resumable-upload.ts and transfer-manager.ts to support different header formats (such as Headers instances, arrays, and plain objects), alongside updating corresponding tests. The code review feedback correctly identifies a case-sensitivity bug in the new getResponseHeader helper and points out that checking for non-Headers and non-array types before setting headers can cause critical metadata and encryption headers to be silently ignored. The reviewer suggests implementing robust getResponseHeader and setHeader helper functions to safely handle all header formats consistently.

Comment on lines +48 to +53
function getResponseHeader(headers: any, name: string): string | undefined {
if (headers && typeof headers.get === 'function') {
return headers.get(name) || undefined;
}
return headers?.[name] || headers?.[name.toLowerCase()] || undefined;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation of getResponseHeader has a case-sensitivity bug when headers is a plain object. For example, if name is 'location' and the object contains { Location: '...' }, both headers?.[name] and headers?.[name.toLowerCase()] will look up 'location' and return undefined.

Additionally, throughout this file, headers are conditionally set only if reqOpts.headers is not an instance of Headers and not an array. If reqOpts.headers is a Headers object or an array, these headers (including critical encryption headers) are silently ignored.

Let's define robust getResponseHeader and setHeader helper functions at the top of the file to handle all header types (Headers object, array of entries, or plain object) correctly.

function getResponseHeader(headers: any, name: string): string | undefined {
  if (!headers) return undefined;
  if (typeof headers.get === 'function') {
    return headers.get(name) || undefined;
  }
  if (Array.isArray(headers)) {
    const entry = headers.find(h => h[0].toLowerCase() === name.toLowerCase());
    return entry ? entry[1] : undefined;
  }
  const key = Object.keys(headers).find(k => k.toLowerCase() === name.toLowerCase());
  return key ? headers[key] : undefined;
}

function setHeader(headers: any, name: string, value: string): void {
  if (!headers) return;
  if (typeof headers.set === 'function') {
    headers.set(name, value);
  } else if (Array.isArray(headers)) {
    const index = headers.findIndex(h => h[0].toLowerCase() === name.toLowerCase());
    if (index !== -1) {
      headers[index][1] = value;
    } else {
      headers.push([name, value]);
    }
  } else {
    headers[name] = value;
  }
}

Comment on lines +842 to 852
const reqHeaders = reqOpts.headers;
if (reqHeaders && !(reqHeaders instanceof Headers) && !Array.isArray(reqHeaders)) {
if (metadata.contentLength) {
reqHeaders['X-Upload-Content-Length'] =
metadata.contentLength.toString();
}

if (metadata.contentType) {
reqOpts.headers!['X-Upload-Content-Type'] = metadata.contentType;
if (metadata.contentType) {
reqHeaders['X-Upload-Content-Type'] = metadata.contentType;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use the new setHeader helper to safely set the upload headers regardless of whether reqOpts.headers is a plain object, a Headers instance, or an array.

    if (reqOpts.headers) {
      if (metadata.contentLength) {
        setHeader(reqOpts.headers, 'X-Upload-Content-Length', metadata.contentLength.toString());
      }

      if (metadata.contentType) {
        setHeader(reqOpts.headers, 'X-Upload-Content-Type', metadata.contentType);
      }
    }

Comment on lines +867 to 871
const reqHeaders = reqOpts.headers;
if (reqHeaders && !(reqHeaders instanceof Headers) && !Array.isArray(reqHeaders)) {
reqHeaders.Origin = this.origin;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use the setHeader helper to safely set the Origin header.

      if (reqOpts.headers) {
        setHeader(reqOpts.headers, 'Origin', this.origin);
      }

Comment on lines 1304 to 1311
reqOpts.headers = reqOpts.headers || {};
reqOpts.headers['x-goog-encryption-algorithm'] = 'AES256';
reqOpts.headers['x-goog-encryption-key'] = this.encryption.key.toString();
reqOpts.headers['x-goog-encryption-key-sha256'] =
this.encryption.hash.toString();
if (!(reqOpts.headers instanceof Headers) && !Array.isArray(reqOpts.headers)) {
reqOpts.headers['x-goog-encryption-algorithm'] = 'AES256';
reqOpts.headers['x-goog-encryption-key'] = this.encryption.key.toString();
reqOpts.headers['x-goog-encryption-key-sha256'] =
this.encryption.hash.toString();
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

Use the setHeader helper to safely set the encryption headers. This ensures customer-supplied encryption keys (CSEK) are not silently ignored if reqOpts.headers is a Headers instance or an array.

      reqOpts.headers = reqOpts.headers || {};
      setHeader(reqOpts.headers, 'x-goog-encryption-algorithm', 'AES256');
      setHeader(reqOpts.headers, 'x-goog-encryption-key', this.encryption.key.toString());
      setHeader(reqOpts.headers, 'x-goog-encryption-key-sha256', this.encryption.hash.toString());

@pearigee pearigee marked this pull request as draft July 7, 2026 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant