From 0520867e51a2758ddf2773c0d910c937d55e21b1 Mon Sep 17 00:00:00 2001 From: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> Date: Thu, 19 Oct 2023 10:54:06 -0400 Subject: [PATCH] fix: add user-agent header to transfer manager and resumable upload (#2334) --- src/nodejs-common/service.ts | 4 ++-- src/nodejs-common/util.ts | 18 ++------------- src/resumable-upload.ts | 5 ++++- src/transfer-manager.ts | 11 ++++++++-- src/util.ts | 13 +++++++++++ test/nodejs-common/service.ts | 33 ++-------------------------- test/nodejs-common/util.ts | 11 ---------- test/resumable-upload.ts | 15 +++++++++++++ test/transfer-manager.ts | 41 ++++++++++++++++++++++++++++++++++- 9 files changed, 87 insertions(+), 64 deletions(-) diff --git a/src/nodejs-common/service.ts b/src/nodejs-common/service.ts index 69fc9dfb6..d98bdae1b 100644 --- a/src/nodejs-common/service.ts +++ b/src/nodejs-common/service.ts @@ -26,7 +26,7 @@ import { PackageJson, util, } from './util'; -import {getRuntimeTrackingString} from '../util'; +import {getRuntimeTrackingString, getUserAgentString} from '../util'; export const DEFAULT_PROJECT_ID_TOKEN = '{{projectId}}'; @@ -242,7 +242,7 @@ export class Service { delete reqOpts.interceptors_; const pkg = this.packageJson; - let userAgent = util.getUserAgentFromPackageJson(pkg); + let userAgent = getUserAgentString(); if (this.providedUserAgent) { userAgent = `${this.providedUserAgent} ${userAgent}`; } diff --git a/src/nodejs-common/util.ts b/src/nodejs-common/util.ts index c119fd71b..8aade7083 100644 --- a/src/nodejs-common/util.ts +++ b/src/nodejs-common/util.ts @@ -32,7 +32,7 @@ import {teenyRequest} from 'teeny-request'; import {Interceptor} from './service-object'; import * as uuid from 'uuid'; import {DEFAULT_PROJECT_ID_TOKEN} from './service'; -import {getRuntimeTrackingString} from '../util'; +import {getRuntimeTrackingString, getUserAgentString} from '../util'; const packageJson = require('../../../package.json'); @@ -1001,20 +1001,6 @@ export class Util { } } - /** - * Create a properly-formatted User-Agent string from a package.json file. - * - * @param {object} packageJson - A module's package.json file. - * @return {string} userAgent - The formatted User-Agent string. - */ - getUserAgentFromPackageJson(packageJson: PackageJson) { - const hyphenatedPackageName = packageJson.name - .replace('@google-cloud', 'gcloud-node') // For legacy purposes. - .replace('/', '-'); // For UA spec-compliance purposes. - - return hyphenatedPackageName + '/' + packageJson.version; - } - /** * Given two parameters, figure out if this is either: * - Just a callback function @@ -1033,7 +1019,7 @@ export class Util { _getDefaultHeaders(gcclGcsCmd?: string) { const headers = { - 'User-Agent': util.getUserAgentFromPackageJson(packageJson), + 'User-Agent': getUserAgentString(), 'x-goog-api-client': `${getRuntimeTrackingString()} gccl/${ packageJson.version } gccl-invocation-id/${uuid.v4()}`, diff --git a/src/resumable-upload.ts b/src/resumable-upload.ts index f0194869f..df316c5c9 100644 --- a/src/resumable-upload.ts +++ b/src/resumable-upload.ts @@ -26,7 +26,7 @@ import {Readable, Writable, WritableOptions} from 'stream'; import retry = require('async-retry'); import {RetryOptions, PreconditionOptions} from './storage'; import * as uuid from 'uuid'; -import {getRuntimeTrackingString} from './util'; +import {getRuntimeTrackingString, getUserAgentString} from './util'; import {GCCL_GCS_CMD_KEY} from './nodejs-common/util'; const NOT_FOUND_STATUS_CODE = 404; @@ -612,6 +612,7 @@ export class Upload extends Writable { ), data: metadata, headers: { + 'User-Agent': getUserAgentString(), 'x-goog-api-client': googAPIClient, ...headers, }, @@ -787,6 +788,7 @@ export class Upload extends Writable { } const headers: GaxiosOptions['headers'] = { + 'User-Agent': getUserAgentString(), 'x-goog-api-client': googAPIClient, }; @@ -936,6 +938,7 @@ export class Upload extends Writable { headers: { 'Content-Length': 0, 'Content-Range': 'bytes */*', + 'User-Agent': getUserAgentString(), 'x-goog-api-client': googAPIClient, }, }; diff --git a/src/transfer-manager.ts b/src/transfer-manager.ts index 6e659a4c5..3d8d16113 100644 --- a/src/transfer-manager.ts +++ b/src/transfer-manager.ts @@ -27,7 +27,7 @@ import {ApiError} from './nodejs-common'; import {GaxiosResponse, Headers} from 'gaxios'; import {createHash} from 'crypto'; import {GCCL_GCS_CMD_KEY} from './nodejs-common/util'; -import {getRuntimeTrackingString} from './util'; +import {getRuntimeTrackingString, getUserAgentString} from './util'; const packageJson = require('../../package.json'); @@ -205,6 +205,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper { #setGoogApiClientHeaders(headers: Headers = {}): Headers { let headerFound = false; + let userAgentFound = false; for (const [key, value] of Object.entries(headers)) { if (key.toLocaleLowerCase().trim() === 'x-goog-api-client') { @@ -216,7 +217,8 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper { key ] = `${value} gccl-gcs-cmd/${GCCL_GCS_CMD_FEATURE.UPLOAD_SHARDED}`; } - break; + } else if (key.toLocaleLowerCase().trim() === 'user-agent') { + userAgentFound = true; } } @@ -227,6 +229,11 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper { } gccl-gcs-cmd/${GCCL_GCS_CMD_FEATURE.UPLOAD_SHARDED}`; } + // If the User-Agent isn't present, add it + if (!userAgentFound) { + headers['User-Agent'] = getUserAgentString(); + } + return headers; } diff --git a/src/util.ts b/src/util.ts index 6704fb91c..3740d4324 100644 --- a/src/util.ts +++ b/src/util.ts @@ -192,6 +192,19 @@ export function getRuntimeTrackingString(): string { } } +/** + * Looks at package.json and creates the user-agent string to be applied to request headers. + * @returns {string} user agent string. + */ +export function getUserAgentString(): string { + const pkg = require('../../package.json'); + const hyphenatedPackageName = pkg.name + .replace('@google-cloud', 'gcloud-node') // For legacy purposes. + .replace('/', '-'); // For UA spec-compliance purposes. + + return hyphenatedPackageName + '/' + pkg.version; +} + export class PassThroughShim extends PassThrough { private shouldEmitReading = true; private shouldEmitWriting = true; diff --git a/test/nodejs-common/service.ts b/test/nodejs-common/service.ts index c92b24df7..6af6185c5 100644 --- a/test/nodejs-common/service.ts +++ b/test/nodejs-common/service.ts @@ -35,6 +35,7 @@ import { util, Util, } from '../../src/nodejs-common/util'; +import {getUserAgentString} from '../../src/util'; proxyquire.noPreserveCache(); @@ -473,40 +474,10 @@ describe('Service', () => { }); it('should add the User Agent', done => { - const userAgent = 'user-agent/0.0.0'; - - const getUserAgentFn = util.getUserAgentFromPackageJson; - util.getUserAgentFromPackageJson = packageJson => { - util.getUserAgentFromPackageJson = getUserAgentFn; - assert.strictEqual(packageJson, service.packageJson); - return userAgent; - }; - - service.makeAuthenticatedRequest = (reqOpts: DecorateRequestOptions) => { - assert.strictEqual(reqOpts.headers!['User-Agent'], userAgent); - done(); - }; - - service.request_(reqOpts, assert.ifError); - }); - - it('should add the provided User Agent', done => { - const userAgent = 'user-agent/0.0.0'; - const providedUserAgent = 'test'; - - service.providedUserAgent = providedUserAgent; - - const getUserAgentFn = util.getUserAgentFromPackageJson; - util.getUserAgentFromPackageJson = packageJson => { - util.getUserAgentFromPackageJson = getUserAgentFn; - assert.strictEqual(packageJson, service.packageJson); - return userAgent; - }; - service.makeAuthenticatedRequest = (reqOpts: DecorateRequestOptions) => { assert.strictEqual( reqOpts.headers!['User-Agent'], - `${providedUserAgent} ${userAgent}` + getUserAgentString() ); done(); }; diff --git a/test/nodejs-common/util.ts b/test/nodejs-common/util.ts index 1f59b1c40..f950f8a67 100644 --- a/test/nodejs-common/util.ts +++ b/test/nodejs-common/util.ts @@ -1869,17 +1869,6 @@ describe('common/util', () => { }); }); - describe('getUserAgentFromPackageJson', () => { - it('should format a User Agent string from a package.json', () => { - const userAgent = util.getUserAgentFromPackageJson({ - name: '@google-cloud/storage', - version: '0.1.0', - }); - - assert.strictEqual(userAgent, 'gcloud-node-storage/0.1.0'); - }); - }); - describe('maybeOptionsOrCallback', () => { it('should allow passing just a callback', () => { const optionsOrCallback = () => {}; diff --git a/test/resumable-upload.ts b/test/resumable-upload.ts index 326a63fc3..fa54a5af3 100644 --- a/test/resumable-upload.ts +++ b/test/resumable-upload.ts @@ -53,6 +53,7 @@ const CHUNK_SIZE_MULTIPLE = 2 ** 18; const queryPath = '/?userProject=user-project-id'; const X_GOOG_API_HEADER_REGEX = /^gl-node\/(?[^W]+) gccl\/(?[^W]+) gccl-invocation-id\/(?[^W]+) gccl-gcs-cmd\/(?[^W]+)$/; +const USER_AGENT_REGEX = /^gcloud-node-storage\/(?[^W]+)$/; function mockAuthorizeRequest( code = 200, @@ -1162,6 +1163,7 @@ describe('resumable-upload', () => { assert.ok( X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) ); + assert.ok(USER_AGENT_REGEX.test(reqOpts.headers['User-Agent'])); const data = await getAllDataFromRequest(); @@ -1178,6 +1180,7 @@ describe('resumable-upload', () => { assert.ok( X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) ); + assert.ok(USER_AGENT_REGEX.test(reqOpts.headers['User-Agent'])); const data = await getAllDataFromRequest(); @@ -1211,6 +1214,7 @@ describe('resumable-upload', () => { assert.ok( X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) ); + assert.ok(USER_AGENT_REGEX.test(reqOpts.headers['User-Agent'])); const data = await getAllDataFromRequest(); @@ -1242,6 +1246,7 @@ describe('resumable-upload', () => { assert.ok( X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) ); + assert.ok(USER_AGENT_REGEX.test(reqOpts.headers['User-Agent'])); const data = await getAllDataFromRequest(); @@ -1272,6 +1277,7 @@ describe('resumable-upload', () => { assert.ok( X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) ); + assert.ok(USER_AGENT_REGEX.test(reqOpts.headers['User-Agent'])); const data = await getAllDataFromRequest(); assert.equal(data.byteLength, CONTENT_LENGTH - NUM_BYTES_WRITTEN); @@ -1456,6 +1462,7 @@ describe('resumable-upload', () => { assert.ok( X_GOOG_API_HEADER_REGEX.test(reqOpts.headers['x-goog-api-client']) ); + assert.ok(USER_AGENT_REGEX.test(reqOpts.headers['User-Agent'])); done(); return {}; }; @@ -2242,6 +2249,7 @@ describe('resumable-upload', () => { request.opts.headers['x-goog-api-client'] ) ); + assert.ok(USER_AGENT_REGEX.test(request.opts.headers['User-Agent'])); done(); }); @@ -2413,6 +2421,9 @@ describe('resumable-upload', () => { request.opts.headers['x-goog-api-client'] ) ); + assert.ok( + USER_AGENT_REGEX.test(request.opts.headers['User-Agent']) + ); } else { // The preceding chunks const endByte = offset + CHUNK_SIZE - 1; @@ -2429,6 +2440,9 @@ describe('resumable-upload', () => { request.opts.headers['x-goog-api-client'] ) ); + assert.ok( + USER_AGENT_REGEX.test(request.opts.headers['User-Agent']) + ); } } @@ -2527,6 +2541,7 @@ describe('resumable-upload', () => { request.opts.headers['x-goog-api-client'] ) ); + assert.ok(USER_AGENT_REGEX.test(request.opts.headers['User-Agent'])); done(); }); diff --git a/test/transfer-manager.ts b/test/transfer-manager.ts index 66910831d..f473bdc69 100644 --- a/test/transfer-manager.ts +++ b/test/transfer-manager.ts @@ -443,6 +443,7 @@ describe('Transfer Manager', () => { const headersToAdd = { 'Content-Type': 'foo/bar', 'x-goog-meta-foo': 'foobar', + 'User-Agent': 'barfoo', }; mockGeneratorFunction = (bucket, fileName, uploadId, partsMap) => { @@ -497,7 +498,7 @@ describe('Transfer Manager', () => { }); it('should set the appropriate `GCCL_GCS_CMD_KEY`', async () => { - let called = true; + let called = false; class TestAuthClient extends AuthClient { async getAccessToken() { return {token: '', res: undefined}; @@ -536,5 +537,43 @@ describe('Transfer Manager', () => { assert(called); }); + + it('should set User-Agent correctly based on package.json', async () => { + let called = false; + class TestAuthClient extends AuthClient { + async getAccessToken() { + return {token: '', res: undefined}; + } + + async getRequestHeaders() { + return {}; + } + + async request(opts: GaxiosOptions) { + called = true; + + assert(opts.headers); + assert('User-Agent' in opts.headers); + assert.match(opts.headers['User-Agent'], /gcloud-node/); + + return { + data: Buffer.from( + ` + 1 + ` + ), + headers: {}, + } as GaxiosResponse; + } + } + + transferManager.bucket.storage.authClient = new GoogleAuth({ + authClient: new TestAuthClient(), + }); + + await transferManager.uploadFileInChunks(filePath); + + assert(called); + }); }); });