Skip to content

Commit 248a505

Browse files
committed
fix: normalize Ollama URLs to handle trailing slashes
- Added url-normalization utility to remove trailing slashes from base URLs - Updated Ollama provider, embedder, and fetcher to use URL normalization - This prevents double slashes in API endpoints when users include trailing slashes - Added comprehensive tests for URL normalization utility Fixes #6078
1 parent 5629199 commit 248a505

File tree

5 files changed

+122
-6
lines changed

5 files changed

+122
-6
lines changed

src/api/providers/fetchers/ollama.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import axios from "axios"
22
import { ModelInfo, ollamaDefaultModelInfo } from "@roo-code/types"
33
import { z } from "zod"
4+
import { joinUrlPath } from "../../../utils/url-normalization"
45

56
const OllamaModelDetailsSchema = z.object({
67
family: z.string(),
@@ -65,15 +66,15 @@ export async function getOllamaModels(baseUrl = "http://localhost:11434"): Promi
6566
return models
6667
}
6768

68-
const response = await axios.get<OllamaModelsResponse>(`${baseUrl}/api/tags`)
69+
const response = await axios.get<OllamaModelsResponse>(joinUrlPath(baseUrl, "/api/tags"))
6970
const parsedResponse = OllamaModelsResponseSchema.safeParse(response.data)
7071
let modelInfoPromises = []
7172

7273
if (parsedResponse.success) {
7374
for (const ollamaModel of parsedResponse.data.models) {
7475
modelInfoPromises.push(
7576
axios
76-
.post<OllamaModelInfoResponse>(`${baseUrl}/api/show`, {
77+
.post<OllamaModelInfoResponse>(joinUrlPath(baseUrl, "/api/show"), {
7778
model: ollamaModel.model,
7879
})
7980
.then((ollamaModelInfo) => {

src/api/providers/ollama.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { type ModelInfo, openAiModelInfoSaneDefaults, DEEP_SEEK_DEFAULT_TEMPERAT
66
import type { ApiHandlerOptions } from "../../shared/api"
77

88
import { XmlMatcher } from "../../utils/xml-matcher"
9+
import { joinUrlPath } from "../../utils/url-normalization"
910

1011
import { convertToOpenAiMessages } from "../transform/openai-format"
1112
import { convertToR1Format } from "../transform/r1-format"
@@ -23,8 +24,9 @@ export class OllamaHandler extends BaseProvider implements SingleCompletionHandl
2324
constructor(options: ApiHandlerOptions) {
2425
super()
2526
this.options = options
27+
const baseUrl = this.options.ollamaBaseUrl || "http://localhost:11434"
2628
this.client = new OpenAI({
27-
baseURL: (this.options.ollamaBaseUrl || "http://localhost:11434") + "/v1",
29+
baseURL: joinUrlPath(baseUrl, "/v1"),
2830
apiKey: "ollama",
2931
})
3032
}

src/services/code-index/embedders/ollama.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { t } from "../../../i18n"
66
import { withValidationErrorHandling, sanitizeErrorMessage } from "../shared/validation-helpers"
77
import { TelemetryService } from "@roo-code/telemetry"
88
import { TelemetryEventName } from "@roo-code/types"
9+
import { joinUrlPath } from "../../../utils/url-normalization"
910

1011
// Timeout constants for Ollama API requests
1112
const OLLAMA_EMBEDDING_TIMEOUT_MS = 60000 // 60 seconds for embedding requests
@@ -32,7 +33,7 @@ export class CodeIndexOllamaEmbedder implements IEmbedder {
3233
*/
3334
async createEmbeddings(texts: string[], model?: string): Promise<EmbeddingResponse> {
3435
const modelToUse = model || this.defaultModelId
35-
const url = `${this.baseUrl}/api/embed` // Endpoint as specified
36+
const url = joinUrlPath(this.baseUrl, "/api/embed") // Endpoint as specified
3637

3738
// Apply model-specific query prefix if required
3839
const queryPrefix = getModelQueryPrefix("ollama", modelToUse)
@@ -140,7 +141,7 @@ export class CodeIndexOllamaEmbedder implements IEmbedder {
140141
return withValidationErrorHandling(
141142
async () => {
142143
// First check if Ollama service is running by trying to list models
143-
const modelsUrl = `${this.baseUrl}/api/tags`
144+
const modelsUrl = joinUrlPath(this.baseUrl, "/api/tags")
144145

145146
// Add timeout to prevent indefinite hanging
146147
const controller = new AbortController()
@@ -197,7 +198,7 @@ export class CodeIndexOllamaEmbedder implements IEmbedder {
197198
}
198199

199200
// Try a test embedding to ensure the model works for embeddings
200-
const testUrl = `${this.baseUrl}/api/embed`
201+
const testUrl = joinUrlPath(this.baseUrl, "/api/embed")
201202

202203
// Add timeout for test request too
203204
const testController = new AbortController()
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { describe, it, expect } from "vitest"
2+
import { normalizeBaseUrl, joinUrlPath } from "../url-normalization"
3+
4+
describe("url-normalization", () => {
5+
describe("normalizeBaseUrl", () => {
6+
it("should remove a single trailing slash", () => {
7+
expect(normalizeBaseUrl("http://localhost:11434/")).toBe("http://localhost:11434")
8+
})
9+
10+
it("should remove multiple trailing slashes", () => {
11+
expect(normalizeBaseUrl("http://localhost:11434//")).toBe("http://localhost:11434")
12+
expect(normalizeBaseUrl("http://localhost:11434///")).toBe("http://localhost:11434")
13+
})
14+
15+
it("should not modify URLs without trailing slashes", () => {
16+
expect(normalizeBaseUrl("http://localhost:11434")).toBe("http://localhost:11434")
17+
})
18+
19+
it("should handle URLs with paths", () => {
20+
expect(normalizeBaseUrl("http://localhost:11434/api/")).toBe("http://localhost:11434/api")
21+
expect(normalizeBaseUrl("http://localhost:11434/api/v1/")).toBe("http://localhost:11434/api/v1")
22+
})
23+
24+
it("should handle URLs with query parameters", () => {
25+
expect(normalizeBaseUrl("http://localhost:11434/?key=value")).toBe("http://localhost:11434/?key=value")
26+
expect(normalizeBaseUrl("http://localhost:11434/api/?key=value")).toBe(
27+
"http://localhost:11434/api/?key=value",
28+
)
29+
})
30+
31+
it("should handle empty strings", () => {
32+
expect(normalizeBaseUrl("")).toBe("")
33+
})
34+
35+
it("should handle URLs with ports", () => {
36+
expect(normalizeBaseUrl("http://localhost:8080/")).toBe("http://localhost:8080")
37+
})
38+
39+
it("should handle HTTPS URLs", () => {
40+
expect(normalizeBaseUrl("https://api.example.com/")).toBe("https://api.example.com")
41+
})
42+
})
43+
44+
describe("joinUrlPath", () => {
45+
it("should join base URL with path correctly", () => {
46+
expect(joinUrlPath("http://localhost:11434", "/api/tags")).toBe("http://localhost:11434/api/tags")
47+
})
48+
49+
it("should handle base URL with trailing slash", () => {
50+
expect(joinUrlPath("http://localhost:11434/", "/api/tags")).toBe("http://localhost:11434/api/tags")
51+
})
52+
53+
it("should handle base URL with multiple trailing slashes", () => {
54+
expect(joinUrlPath("http://localhost:11434//", "/api/tags")).toBe("http://localhost:11434/api/tags")
55+
})
56+
57+
it("should handle path without leading slash", () => {
58+
expect(joinUrlPath("http://localhost:11434", "api/tags")).toBe("http://localhost:11434/api/tags")
59+
})
60+
61+
it("should handle complex paths", () => {
62+
expect(joinUrlPath("http://localhost:11434/", "/v1/api/embed")).toBe("http://localhost:11434/v1/api/embed")
63+
})
64+
65+
it("should handle base URL with existing path", () => {
66+
expect(joinUrlPath("http://localhost:11434/ollama", "/api/tags")).toBe(
67+
"http://localhost:11434/ollama/api/tags",
68+
)
69+
expect(joinUrlPath("http://localhost:11434/ollama/", "/api/tags")).toBe(
70+
"http://localhost:11434/ollama/api/tags",
71+
)
72+
})
73+
74+
it("should handle empty path", () => {
75+
expect(joinUrlPath("http://localhost:11434", "")).toBe("http://localhost:11434/")
76+
})
77+
})
78+
})

src/utils/url-normalization.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* Normalizes a base URL by removing trailing slashes.
3+
* This prevents double slashes when concatenating paths.
4+
*
5+
* @param url - The URL to normalize
6+
* @returns The normalized URL without trailing slashes
7+
*
8+
* @example
9+
* normalizeBaseUrl("http://localhost:11434/") // returns "http://localhost:11434"
10+
* normalizeBaseUrl("http://localhost:11434") // returns "http://localhost:11434"
11+
* normalizeBaseUrl("http://localhost:11434//") // returns "http://localhost:11434"
12+
*/
13+
export function normalizeBaseUrl(url: string): string {
14+
// Remove all trailing slashes
15+
return url.replace(/\/+$/, "")
16+
}
17+
18+
/**
19+
* Joins a base URL with a path, ensuring no double slashes.
20+
*
21+
* @param baseUrl - The base URL (will be normalized)
22+
* @param path - The path to append (should start with /)
23+
* @returns The joined URL
24+
*
25+
* @example
26+
* joinUrlPath("http://localhost:11434/", "/api/tags") // returns "http://localhost:11434/api/tags"
27+
* joinUrlPath("http://localhost:11434", "/api/tags") // returns "http://localhost:11434/api/tags"
28+
*/
29+
export function joinUrlPath(baseUrl: string, path: string): string {
30+
const normalizedBase = normalizeBaseUrl(baseUrl)
31+
// Ensure path starts with a single slash
32+
const normalizedPath = path.startsWith("/") ? path : `/${path}`
33+
return `${normalizedBase}${normalizedPath}`
34+
}

0 commit comments

Comments
 (0)