Skip to content

fix(googleapis-common): validate and encode URI path parameters #9162

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
danieljbruce wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(googleapis-common): validate and encode URI path parameters #9162

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
69 changes: 67 additions & 2 deletions core/packages/nodejs-googleapis-common/src/apirequest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,38 @@ function isReadableStream(obj: any) {
);
}

export function validateUriPathSegment(
propertyName: string,
value: string,
): void {
if (value === '.' || value === '..') {
throw new Error(`Invalid value ${value} for ${propertyName}`);
}
}

export function validateUriPath(propertyName: string, value: string): void {
const segments = value.split('/');
for (const segment of segments) {
if (segment === '.' || segment === '..') {
throw new Error(
`Value for ${propertyName} must not contain segments that are exactly . or ..`,
);
}
}
}

export function encodeWithSlashes(str: string): string {
return [...str]
.map(c => (c.match(/[-_.~0-9a-zA-Z]/) ? c : encodeURIComponent(c)))
.join('');
}

export function encodeWithoutSlashes(str: string): string {
return [...str]
.map(c => (c.match(/[-_.~0-9a-zA-Z/]/) ? c : encodeURIComponent(c)))
.join('');
}
Comment on lines +71 to +81

@gemini-code-assist gemini-code-assist Bot Aug 18, 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.

medium

The helper functions encodeWithSlashes and encodeWithoutSlashes can be significantly simplified and optimized:

  1. encodeWithSlashes is functionally identical to the built-in encodeURIComponent because all characters matched by the regex /[-_.~0-9a-zA-Z]/ are also preserved by encodeURIComponent, and any other characters (including !, *, ', (, )) are delegated to encodeURIComponent anyway. We can simply return encodeURIComponent(str) directly.
  2. encodeWithoutSlashes can be implemented much more efficiently by splitting the string by /, mapping each segment with encodeURIComponent, and joining them back with /. This avoids the overhead of spreading the string into a character array, executing a regex match on every single character, and joining them back.

This improves both readability and performance. Note that we pass encodeURIComponent directly to map instead of wrapping it in an arrow function (e.g., map(val => encodeURIComponent(val))) to avoid unnecessary closure allocations.

export function encodeWithSlashes(str: string): string {
  return encodeURIComponent(str);
}

export function encodeWithoutSlashes(str: string): string {
  return str.split('/').map(encodeURIComponent).join('/');
}
References
  1. Avoid wrapping methods in arrow functions for default cases to prevent unnecessary closure allocations and extra call stack frames.


function getMissingParams(params: SchemaParameters, required: string[]) {
const missing = new Array<string>();
required.forEach(param => {
Expand Down Expand Up @@ -165,15 +197,48 @@ async function createAPIRequestAsync<T>(
}

// Parse urls
const processedParams = Object.assign({}, params);
if (parameters.pathParams && parameters.pathParams.length > 0) {
const urlStr =
typeof options.url === 'object'
? options.url.toString()
: options.url || '';
const mediaUrlStr = parameters.mediaUrl || '';
for (const param of parameters.pathParams) {
if (
processedParams[param] !== undefined &&
processedParams[param] !== null
) {
const valStr = String(processedParams[param]);
const isReservedInUrl = new RegExp(
`\\{\\+[^}]*\\b${param}\\b[^}]*\\}`,
).test(urlStr);
const isReservedInMediaUrl = new RegExp(
`\\{\\+[^}]*\\b${param}\\b[^}]*\\}`,
).test(mediaUrlStr);
const isReserved = isReservedInUrl || isReservedInMediaUrl;
if (isReserved) {
validateUriPath(param, valStr);
processedParams[param] = encodeWithoutSlashes(valStr);
} else {
validateUriPathSegment(param, valStr);
// Standard expressions ({param}) are automatically percent-encoded by url-template.
// We only need validation here so url-template does not double-encode (%25...).
}
}
}
}

if (options.url) {
let url = options.url;
if (typeof url === 'object') {
url = url.toString();
}
options.url = urlTemplate.parse(url).expand(params);
options.url = urlTemplate.parse(url).expand(processedParams);
}
if (parameters.mediaUrl) {
parameters.mediaUrl = urlTemplate.parse(parameters.mediaUrl).expand(params);
parameters.mediaUrl =
urlTemplate.parse(parameters.mediaUrl).expand(processedParams);
}

// Rewrite url if rootUrl is globally set
Expand Down
107 changes: 106 additions & 1 deletion core/packages/nodejs-googleapis-common/test/test.apirequest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@ import {URL} from 'url';
import * as sinon from 'sinon';

import {GlobalOptions, MethodOptions} from '../src/api';
import {createAPIRequest} from '../src/apirequest';
import {
createAPIRequest,
validateUriPathSegment,
validateUriPath,
encodeWithSlashes,
encodeWithoutSlashes,
} from '../src/apirequest';
import {GoogleAuth} from 'google-auth-library';
import {GaxiosResponse} from 'gaxios';

Expand Down Expand Up @@ -503,6 +509,105 @@ describe('createAPIRequest', () => {
});
});

describe('URI path validation and encoding helpers', () => {
it('validateUriPathSegment should reject . and ..', () => {
assert.throws(
() => validateUriPathSegment('name', '.'),
/Invalid value \. for name/,
);
assert.throws(
() => validateUriPathSegment('name', '..'),
/Invalid value \.\. for name/,
);
assert.doesNotThrow(() => validateUriPathSegment('name', 'valid'));
assert.doesNotThrow(() => validateUriPathSegment('name', 'foo/bar'));
});

it('validateUriPath should reject segments that are . or ..', () => {
assert.throws(
() => validateUriPath('name', '.'),
/Value for name must not contain segments that are exactly \. or \.\./,
);
assert.throws(
() => validateUriPath('name', '..'),
/Value for name must not contain segments that are exactly \. or \.\./,
);
assert.throws(
() => validateUriPath('name', 'foo/./bar'),
/Value for name must not contain segments that are exactly \. or \.\./,
);
assert.throws(
() => validateUriPath('name', 'foo/../bar'),
/Value for name must not contain segments that are exactly \. or \.\./,
);
assert.doesNotThrow(() => validateUriPath('name', 'foo/bar'));
assert.doesNotThrow(() => validateUriPath('name', 'foo..bar'));
});

it('encodeWithSlashes should encode slashes and reserved characters', () => {
assert.strictEqual(encodeWithSlashes('foo/bar'), 'foo%2Fbar');
assert.strictEqual(encodeWithSlashes('a?b#c'), 'a%3Fb%23c');
});

it('encodeWithoutSlashes should preserve slashes but encode reserved characters', () => {
assert.strictEqual(encodeWithoutSlashes('foo/bar'), 'foo/bar');
assert.strictEqual(encodeWithoutSlashes('a?b#c'), 'a%3Fb%23c');
});

it('should validate standard path params ({param}) against path traversal', async () => {
await assert.rejects(
createAPIRequest<FakeParams>({
options: {url: `${url}/projects/{projectId}`},
params: {projectId: '.'},
requiredParams: [],
pathParams: ['projectId'],
context: fakeContext,
}),
/Invalid value \. for projectId/,
);

await assert.rejects(
createAPIRequest<FakeParams>({
options: {url: `${url}/projects/{projectId}`},
params: {projectId: '..'},
requiredParams: [],
pathParams: ['projectId'],
context: fakeContext,
}),
/Invalid value \.\. for projectId/,
);
});

it('should validate reserved path params ({+param}) against path traversal and encode non-slash characters', async () => {
await assert.rejects(
createAPIRequest<FakeParams>({
options: {url: `${url}/v1/{+name}`},
params: {name: 'projects/../locations'},
requiredParams: [],
pathParams: ['name'],
context: fakeContext,
}),
/Value for name must not contain segments that are exactly \. or \.\./,
);

const scope = nock(url)
.get('/v1/projects/p1/locations%3Ffoo%3Dbar')
.reply(200, fakeResponse);
const res = await createAPIRequest<FakeParams>({
options: {url: `${url}/v1/{+name}`},
params: {name: 'projects/p1/locations?foo=bar'},
requiredParams: [],
pathParams: ['name'],
context: fakeContext,
});
scope.done();
assert.strictEqual(
res.config.url.toString(),
`${url}/v1/projects/p1/locations%3Ffoo%3Dbar`,
);
});
});

describe('TPC', () => {
it('should allow setting universeDomain', async () => {
const gduUrl = 'https://api.googleapis.com/path?param=value#extra';
Expand Down
Loading
Loading