diff --git a/docs/public-api/apify-client.api.md b/docs/public-api/apify-client.api.md index 5cbc2ddfe..ce3769c33 100644 --- a/docs/public-api/apify-client.api.md +++ b/docs/public-api/apify-client.api.md @@ -27,6 +27,7 @@ import type { STORAGE_GENERAL_ACCESS } from '@apify/consts'; import { STORAGE_OWNERSHIP_FILTER } from '@apify/consts'; import type { ValueOf } from 'type-fest'; import type { WEBHOOK_EVENT_TYPES } from '@apify/consts'; +import type { z } from 'zod'; // @public export interface AccountAndUsageLimits { @@ -611,7 +612,7 @@ abstract class ApiClient { protected _params(endpointParams?: T): Record; publicBaseUrl: string; // (undocumented) - protected _publicUrl(path?: string): string; + protected _publicUrl(path?: string | string[]): string; // (undocumented) resourcePath: string; // (undocumented) @@ -623,7 +624,7 @@ abstract class ApiClient { // (undocumented) url: string; // (undocumented) - protected _url(path?: string): string; + protected _url(path?: string | string[]): string; } // Not exported by the entry point; reachable only as a referenced type. @@ -743,6 +744,12 @@ interface ApifyResponse extends AxiosResponse { config: ApifyRequestConfig & InternalAxiosRequestConfig; } +// @public +export class ArgumentValidationError extends Error { + constructor(error: z.ZodError, value: unknown, label?: string); + readonly issues: z.ZodError['issues']; +} + // @public (undocumented) export interface BaseActorVersion { // (undocumented) @@ -998,13 +1005,13 @@ export class DatasetClient = Record { // (undocumented) expiresInSecs?: number; } // @public -export interface DatasetClientDownloadItemsOptions extends DatasetClientListItemOptions { +export interface DatasetClientDownloadItemsOptions extends Omit { // (undocumented) attachment?: boolean; // (undocumented) @@ -1256,7 +1263,7 @@ export class InvalidResponseBodyError extends Error { } // @public -export interface KeyValueClientCreateKeysUrlOptions extends KeyValueClientListKeysOptions { +export interface KeyValueClientCreateKeysUrlOptions extends Omit { // (undocumented) expiresInSecs?: number; } diff --git a/src/base/api_client.ts b/src/base/api_client.ts index 00f03e165..4acf1816f 100644 --- a/src/base/api_client.ts +++ b/src/base/api_client.ts @@ -1,6 +1,7 @@ import type { ApifyClient } from '../apify_client'; import type { HttpClient } from '../http_client'; import type { PaginatedResponse, PaginationOptions } from '../utils'; +import { toPath, toPathSegment } from '../utils'; /** @private */ export interface ApiClientOptions { @@ -50,7 +51,7 @@ export abstract class ApiClient { this.baseUrl = baseUrl; this.publicBaseUrl = publicBaseUrl; this.resourcePath = resourcePath; - this.url = id ? `${baseUrl}/${resourcePath}/${this.safeId}` : `${baseUrl}/${resourcePath}`; + this.url = id ? `${baseUrl}/${resourcePath}/${toPathSegment(this.safeId!)}` : `${baseUrl}/${resourcePath}`; this.apifyClient = apifyClient; this.httpClient = httpClient; this.params = params; @@ -67,15 +68,15 @@ export abstract class ApiClient { return { ...baseOptions, ...moreOptions } as BaseOptions & T; } - protected _url(path?: string): string { - return path ? `${this.url}/${path}` : this.url; + protected _url(path?: string | string[]): string { + return path ? `${this.url}/${toPath(path)}` : this.url; } - protected _publicUrl(path?: string): string { + protected _publicUrl(path?: string | string[]): string { const url = this.id - ? `${this.publicBaseUrl}/${this.resourcePath}/${this.safeId}` + ? `${this.publicBaseUrl}/${this.resourcePath}/${toPathSegment(this.safeId!)}` : `${this.publicBaseUrl}/${this.resourcePath}`; - return path ? `${url}/${path}` : url; + return path ? `${url}/${toPath(path)}` : url; } protected _params(endpointParams?: T): Record { @@ -83,8 +84,7 @@ export abstract class ApiClient { } protected _toSafeId(id: string): string { - // The id has the format `username/actor-name`, so we only need to replace the first `/`. - return id.replace('/', '~'); + return id.replaceAll('/', '~'); } /** diff --git a/src/resource_clients/key_value_store.ts b/src/resource_clients/key_value_store.ts index cfd0bc183..c73db249c 100644 --- a/src/resource_clients/key_value_store.ts +++ b/src/resource_clients/key_value_store.ts @@ -254,7 +254,7 @@ export class KeyValueStoreClient extends ResourceClient { const store = await this.get(); - const recordPublicUrl = new URL(this._publicUrl(`records/${key}`)); + const recordPublicUrl = new URL(this._publicUrl(['records', key])); if (store?.urlSigningSecretKey) { const signature = await createHmacSignatureAsync(store.urlSigningSecretKey, key); @@ -330,7 +330,7 @@ export class KeyValueStoreClient extends ResourceClient { */ async recordExists(key: string): Promise { const requestOpts: Record = { - url: this._url(`records/${key}`), + url: this._url(['records', key]), method: 'HEAD', params: this._params(), }; @@ -384,7 +384,7 @@ export class KeyValueStoreClient extends ResourceClient { if (parsed.signature) queryParams.signature = parsed.signature; const requestOpts: Record = { - url: this._url(`records/${key}`), + url: this._url(['records', key]), method: 'GET', params: this._params(queryParams), timeout: DEFAULT_TIMEOUT_MILLIS, @@ -481,7 +481,7 @@ export class KeyValueStoreClient extends ResourceClient { } const uploadOpts: ApifyRequestConfig = { - url: this._url(`records/${key}`), + url: this._url(['records', key]), method: 'PUT', params: this._params(), data: value, @@ -508,7 +508,7 @@ export class KeyValueStoreClient extends ResourceClient { parseArgument(key, keySchema); await this.httpClient.call({ - url: this._url(`records/${key}`), + url: this._url(['records', key]), method: 'DELETE', params: this._params(), timeout: SMALL_TIMEOUT_MILLIS, diff --git a/src/resource_clients/request_queue.ts b/src/resource_clients/request_queue.ts index 09817a4be..19ace00cf 100644 --- a/src/resource_clients/request_queue.ts +++ b/src/resource_clients/request_queue.ts @@ -518,7 +518,7 @@ export class RequestQueueClient extends ResourceClient { async getRequest(id: string): Promise { parseArgument(id, requestIdSchema); const requestOpts: ApifyRequestConfig = { - url: this._url(`requests/${id}`), + url: this._url(['requests', id]), method: 'GET', timeout: Math.min(SMALL_TIMEOUT_MILLIS, this.timeoutMillis ?? Infinity), params: this._params(), @@ -549,7 +549,7 @@ export class RequestQueueClient extends ResourceClient { const parsed = parseArgument(options, forefrontOptionsSchema, 'RequestQueueClientAddRequestOptions'); const response = await this.httpClient.call({ - url: this._url(`requests/${request.id}`), + url: this._url(['requests', request.id]), method: 'PUT', timeout: Math.min(MEDIUM_TIMEOUT_MILLIS, this.timeoutMillis ?? Infinity), data: request, @@ -571,7 +571,7 @@ export class RequestQueueClient extends ResourceClient { parseArgument(id, requestIdSchema); await this.httpClient.call({ - url: this._url(`requests/${id}`), + url: this._url(['requests', id]), method: 'DELETE', timeout: Math.min(SMALL_TIMEOUT_MILLIS, this.timeoutMillis ?? Infinity), params: this._params({ @@ -617,7 +617,7 @@ export class RequestQueueClient extends ResourceClient { ); const response = await this.httpClient.call({ - url: this._url(`requests/${id}/lock`), + url: this._url(['requests', id, 'lock']), method: 'PUT', timeout: Math.min(MEDIUM_TIMEOUT_MILLIS, this.timeoutMillis ?? Infinity), params: this._params({ @@ -646,7 +646,7 @@ export class RequestQueueClient extends ResourceClient { const parsed = parseArgument(options, forefrontOptionsSchema, 'RequestQueueClientDeleteRequestLockOptions'); await this.httpClient.call({ - url: this._url(`requests/${id}/lock`), + url: this._url(['requests', id, 'lock']), method: 'DELETE', timeout: Math.min(SMALL_TIMEOUT_MILLIS, this.timeoutMillis ?? Infinity), params: this._params({ diff --git a/src/utils.ts b/src/utils.ts index 602c06884..5cbc2b90c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -472,3 +472,27 @@ export const mutuallyExclusive = (...keys: (keyof T & string)[ (value) => keys.filter((key) => typeof value[key] !== 'undefined').length <= 1, `At most one of the following fields is allowed: ${keys.join(', ')}`, ]; + +const pathSegmentSchema = z + .string() + .nonempty() + .refine((v) => v !== '.' && v !== '..', { error: 'URL path must not contain dot segments' }); + +/** + * Percent-encodes a caller-supplied URL path segment so it cannot restructure the request path. + * + * Empty strings and dot segments are rejected instead of encoded, because URL parsers resolve + * dot segments after decoding - `records/%2E%2E` collapses to the parent endpoint just like `records/..`. + */ +export function toPathSegment(value: string): string { + const valueParsed = parseArgument(value, pathSegmentSchema); + return encodeURIComponent(valueParsed); +} + +/** + * Builds a URL path from segments. A plain string is used as-is, so literal paths such as + * `requests/batch` keep their separators; an array has each of its segments encoded individually. + */ +export function toPath(path: string | string[]): string { + return Array.isArray(path) ? path.map(toPathSegment).join('/') : path; +} diff --git a/test/url_path_encoding.test.ts b/test/url_path_encoding.test.ts new file mode 100644 index 000000000..4f1b6bb34 --- /dev/null +++ b/test/url_path_encoding.test.ts @@ -0,0 +1,63 @@ +import http from 'node:http'; +import type { AddressInfo } from 'node:net'; + +import { ApifyClient } from 'apify-client'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +describe('URL path encoding', () => { + let server: http.Server; + let client: ApifyClient; + let lastPath: string | undefined; + + beforeAll(async () => { + server = http.createServer((req, res) => { + lastPath = req.url; + res.setHeader('content-type', 'application/json'); + res.end(JSON.stringify({ data: {} })); + }); + await new Promise((resolve) => server.listen(0, resolve)); + client = new ApifyClient({ + baseUrl: `http://localhost:${(server.address() as AddressInfo).port}`, + token: 'token', + maxRetries: 0, + }); + }); + + afterAll(async () => { + await new Promise((resolve) => server.close(resolve)); + }); + + it('encodes record keys instead of letting them restructure the path', async () => { + await client.keyValueStore('STORE').getRecord('../../actor-runs/VICTIM/abort'); + expect(lastPath).toBe( + '/v2/key-value-stores/STORE/records/..%2F..%2Factor-runs%2FVICTIM%2Fabort?attachment=true', + ); + + await client.keyValueStore('STORE').getRecord('foo?injected=1'); + expect(lastPath).toBe('/v2/key-value-stores/STORE/records/foo%3Finjected%3D1?attachment=true'); + + await client.keyValueStore('STORE').getRecord('foo#frag'); + expect(lastPath).toBe('/v2/key-value-stores/STORE/records/foo%23frag?attachment=true'); + }); + + it('encodes request ids', async () => { + await client.requestQueue('QUEUE').getRequest('../../../datasets/V/items'); + expect(lastPath).toBe('/v2/request-queues/QUEUE/requests/..%2F..%2F..%2Fdatasets%2FV%2Fitems'); + }); + + it('keeps literal path separators', async () => { + await client.requestQueue('QUEUE').prolongRequestLock('abc', { lockSecs: 1 }); + expect(lastPath).toBe('/v2/request-queues/QUEUE/requests/abc/lock?lockSecs=1'); + }); + + it('replaces every slash in a resource id', async () => { + await client.actor('user/name/extra').get(); + expect(lastPath).toBe('/v2/actors/user~name~extra'); + }); + + it('rejects dot segments and empty values', async () => { + await expect(client.keyValueStore('STORE').getRecord('..')).rejects.toThrow(/must not contain dot segments/); + await expect(client.requestQueue('QUEUE').getRequest('.')).rejects.toThrow(/must not contain dot segments/); + expect(() => client.dataset('..')).toThrow(/must not contain dot segments/); + }); +});