diff --git a/src/internal/uploads.ts b/src/internal/uploads.ts index 15073983b..2456be048 100644 --- a/src/internal/uploads.ts +++ b/src/internal/uploads.ts @@ -49,20 +49,35 @@ export function makeFile( return new File(fileBits as any, fileName ?? 'unknown_file', options); } -export function getName(value: any): string | undefined { - return ( - ( - (typeof value === 'object' && - value !== null && - (('name' in value && value.name && String(value.name)) || - ('url' in value && value.url && String(value.url)) || - ('filename' in value && value.filename && String(value.filename)) || - ('path' in value && value.path && String(value.path)))) || - '' - ) - .split(/[\\/]/) - .pop() || undefined - ); +function basename(name: string, source: 'url' | 'path' | 'explicit'): string | undefined { + if (source === 'url') { + try { + name = new URL(name).pathname; + } catch { + // Fall back to separator splitting for non-URL values. + } + } + return name.split(/[\\/]/).pop() || undefined; +} + +export function getName(value: any, opts?: { stripFilename?: boolean | undefined }): string | undefined { + if (typeof value !== 'object' || value === null) return undefined; + + const explicitName = + ('name' in value && value.name && String(value.name)) || + ('filename' in value && value.filename && String(value.filename)); + if (explicitName) { + if (opts?.stripFilename === false) return explicitName; + return basename(explicitName, 'explicit'); + } + + const urlName = 'url' in value && value.url && String(value.url); + if (urlName) return basename(urlName, 'url'); + + const pathName = 'path' in value && value.path && String(value.path); + if (pathName) return basename(pathName, 'path'); + + return undefined; } export const isAsyncIterable = (value: any): value is AsyncIterable => @@ -75,10 +90,11 @@ export const isAsyncIterable = (value: any): value is AsyncIterable => export const maybeMultipartFormRequestOptions = async ( opts: RequestOptions, fetch: OpenAI | Fetch, + formOptions?: CreateFormOptions, ): Promise => { if (!hasUploadableValue(opts.body)) return opts; - return { ...opts, body: await createForm(opts.body, fetch) }; + return { ...opts, body: await createForm(opts.body, fetch, formOptions) }; }; type MultipartFormRequestOptions = Omit & { body: unknown }; @@ -86,8 +102,9 @@ type MultipartFormRequestOptions = Omit & { body: unknow export const multipartFormRequestOptions = async ( opts: MultipartFormRequestOptions, fetch: OpenAI | Fetch, + formOptions?: CreateFormOptions, ): Promise => { - return { ...opts, body: await createForm(opts.body, fetch) }; + return { ...opts, body: await createForm(opts.body, fetch, formOptions) }; }; const supportsFormDataMap = /* @__PURE__ */ new WeakMap>(); @@ -122,9 +139,14 @@ function supportsFormData(fetchObject: OpenAI | Fetch): Promise { return promise; } +export type CreateFormOptions = { + stripFilenames?: boolean; +}; + export const createForm = async >( body: T | undefined, fetch: OpenAI | Fetch, + options: CreateFormOptions = {}, ): Promise => { if (!(await supportsFormData(fetch))) { throw new TypeError( @@ -132,7 +154,9 @@ export const createForm = async >( ); } const form = new FormData(); - await Promise.all(Object.entries(body || {}).map(([key, value]) => addFormValue(form, key, value))); + await Promise.all( + Object.entries(body || {}).map(([key, value]) => addFormValue(form, key, value, options)), + ); return form; }; @@ -156,7 +180,12 @@ const hasUploadableValue = (value: unknown): boolean => { return false; }; -const addFormValue = async (form: FormData, key: string, value: unknown): Promise => { +const addFormValue = async ( + form: FormData, + key: string, + value: unknown, + options: CreateFormOptions, +): Promise => { if (value === undefined) return; if (value == null) { throw new TypeError( @@ -168,16 +197,25 @@ const addFormValue = async (form: FormData, key: string, value: unknown): Promis if (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') { form.append(key, String(value)); } else if (value instanceof Response) { - form.append(key, makeFile([await value.blob()], getName(value))); + form.append( + key, + makeFile([await value.blob()], getName(value, { stripFilename: options.stripFilenames })), + ); } else if (isAsyncIterable(value)) { - form.append(key, makeFile([await new Response(ReadableStreamFrom(value)).blob()], getName(value))); + form.append( + key, + makeFile( + [await new Response(ReadableStreamFrom(value)).blob()], + getName(value, { stripFilename: options.stripFilenames }), + ), + ); } else if (isNamedBlob(value)) { - form.append(key, value, getName(value)); + form.append(key, value, getName(value, { stripFilename: options.stripFilenames })); } else if (Array.isArray(value)) { - await Promise.all(value.map((entry) => addFormValue(form, key + '[]', entry))); + await Promise.all(value.map((entry) => addFormValue(form, key + '[]', entry, options))); } else if (typeof value === 'object') { await Promise.all( - Object.entries(value).map(([name, prop]) => addFormValue(form, `${key}[${name}]`, prop)), + Object.entries(value).map(([name, prop]) => addFormValue(form, `${key}[${name}]`, prop, options)), ); } else { throw new TypeError( diff --git a/src/resources/skills/skills.ts b/src/resources/skills/skills.ts index c7d9ff89c..bcf798474 100644 --- a/src/resources/skills/skills.ts +++ b/src/resources/skills/skills.ts @@ -30,7 +30,10 @@ export class Skills extends APIResource { * Create a new skill. */ create(body: SkillCreateParams | null | undefined = {}, options?: RequestOptions): APIPromise { - return this._client.post('/skills', maybeMultipartFormRequestOptions({ body, ...options }, this._client)); + return this._client.post( + '/skills', + maybeMultipartFormRequestOptions({ body, ...options }, this._client, { stripFilenames: false }), + ); } /** diff --git a/src/resources/skills/versions/versions.ts b/src/resources/skills/versions/versions.ts index 19ab483f4..21ea47241 100644 --- a/src/resources/skills/versions/versions.ts +++ b/src/resources/skills/versions/versions.ts @@ -23,7 +23,7 @@ export class Versions extends APIResource { ): APIPromise { return this._client.post( path`/skills/${skillID}/versions`, - maybeMultipartFormRequestOptions({ body, ...options }, this._client), + maybeMultipartFormRequestOptions({ body, ...options }, this._client, { stripFilenames: false }), ); } diff --git a/tests/form.test.ts b/tests/form.test.ts index 08cae4da0..db1edc010 100644 --- a/tests/form.test.ts +++ b/tests/form.test.ts @@ -1,5 +1,6 @@ import { multipartFormRequestOptions, createForm } from 'openai/internal/uploads'; import { toFile } from 'openai/core/uploads'; +import fs from 'fs'; describe('form data validation', () => { test('valid values do not error', async () => { @@ -82,4 +83,55 @@ describe('form data validation', () => { ); expect(Array.from(form2.entries())).toEqual([['bar[]', 'foo']]); }); + + test('file names strip path separators by default', async () => { + const form = await createForm( + { + file: new File(['Some content'], 'my-skill/SKILL.md'), + }, + fetch, + ); + + expect((form.get('file') as File).name).toBe('SKILL.md'); + }); + + test('file names can preserve path separators for APIs that require directories', async () => { + const form = await createForm( + { + files: [new File(['Some content'], 'my-skill/SKILL.md')], + }, + fetch, + { stripFilenames: false }, + ); + + expect((form.get('files[]') as File).name).toBe('my-skill/SKILL.md'); + }); + + test('path-preserving mode still strips inferred Response URL filenames', async () => { + class MockResponse extends Response { + override url = 'https://example.com/my-skill/SKILL.md'; + } + + const form = await createForm( + { + files: [new MockResponse('Some content', { status: 200 })], + }, + fetch, + { stripFilenames: false }, + ); + + expect((form.get('files[]') as File).name).toBe('SKILL.md'); + }); + + test('path-preserving mode still strips inferred ReadStream paths', async () => { + const form = await createForm( + { + files: [fs.createReadStream('tests/uploads.test.ts')], + }, + fetch, + { stripFilenames: false }, + ); + + expect((form.get('files[]') as File).name).toBe('uploads.test.ts'); + }); });