Skip to content

Commit 2a9fd5e

Browse files
CopilotlpcoxCopilot
authored
Refactor OpenAI BYOK base URL parsing to reuse shared proxy URL normalization (#4949)
* Initial plan * Refactor OpenAI BYOK URL parsing to reuse proxy-utils * fix: update JSDoc for normalizeApiTarget and rename test for clarity - JSDoc now documents that falsy values (including '') are returned as-is - Test name clarifies it covers path + query string, not just path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Landon Cox <landon.cox@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 55a764d commit 2a9fd5e

4 files changed

Lines changed: 91 additions & 37 deletions

File tree

containers/api-proxy/oidc-token-provider.test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,20 @@ describe('OpenAI adapter with OIDC', () => {
282282
expect(adapter.getReflectionInfo().auth_type).toBe('static-key');
283283
});
284284

285+
it('should not warn when COPILOT_PROVIDER_BASE_URL includes a path and query string', () => {
286+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
287+
try {
288+
createOpenAIAdapter({
289+
COPILOT_PROVIDER_TYPE: 'azure',
290+
COPILOT_PROVIDER_BASE_URL: 'https://my-resource.openai.azure.com/openai/deployments/gpt-5?api-version=2024-02-01',
291+
COPILOT_PROVIDER_API_KEY: 'azure-byok-key',
292+
});
293+
expect(warnSpy).not.toHaveBeenCalled();
294+
} finally {
295+
warnSpy.mockRestore();
296+
}
297+
});
298+
285299
it('should prefer explicit OPENAI_* config over Copilot Azure BYOK env vars', () => {
286300
const adapter = createOpenAIAdapter({
287301
OPENAI_API_KEY: 'sk-openai',

containers/api-proxy/providers/openai.js

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,10 @@
1010
* Base path: OPENAI_API_BASE_PATH (default: /v1 for the public endpoint)
1111
*/
1212

13-
const { normalizeBasePath, validateAuthHeaderEnv } = require('../proxy-utils');
13+
const { parseApiTargetAndBasePath, validateAuthHeaderEnv } = require('../proxy-utils');
1414
const { createBaseAdapterConfig, createAdapterMethods } = require('../adapter-factory');
1515
const { resolveCloudOidcProviders } = require('./cloud-oidc-init');
1616

17-
function parseByokBaseUrl(baseUrl) {
18-
if (!baseUrl) return { target: undefined, basePath: '' };
19-
const trimmed = baseUrl.trim();
20-
if (!trimmed) return { target: undefined, basePath: '' };
21-
22-
const candidate = /^[a-zA-Z][a-zA-Z\d+\-.]*:\/\//.test(trimmed)
23-
? trimmed
24-
: `https://${trimmed}`;
25-
try {
26-
const parsed = new URL(candidate);
27-
return {
28-
target: parsed.hostname || undefined,
29-
basePath: normalizeBasePath(parsed.pathname),
30-
};
31-
} catch {
32-
return { target: undefined, basePath: '' };
33-
}
34-
}
35-
3617
/**
3718
* Create the OpenAI provider adapter.
3819
*
@@ -58,7 +39,7 @@ function createOpenAIAdapter(env, deps = {}) {
5839
return '';
5940
})();
6041
const copilotByokApiKey = (env.COPILOT_PROVIDER_API_KEY || '').trim() || undefined;
61-
const { target: copilotByokTarget, basePath: copilotByokBasePath } = parseByokBaseUrl(env.COPILOT_PROVIDER_BASE_URL);
42+
const { target: copilotByokTarget, basePath: copilotByokBasePath } = parseApiTargetAndBasePath(env.COPILOT_PROVIDER_BASE_URL);
6243

6344
const apiKey = openaiApiKey || (copilotAzureByokEnabled ? copilotByokApiKey : undefined);
6445
const explicitOpenAITarget = env.OPENAI_API_TARGET ? openaiTarget : undefined;

containers/api-proxy/proxy-utils.js

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,34 +18,75 @@ const { URL } = require('url');
1818
* *_API_BASE_PATH environment variables.
1919
*
2020
* @param {string|undefined} value - Raw env var value
21-
* @returns {string|undefined} Bare hostname, or undefined if input is falsy
21+
* @returns {string|undefined} Bare hostname, the original falsy value if input is falsy (e.g. '' stays ''), or undefined if parsing fails
2222
*/
2323
function normalizeApiTarget(value) {
2424
if (!value) return value;
2525

26-
const trimmed = value.trim();
27-
if (!trimmed) return undefined;
28-
29-
const candidate = /^[a-zA-Z][a-zA-Z\d+\-.]*:\/\//.test(trimmed)
30-
? trimmed
31-
: `https://${trimmed}`;
32-
33-
try {
34-
const parsed = new URL(candidate);
26+
const parsed = parseApiTargetUrl(value);
27+
if (parsed.kind === 'empty') return undefined;
28+
if (parsed.kind === 'invalid') {
29+
console.warn(`Invalid API target ${parsed.safe}; expected a hostname (e.g. 'api.example.com') or URL`);
30+
return undefined;
31+
}
3532

33+
if (parsed.kind === 'ok') {
3634
if (parsed.pathname !== '/' || parsed.search || parsed.hash || parsed.username || parsed.password || parsed.port) {
37-
const safe = trimmed.replace(/[\x00-\x1f\x7f]/g, '?');
3835
console.warn(
39-
`Ignoring unsupported API target URL components in ${safe}; ` +
36+
`Ignoring unsupported API target URL components in ${parsed.safe}; ` +
4037
'configure path prefixes via the corresponding *_API_BASE_PATH environment variable.'
4138
);
4239
}
4340

4441
return parsed.hostname || undefined;
45-
} catch (err) {
46-
const safe = trimmed.replace(/[\x00-\x1f\x7f]/g, '?');
47-
console.warn(`Invalid API target ${safe}; expected a hostname (e.g. 'api.example.com') or URL`);
48-
return undefined;
42+
}
43+
44+
return undefined;
45+
}
46+
47+
/**
48+
* Parse a target URL/hostname into hostname + normalized pathname.
49+
* Intended for BYOK-style base URLs where path components are expected and valid.
50+
*
51+
* @param {string|undefined} value - Raw env var value
52+
* @returns {{ target: string|undefined, basePath: string }} Parsed hostname and normalized base path
53+
*/
54+
function parseApiTargetAndBasePath(value) {
55+
if (!value) return { target: undefined, basePath: '' };
56+
57+
const parsed = parseApiTargetUrl(value);
58+
if (parsed.kind !== 'ok') return { target: undefined, basePath: '' };
59+
60+
return {
61+
target: parsed.hostname || undefined,
62+
basePath: normalizeBasePath(parsed.pathname),
63+
};
64+
}
65+
66+
function parseApiTargetUrl(value) {
67+
const trimmed = value.trim();
68+
if (!trimmed) return { kind: 'empty' };
69+
70+
const candidate = /^[a-zA-Z][a-zA-Z\d+\-.]*:\/\//.test(trimmed)
71+
? trimmed
72+
: `https://${trimmed}`;
73+
const safe = trimmed.replace(/[\x00-\x1f\x7f]/g, '?');
74+
75+
try {
76+
const parsed = new URL(candidate);
77+
return {
78+
kind: 'ok',
79+
safe,
80+
hostname: parsed.hostname,
81+
pathname: parsed.pathname,
82+
search: parsed.search,
83+
hash: parsed.hash,
84+
username: parsed.username,
85+
password: parsed.password,
86+
port: parsed.port,
87+
};
88+
} catch {
89+
return { kind: 'invalid', safe };
4990
}
5091
}
5192

@@ -262,6 +303,7 @@ function validateAuthHeaderEnv(envVarName, rawValue, defaultHeader) {
262303

263304
module.exports = {
264305
normalizeApiTarget,
306+
parseApiTargetAndBasePath,
265307
normalizeBasePath,
266308
buildUpstreamPath,
267309
stripGeminiKeyParam,

containers/api-proxy/server.routing.test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
const {
88
normalizeApiTarget,
9+
parseApiTargetAndBasePath,
910
normalizeBasePath,
1011
buildUpstreamPath,
1112
makeProviderNotConfiguredResponse,
@@ -52,6 +53,22 @@ describe('normalizeApiTarget', () => {
5253
});
5354
});
5455

56+
describe('parseApiTargetAndBasePath', () => {
57+
it('extracts hostname and normalized path from full URL', () => {
58+
expect(parseApiTargetAndBasePath('https://my-gateway.example.com/openai/deployments/gpt-5'))
59+
.toEqual({ target: 'my-gateway.example.com', basePath: '/openai/deployments/gpt-5' });
60+
});
61+
62+
it('extracts hostname and path from schemeless URL-like value', () => {
63+
expect(parseApiTargetAndBasePath('my-gateway.example.com/openai/deployments/gpt-5'))
64+
.toEqual({ target: 'my-gateway.example.com', basePath: '/openai/deployments/gpt-5' });
65+
});
66+
67+
it('returns empty values for invalid input', () => {
68+
expect(parseApiTargetAndBasePath('://invalid')).toEqual({ target: undefined, basePath: '' });
69+
});
70+
});
71+
5572
describe('createAdapterMethods', () => {
5673
it('builds default validation, model-fetch, and reflection methods', () => {
5774
const methods = createAdapterMethods({

0 commit comments

Comments
 (0)