Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
pearigee wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions handwritten/storage/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@
"async-retry": "^1.3.3",
"duplexify": "^4.1.3",
"fast-xml-parser": "^5.3.4",
"gaxios": "^6.0.2",
"google-auth-library": "^9.6.3",
"gaxios": "^7.1.6",
"google-auth-library": "^10.9.0",
"html-entities": "^2.5.2",
"mime": "^3.0.0",
"p-limit": "^3.0.1",
"retry-request": "^7.0.0",
"teeny-request": "^9.0.0"
"teeny-request": "^10.1.0"
},
"devDependencies": {
"@babel/cli": "^7.22.10",
Expand Down
66 changes: 41 additions & 25 deletions handwritten/storage/src/resumable-upload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import AbortController from 'abort-controller';
import {createHash} from 'crypto';
import {
GaxiosOptions,
Expand Down Expand Up @@ -46,6 +45,13 @@ const NOT_FOUND_STATUS_CODE = 404;
const RESUMABLE_INCOMPLETE_STATUS_CODE = 308;
const packageJson = getPackageJSON();

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;
}
Comment on lines +48 to +53

@gemini-code-assist gemini-code-assist Bot Jul 7, 2026

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;
  }
}


export const PROTOCOL_REGEX = /^(\w*):\/\//;

export interface ErrorWithCode extends Error {
Expand Down Expand Up @@ -615,7 +621,7 @@ export class Upload extends Writable {
*
* @param headers The headers object to modify.
*/
#applyChecksumHeaders(headers: GaxiosOptions['headers']) {
#applyChecksumHeaders(headers: Record<string, string>) {
const checksums: string[] = [];

if (this.#hashValidator?.crc32cEnabled) {
Expand All @@ -631,7 +637,7 @@ export class Upload extends Writable {
}

if (checksums.length > 0) {
headers!['X-Goog-Hash'] = checksums.join(',');
headers['X-Goog-Hash'] = checksums.join(',');
}
}

Expand Down Expand Up @@ -792,7 +798,7 @@ export class Upload extends Writable {

protected async createURIAsync(): Promise<string> {
const metadata = {...this.metadata};
const headers: gaxios.Headers = {};
const headers: Record<string, string> = {};

// Delete content length and content type from metadata if they exist.
// These are headers and should not be sent as part of the metadata.
Expand All @@ -802,7 +808,7 @@ export class Upload extends Writable {
}

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

Expand Down Expand Up @@ -833,13 +839,16 @@ export class Upload extends Writable {
},
};

if (metadata.contentLength) {
reqOpts.headers!['X-Upload-Content-Length'] =
metadata.contentLength.toString();
}
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;
}
}
Comment on lines +842 to 852

@gemini-code-assist gemini-code-assist Bot Jul 7, 2026

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);
      }
    }


if (typeof this.generation !== 'undefined') {
Expand All @@ -855,15 +864,18 @@ export class Upload extends Writable {
}

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

@gemini-code-assist gemini-code-assist Bot Jul 7, 2026

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);
      }

const uri = await AsyncRetry(
async (bail: (err: Error) => void) => {
try {
const res = await this.makeRequest(reqOpts);
// We have successfully got a URI we can now create a new invocation id
this.currentInvocationId.uri = crypto.randomUUID();
return res.headers.location;
return getResponseHeader(res.headers, 'location') || '';
} catch (err) {
const e = err as GaxiosError;
const apiError = {
Expand All @@ -882,7 +894,8 @@ export class Upload extends Writable {
) {
throw e;
} else {
return bail(e);
bail(e);
return '';
}
}
},
Expand Down Expand Up @@ -1005,7 +1018,7 @@ export class Upload extends Writable {
googAPIClient += ` gccl-gcs-cmd/${this.#gcclGcsCmd}`;
}

const headers: GaxiosOptions['headers'] = {
const headers: Record<string, string> = {
'User-Agent': getUserAgentString(),
'x-goog-api-client': googAPIClient,
};
Expand Down Expand Up @@ -1050,7 +1063,7 @@ export class Upload extends Writable {

// `Content-Length` for multiple chunk uploads is the size of the chunk,
// not the overall object
headers['Content-Length'] = bytesToUpload;
headers['Content-Length'] = bytesToUpload.toString();
headers['Content-Range'] =
`bytes ${this.offset}-${endingByte}/${totalObjectSize}`;

Expand Down Expand Up @@ -1111,7 +1124,7 @@ export class Upload extends Writable {
const shouldContinueWithNextMultiChunkRequest =
this.chunkSize &&
resp.status === RESUMABLE_INCOMPLETE_STATUS_CODE &&
resp.headers.range &&
getResponseHeader(resp.headers, 'range') &&
moreDataToUpload;

/**
Expand All @@ -1127,7 +1140,7 @@ export class Upload extends Writable {
// Use the upper value in this header to determine where to start the next chunk.
// We should not assume that the server received all bytes sent in the request.
// https://cloud.google.com/storage/docs/performing-resumable-uploads#chunked-upload
const range: string = resp.headers.range;
const range: string = getResponseHeader(resp.headers, 'range')!;
this.offset = Number(range.split('-')[1]) + 1;

// We should not assume that the server received all bytes sent in the request.
Expand Down Expand Up @@ -1223,7 +1236,7 @@ export class Upload extends Writable {
method: 'PUT',
url: this.uri,
headers: {
'Content-Length': 0,
'Content-Length': '0',
'Content-Range': 'bytes */*',
'User-Agent': getUserAgentString(),
'x-goog-api-client': googAPIClient,
Expand Down Expand Up @@ -1264,8 +1277,9 @@ export class Upload extends Writable {
const resp = await this.checkUploadStatus({retry: false});

if (resp.status === RESUMABLE_INCOMPLETE_STATUS_CODE) {
if (typeof resp.headers.range === 'string') {
this.offset = Number(resp.headers.range.split('-')[1]) + 1;
const range = getResponseHeader(resp.headers, 'range');
if (typeof range === 'string') {
this.offset = Number(range.split('-')[1]) + 1;
return;
}
}
Expand All @@ -1288,10 +1302,12 @@ export class Upload extends Writable {
private async makeRequest(reqOpts: GaxiosOptions): GaxiosPromise {
if (this.encryption) {
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();
}
}
Comment on lines 1304 to 1311

@gemini-code-assist gemini-code-assist Bot Jul 7, 2026

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());


if (this.userProject) {
Expand Down
18 changes: 9 additions & 9 deletions handwritten/storage/src/transfer-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {GoogleAuth} from 'google-auth-library';
import {XMLParser, XMLBuilder} from 'fast-xml-parser';
import AsyncRetry from 'async-retry';
import {ApiError} from './nodejs-common/index.js';
import {GaxiosResponse, Headers} from 'gaxios';
import {GaxiosResponse} from 'gaxios';
import {createHash} from 'crypto';
import {GCCL_GCS_CMD_KEY} from './nodejs-common/util.js';
import {getRuntimeTrackingString, getUserAgentString} from './util.js';
Expand Down Expand Up @@ -220,7 +220,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper {
};
}

#setGoogApiClientHeaders(headers: Headers = {}): Headers {
#setGoogApiClientHeaders(headers: Record<string, string> = {}): Record<string, string> {
let headerFound = false;
let userAgentFound = false;

Expand Down Expand Up @@ -258,11 +258,11 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper {
*
* @returns {Promise<void>}
*/
async initiateUpload(headers: Headers = {}): Promise<void> {
async initiateUpload(headers: Record<string, string> = {}): Promise<void> {
const url = `${this.baseUrl}?uploads`;
return AsyncRetry(async bail => {
try {
const res = await this.authClient.request({
const res = await this.authClient.request<any>({
headers: this.#setGoogApiClientHeaders(headers),
method: 'POST',
url,
Expand Down Expand Up @@ -294,7 +294,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper {
validation?: 'md5' | 'crc32c' | false,
): Promise<void> {
const url = `${this.baseUrl}?partNumber=${partNumber}&uploadId=${this.uploadId}`;
let headers: Headers = this.#setGoogApiClientHeaders();
let headers: Record<string, string> = this.#setGoogApiClientHeaders();

if (validation === 'md5') {
const hash = createHash('md5').update(chunk).digest('base64');
Expand All @@ -309,7 +309,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper {

return AsyncRetry(async bail => {
try {
const res = await this.authClient.request({
const res = await this.authClient.request<any>({
url,
method: 'PUT',
body: chunk,
Expand All @@ -318,7 +318,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper {
if (res.data && res.data.error) {
throw res.data.error;
}
this.partsMap.set(partNumber, res.headers['etag']);
this.partsMap.set(partNumber, res.headers.get('etag') || '');
} catch (e) {
this.#handleErrorResponse(e as Error, bail);
}
Expand All @@ -344,7 +344,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper {
)}</CompleteMultipartUpload>`;
return AsyncRetry(async bail => {
try {
const res = await this.authClient.request({
const res = await this.authClient.request<any>({
headers: this.#setGoogApiClientHeaders(),
url,
method: 'POST',
Expand All @@ -371,7 +371,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper {
const url = `${this.baseUrl}?uploadId=${this.uploadId}`;
return AsyncRetry(async bail => {
try {
const res = await this.authClient.request({
const res = await this.authClient.request<any>({
url,
method: 'DELETE',
});
Expand Down
2 changes: 1 addition & 1 deletion handwritten/storage/system-test/kitchen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ describe('resumable-upload', () => {
});

assert.ok(!resp.data);
assert.equal(resp.headers['content-length'], '0');
assert.equal(resp.headers.get('content-length'), '0');
});

it('should return a non-resumable failed upload', done => {
Expand Down
2 changes: 1 addition & 1 deletion handwritten/storage/test/nodejs-common/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ describe('Service', () => {
}

async getRequestHeaders() {
return {};
return new Headers();
}

request = OAuth2Client.prototype.request.bind(this);
Expand Down
2 changes: 1 addition & 1 deletion handwritten/storage/test/nodejs-common/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ describe('common/util', () => {
}

async getRequestHeaders() {
return {};
return new Headers();
}

request = OAuth2Client.prototype.request.bind(this);
Expand Down
Loading
Loading