Fix custom OpenAI provider dropping port when URL scheme is omitted (#9730)
Signed-off-by: Michael Neale <michael.neale@gmail.com> Signed-off-by: Douwe M Osinga <douwe@sidewalklabs.com> Co-authored-by: Douwe M Osinga <douwe@sidewalklabs.com>
This commit is contained in:
@@ -313,6 +313,10 @@ impl ApiClient {
|
||||
})
|
||||
}
|
||||
|
||||
pub fn host(&self) -> &str {
|
||||
&self.host
|
||||
}
|
||||
|
||||
fn rebuild_client(&mut self) -> Result<()> {
|
||||
let mut client_builder = Client::builder()
|
||||
.timeout(self.timeout)
|
||||
|
||||
@@ -85,7 +85,37 @@ struct ParsedBaseUrl {
|
||||
from_base_url: bool,
|
||||
}
|
||||
|
||||
/// Ensure a base URL has an explicit scheme.
|
||||
///
|
||||
/// Users frequently enter hosts like `localhost:1234` without a scheme. The
|
||||
/// `url` crate parses such input as `scheme="localhost"`, `path="1234"`,
|
||||
/// silently dropping both the host and the port. When no `://` is present we
|
||||
/// prepend a sensible scheme (`http://` for local hosts, `https://`
|
||||
/// otherwise) so the host and port survive parsing.
|
||||
pub(crate) fn ensure_url_scheme(raw_url: &str) -> String {
|
||||
let trimmed = raw_url.trim();
|
||||
if trimmed.contains("://") {
|
||||
return trimmed.to_string();
|
||||
}
|
||||
|
||||
let host_part = trimmed.split(['/', '?']).next().unwrap_or(trimmed);
|
||||
let bare_host = if let Some(rest) = host_part.strip_prefix('[') {
|
||||
rest.split(']').next().unwrap_or(rest)
|
||||
} else {
|
||||
host_part.split(':').next().unwrap_or(host_part)
|
||||
};
|
||||
let is_local = bare_host == "localhost"
|
||||
|| bare_host == "127.0.0.1"
|
||||
|| bare_host == "0.0.0.0"
|
||||
|| bare_host == "::1";
|
||||
|
||||
let scheme = if is_local { "http" } else { "https" };
|
||||
format!("{}://{}", scheme, trimmed)
|
||||
}
|
||||
|
||||
pub(crate) fn parse_openai_base_url(raw_url: &str) -> Result<OpenAiBaseUrlParts> {
|
||||
let raw_url = ensure_url_scheme(raw_url);
|
||||
let raw_url = raw_url.as_str();
|
||||
let parsed = url::Url::parse(raw_url)
|
||||
.map_err(|e| anyhow::anyhow!("Invalid OPENAI_BASE_URL '{}': {}", raw_url, e))?;
|
||||
|
||||
@@ -397,7 +427,8 @@ impl OpenAiProvider {
|
||||
let global_config = crate::config::Config::global();
|
||||
let api_key = Self::resolve_api_key(&config, &|key| global_config.get_secret(key))?;
|
||||
|
||||
let url = url::Url::parse(&config.base_url)
|
||||
let normalized_base_url = ensure_url_scheme(&config.base_url);
|
||||
let url = url::Url::parse(&normalized_base_url)
|
||||
.map_err(|e| anyhow::anyhow!("Invalid base URL '{}': {}", config.base_url, e))?;
|
||||
|
||||
let host = if let Some(port) = url.port() {
|
||||
@@ -1421,6 +1452,51 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ensure_url_scheme_adds_http_for_local_hosts() {
|
||||
assert_eq!(ensure_url_scheme("localhost:1234"), "http://localhost:1234");
|
||||
assert_eq!(
|
||||
ensure_url_scheme("127.0.0.1:8080/v1"),
|
||||
"http://127.0.0.1:8080/v1"
|
||||
);
|
||||
assert_eq!(ensure_url_scheme("0.0.0.0:3000"), "http://0.0.0.0:3000");
|
||||
assert_eq!(ensure_url_scheme("[::1]:1234"), "http://[::1]:1234");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ensure_url_scheme_adds_https_for_remote_hosts() {
|
||||
assert_eq!(
|
||||
ensure_url_scheme("api.example.com:8443/v1"),
|
||||
"https://api.example.com:8443/v1"
|
||||
);
|
||||
assert_eq!(ensure_url_scheme("example.com"), "https://example.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ensure_url_scheme_preserves_existing_scheme() {
|
||||
assert_eq!(
|
||||
ensure_url_scheme("http://localhost:1234"),
|
||||
"http://localhost:1234"
|
||||
);
|
||||
assert_eq!(
|
||||
ensure_url_scheme("https://api.openai.com/v1"),
|
||||
"https://api.openai.com/v1"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_custom_config_preserves_port_without_scheme() {
|
||||
let mut config =
|
||||
base_declarative_config(vec![ModelInfo::new("m1".to_string(), 128000)], None);
|
||||
config.base_url = "localhost:1234".to_string();
|
||||
|
||||
let provider =
|
||||
OpenAiProvider::from_custom_config(ModelConfig::new_or_fail("m1"), config).unwrap();
|
||||
|
||||
assert_eq!(provider.api_client.host(), "http://localhost:1234");
|
||||
assert_eq!(provider.base_path, "v1/chat/completions");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn fetch_supported_models_static_only_skips_api() {
|
||||
// Any request to the mock returns 500 — if the fix calls the API, the test fails.
|
||||
|
||||
Reference in New Issue
Block a user