-
Notifications
You must be signed in to change notification settings - Fork 17
Handle unversioned OpenAI /responses paths in API proxy sidecar
#2080
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.
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
Changes from 3 commits
4e8f946
11a2ae9
aeb6845
d4ab433
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -149,10 +149,12 @@ function normalizeBasePath(rawPath) { | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Build the full upstream path by joining basePath, reqUrl's pathname, and query string. | ||||||||||||||||||||||||||
| * Applies provider-safe defaults and avoids duplicate prefixing when the incoming | ||||||||||||||||||||||||||
| * path already includes the configured base path. | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * Examples: | ||||||||||||||||||||||||||
| * buildUpstreamPath('/v1/chat/completions', 'api.openai.com', '') | ||||||||||||||||||||||||||
| * → '/v1/chat/completions' | ||||||||||||||||||||||||||
| * buildUpstreamPath('/responses', 'api.openai.com', '') | ||||||||||||||||||||||||||
| * → '/v1/responses' | ||||||||||||||||||||||||||
| * buildUpstreamPath('/v1/chat/completions', 'host.databricks.com', '/serving-endpoints') | ||||||||||||||||||||||||||
| * → '/serving-endpoints/v1/chat/completions' | ||||||||||||||||||||||||||
| * buildUpstreamPath('/v1/messages?stream=true', 'host.com', '/anthropic') | ||||||||||||||||||||||||||
|
|
@@ -165,8 +167,30 @@ function normalizeBasePath(rawPath) { | |||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| function buildUpstreamPath(reqUrl, targetHost, basePath) { | ||||||||||||||||||||||||||
| const targetUrl = new URL(reqUrl, `https://${targetHost}`); | ||||||||||||||||||||||||||
| const prefix = basePath === '/' ? '' : basePath; | ||||||||||||||||||||||||||
| return prefix + targetUrl.pathname + targetUrl.search; | ||||||||||||||||||||||||||
| const pathname = targetUrl.pathname; | ||||||||||||||||||||||||||
| let prefix = basePath === '/' ? '' : basePath; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // OpenAI's canonical API paths are versioned under /v1, while some newer | ||||||||||||||||||||||||||
| // clients (for example Codex CLI with OPENAI_BASE_URL pointing at the sidecar) | ||||||||||||||||||||||||||
| // send unversioned paths like /responses. Add /v1 only for the default | ||||||||||||||||||||||||||
| // OpenAI host when no explicit base path is configured. | ||||||||||||||||||||||||||
| if (!prefix) { | ||||||||||||||||||||||||||
| let normalizedTargetHost = targetHost; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| normalizedTargetHost = new URL(`https://${targetHost}`).hostname; | ||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||
| // Fall back to the raw host value if parsing fails. | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (normalizedTargetHost === 'api.openai.com') { | ||||||||||||||||||||||||||
| prefix = '/v1'; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| if (!prefix) { | |
| let normalizedTargetHost = targetHost; | |
| try { | |
| normalizedTargetHost = new URL(`https://${targetHost}`).hostname; | |
| } catch { | |
| // Fall back to the raw host value if parsing fails. | |
| } | |
| if (normalizedTargetHost === 'api.openai.com') { | |
| prefix = '/v1'; | |
| } | |
| if (!prefix && targetUrl.hostname === 'api.openai.com') { | |
| prefix = '/v1'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildUpstreamPath()currently accepts protocol-relative paths (e.g.//host/path). Becausenew URL()treats//...as a host override, this can drop the first path segment and produce surprising upstream paths. Consider rejectingreqUrlvalues that start with//(or normalizing them) before callingnew URL()to ensure only origin-form paths are forwarded.See below for a potential fix: