fix: exclude encoding_format parameter for Google Gemini embeddings API #7762
Conversation
- Added isGeminiEndpoint detection to OpenAICompatibleEmbedder - Conditionally include encoding_format only for non-Gemini endpoints - Gemini API does not support the OpenAI-specific encoding_format parameter - Added comprehensive tests to verify Gemini compatibility Fixes #7761
dosubot
bot
added
size:L
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * @returns true if it's a Gemini endpoint | ||
| */ | ||
| private isGeminiUrl(url: string): boolean { | ||
| return url.includes("generativelanguage.googleapis.com") |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To robustly detect if the provided URL belongs to the Google Gemini API, we should parse the URL and inspect the hostname (not the full string) for an exact match or subdomain match. This avoids the scenario where the substring appears elsewhere in the URL (path, query, etc).
- Parse the input URL using the standard
URLclass (available in Node.js v10+ and modern browsers). - Extract the hostname (e.g.,
generativelanguage.googleapis.comorv1.generativelanguage.googleapis.com). - Accept hostnames that either exactly match
generativelanguage.googleapis.comor end with.generativelanguage.googleapis.com(to account for subdomains). - Implement this purely within the
isGeminiUrlmethod. Add a try-catch to handle invalid URLs gracefully and return false if parsing fails. - No additional package imports are required, as
URLis a standard global object.
The required change is only within the method isGeminiUrl in src/services/code-index/embedders/openai-compatible.ts.
| @@ -190,7 +190,15 @@ | ||
| * @returns true if it's a Gemini endpoint | ||
| */ | ||
| private isGeminiUrl(url: string): boolean { | ||
| return url.includes("generativelanguage.googleapis.com") | ||
| try { | ||
| const { hostname } = new URL(url); | ||
| return ( | ||
| hostname === "generativelanguage.googleapis.com" || | ||
| hostname.endsWith(".generativelanguage.googleapis.com") | ||
| ); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| * @returns true if it's a Gemini endpoint | ||
| */ | ||
| private isGeminiUrl(url: string): boolean { | ||
| return url.includes("generativelanguage.googleapis.com") |
There was a problem hiding this comment.
Is using url.includes("generativelanguage.googleapis.com") robust enough for detection? This would match URLs like https://evil.com/redirect?to=generativelanguage.googleapis.com. Consider using a more specific pattern like checking if the URL starts with the Gemini domain or using a regex pattern:
| return url.includes("generativelanguage.googleapis.com") | |
| private isGeminiUrl(url: string): boolean { | |
| return url.startsWith("https://generativelanguage.googleapis.com/") || | |
| url.startsWith("http://generativelanguage.googleapis.com/") | |
| } |
| this.defaultModelId = modelId || getDefaultModelId("openai-compatible") | ||
| // Cache the URL type check for performance | ||
| this.isFullUrl = this.isFullEndpointUrl(baseUrl) | ||
| this.isGeminiEndpoint = this.isGeminiUrl(baseUrl) |
There was a problem hiding this comment.
Nice work documenting why we exclude encoding_format for Gemini! Could we also add a class-level comment or constructor documentation mentioning that this class handles both standard OpenAI-compatible endpoints and Google Gemini endpoints with their specific requirements? This would help future maintainers understand the dual purpose of this class.
| // when processing numeric arrays, which breaks compatibility with models using larger dimensions. | ||
| // By requesting base64 encoding, we bypass the package's parser and handle decoding ourselves. | ||
| // However, Gemini doesn't support this parameter, so we exclude it for Gemini endpoints. | ||
| if (!this.isGeminiEndpoint) { |
There was a problem hiding this comment.
As we add more provider-specific quirks, would it make sense to extract this provider detection logic into a separate utility? Something like a ProviderDetector class that could handle all provider-specific logic? Though this might be premature optimization at this point - just something to consider if we keep adding more provider-specific conditions.
hannesrudolph
added
the
Issue/PR - Triage
Description
This PR fixes issue #7761 where Google Gemini's embeddings API was returning a 400 error due to the OpenAI-specific
encoding_formatparameter being incorrectly included in the request.Problem
The
OpenAICompatibleEmbedderclass was unconditionally sendingencoding_format: "base64"with all embedding requests. While this parameter is needed for OpenAI and most OpenAI-compatible APIs to work around a parsing issue, Google Gemini's API doesn't support this parameter and returns a 400 error when it's included.Solution
encoding_formatparameter only for Gemini endpointsChanges
isGeminiEndpointproperty andisGeminiUrl()method to detect Gemini URLsencoding_formatTesting
encoding_formatis NOT sent to Gemini endpointsencoding_formatIS still sent to non-Gemini endpointsReview Confidence
Code review completed with 95% confidence score. Implementation properly addresses the issue with no security concerns.
Fixes #7761
Important
Excludes
encoding_formatfor Google Gemini endpoints inOpenAICompatibleEmbedder, ensuring compatibility while maintaining it for other OpenAI-compatible endpoints.encoding_formatparameter for Google Gemini endpoints inOpenAICompatibleEmbedder.encoding_formatfor other OpenAI-compatible endpoints.isGeminiEndpointproperty andisGeminiUrl()method inopenai-compatible.tsto identify Gemini URLs.openai-compatible.spec.tsto verify exclusion ofencoding_formatfor Gemini and inclusion for others.This description was created by
for 22abb2c. You can customize this summary. It will automatically update as commits are pushed.