fix: align Vertex content fetch flow with Gemini and handle base64 - #3038
Conversation
WalkthroughAdds Vertex AI video support to the video proxy: resolves Vertex task-produced video URLs (including data: base64 payloads) via a new getVertexVideoURL flow, validates/normalizes the URL, and streams decoded data-URL payloads or proxies regular URLs. Changes
Sequence DiagramsequenceDiagram
participant Client
participant VideoProxy
participant VertexAPI
participant Decoder
Client->>VideoProxy: Request video (Vertex channel)
VideoProxy->>VertexAPI: getVertexVideoURL(channel, task)
VertexAPI-->>VideoProxy: Resolved URL (regular or data:)
alt data: URL
VideoProxy->>Decoder: writeVideoDataURL(dataURL)
Decoder->>Decoder: Decode base64, infer MIME, set headers
Decoder-->>Client: Stream decoded bytes with caching headers
else regular URL
VideoProxy->>VideoProxy: Proxy upstream fetch (forward headers)
VideoProxy-->>Client: Stream upstream response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/video_proxy_gemini.go`:
- Around line 184-185: The code validates strings.TrimSpace(taskInfo.Url) but
returns taskInfo.Url, so update the return to use the trimmed URL: compute a
trimmed variable (e.g., trimmedURL := strings.TrimSpace(taskInfo.Url)) after the
parseErr check and return trimmedURL (and nil error) instead of taskInfo.Url;
adjust any subsequent use of taskInfo.Url in this function (in
controller/video_proxy_gemini.go) to use the trimmed variable to avoid
whitespace-related parse failures.
- Around line 236-247: The buildVideoDataURL function currently infers mime as
"video/<encoding>" when mimeType is empty, which incorrectly produces types like
"video/base64"; update buildVideoDataURL to treat encoding as a MIME subtype
only if it looks like a real mime (contains '/' ) or matches known video file
extensions (e.g., "mp4","webm","mov","mkv","avi"); otherwise treat encodings
like "base64" as transport markers and fall back to a safe default MIME such as
"video/mp4" (or use encoding when it contains a slash), ensuring the logic
around mimeType, encoding and the enc variable in buildVideoDataURL is adjusted
accordingly.
- Around line 213-220: The current parsing only inspects videos[0] and only the
bytesBase64Encoded field, so it misses later entries and valid uri shapes;
update the parsing of resp["videos"] in the handler to iterate all items in the
videos slice, for each item (map[string]any) first check bytesBase64Encoded and
if present build and return the data URL via buildVideoDataURL(mimeType,
encoding, bytesBase64Encoded), otherwise check for a uri string and return that
URI (ensuring it is non-empty); reference the resp variable, the videos slice,
each video map, and buildVideoDataURL when making the change.
In `@controller/video_proxy.go`:
- Around line 164-194: writeVideoDataURL decodes the entire base64 payload into
memory (payload) which can OOM; before allocating, estimate the decoded size
from payload length (or set a maxBytes constant) and reject/return an error if
it exceeds the limit, and then stream decode instead of decoding into []byte.
Locate writeVideoDataURL and replace the direct
base64.StdEncoding.DecodeString(payload) usage with a guarded approach: compute
expectedDecodedSize := (len(payload) * 3) / 4 (adjust for padding) or compare
len(payload) against maxBase64Len, return an error if it exceeds allowed size,
and then use a streaming decoder (base64.NewDecoder) to copy into c.Writer with
an io.LimitedReader / io.CopyN to enforce the same limit rather than allocating
videoBytes. Ensure headers (Content-Type, Cache-Control) are set before
streaming and any decode errors are handled and returned.
| if parseErr == nil && taskInfo != nil && strings.TrimSpace(taskInfo.Url) != "" { | ||
| return taskInfo.Url, nil |
There was a problem hiding this comment.
Return the trimmed URL after validation.
You validate TrimSpace(taskInfo.Url) but return taskInfo.Url directly. Returning the trimmed value avoids avoidable URL-parse failures from surrounding whitespace.
💡 Proposed fix
- if parseErr == nil && taskInfo != nil && strings.TrimSpace(taskInfo.Url) != "" {
- return taskInfo.Url, nil
+ if parseErr == nil && taskInfo != nil && strings.TrimSpace(taskInfo.Url) != "" {
+ return strings.TrimSpace(taskInfo.Url), nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if parseErr == nil && taskInfo != nil && strings.TrimSpace(taskInfo.Url) != "" { | |
| return taskInfo.Url, nil | |
| if parseErr == nil && taskInfo != nil && strings.TrimSpace(taskInfo.Url) != "" { | |
| return strings.TrimSpace(taskInfo.Url), nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/video_proxy_gemini.go` around lines 184 - 185, The code validates
strings.TrimSpace(taskInfo.Url) but returns taskInfo.Url, so update the return
to use the trimmed URL: compute a trimmed variable (e.g., trimmedURL :=
strings.TrimSpace(taskInfo.Url)) after the parseErr check and return trimmedURL
(and nil error) instead of taskInfo.Url; adjust any subsequent use of
taskInfo.Url in this function (in controller/video_proxy_gemini.go) to use the
trimmed variable to avoid whitespace-related parse failures.
| if videos, ok := resp["videos"].([]any); ok && len(videos) > 0 { | ||
| if video, ok := videos[0].(map[string]any); ok && video != nil { | ||
| if b64, _ := video["bytesBase64Encoded"].(string); strings.TrimSpace(b64) != "" { | ||
| mime, _ := video["mimeType"].(string) | ||
| enc, _ := video["encoding"].(string) | ||
| return buildVideoDataURL(mime, enc, b64) | ||
| } | ||
| } |
There was a problem hiding this comment.
videos parsing currently misses common valid URL shapes.
The logic only checks videos[0] and only bytesBase64Encoded. If uri exists (or the valid item is later in the array), this returns empty and can incorrectly fail resolution.
💡 Proposed fix
- if videos, ok := resp["videos"].([]any); ok && len(videos) > 0 {
- if video, ok := videos[0].(map[string]any); ok && video != nil {
- if b64, _ := video["bytesBase64Encoded"].(string); strings.TrimSpace(b64) != "" {
- mime, _ := video["mimeType"].(string)
- enc, _ := video["encoding"].(string)
- return buildVideoDataURL(mime, enc, b64)
- }
- }
- }
+ if videos, ok := resp["videos"].([]any); ok {
+ for _, item := range videos {
+ video, ok := item.(map[string]any)
+ if !ok || video == nil {
+ continue
+ }
+ if uri, _ := video["uri"].(string); strings.TrimSpace(uri) != "" {
+ return strings.TrimSpace(uri)
+ }
+ if b64, _ := video["bytesBase64Encoded"].(string); strings.TrimSpace(b64) != "" {
+ mime, _ := video["mimeType"].(string)
+ enc, _ := video["encoding"].(string)
+ return buildVideoDataURL(mime, enc, b64)
+ }
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if videos, ok := resp["videos"].([]any); ok && len(videos) > 0 { | |
| if video, ok := videos[0].(map[string]any); ok && video != nil { | |
| if b64, _ := video["bytesBase64Encoded"].(string); strings.TrimSpace(b64) != "" { | |
| mime, _ := video["mimeType"].(string) | |
| enc, _ := video["encoding"].(string) | |
| return buildVideoDataURL(mime, enc, b64) | |
| } | |
| } | |
| if videos, ok := resp["videos"].([]any); ok { | |
| for _, item := range videos { | |
| video, ok := item.(map[string]any) | |
| if !ok || video == nil { | |
| continue | |
| } | |
| if uri, _ := video["uri"].(string); strings.TrimSpace(uri) != "" { | |
| return strings.TrimSpace(uri) | |
| } | |
| if b64, _ := video["bytesBase64Encoded"].(string); strings.TrimSpace(b64) != "" { | |
| mime, _ := video["mimeType"].(string) | |
| enc, _ := video["encoding"].(string) | |
| return buildVideoDataURL(mime, enc, b64) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/video_proxy_gemini.go` around lines 213 - 220, The current parsing
only inspects videos[0] and only the bytesBase64Encoded field, so it misses
later entries and valid uri shapes; update the parsing of resp["videos"] in the
handler to iterate all items in the videos slice, for each item (map[string]any)
first check bytesBase64Encoded and if present build and return the data URL via
buildVideoDataURL(mimeType, encoding, bytesBase64Encoded), otherwise check for a
uri string and return that URI (ensuring it is non-empty); reference the resp
variable, the videos slice, each video map, and buildVideoDataURL when making
the change.
| func buildVideoDataURL(mimeType string, encoding string, base64Data string) string { | ||
| mime := strings.TrimSpace(mimeType) | ||
| if mime == "" { | ||
| enc := strings.TrimSpace(encoding) | ||
| if enc == "" { | ||
| enc = "mp4" | ||
| } | ||
| if strings.Contains(enc, "/") { | ||
| mime = enc | ||
| } else { | ||
| mime = "video/" + enc | ||
| } |
There was a problem hiding this comment.
Do not infer MIME as video/<encoding> for encoding markers like base64.
When encoding is a transport marker (for example, base64), this generates invalid content types such as video/base64, which can break playback/content handling.
💡 Proposed fix
func buildVideoDataURL(mimeType string, encoding string, base64Data string) string {
mime := strings.TrimSpace(mimeType)
if mime == "" {
- enc := strings.TrimSpace(encoding)
- if enc == "" {
- enc = "mp4"
- }
- if strings.Contains(enc, "/") {
- mime = enc
- } else {
- mime = "video/" + enc
- }
+ enc := strings.ToLower(strings.TrimSpace(encoding))
+ switch enc {
+ case "", "base64":
+ mime = "video/mp4"
+ default:
+ if strings.Contains(enc, "/") {
+ mime = enc
+ } else {
+ mime = "video/" + enc
+ }
+ }
}
return "data:" + mime + ";base64," + base64Data
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func buildVideoDataURL(mimeType string, encoding string, base64Data string) string { | |
| mime := strings.TrimSpace(mimeType) | |
| if mime == "" { | |
| enc := strings.TrimSpace(encoding) | |
| if enc == "" { | |
| enc = "mp4" | |
| } | |
| if strings.Contains(enc, "/") { | |
| mime = enc | |
| } else { | |
| mime = "video/" + enc | |
| } | |
| func buildVideoDataURL(mimeType string, encoding string, base64Data string) string { | |
| mime := strings.TrimSpace(mimeType) | |
| if mime == "" { | |
| enc := strings.ToLower(strings.TrimSpace(encoding)) | |
| switch enc { | |
| case "", "base64": | |
| mime = "video/mp4" | |
| default: | |
| if strings.Contains(enc, "/") { | |
| mime = enc | |
| } else { | |
| mime = "video/" + enc | |
| } | |
| } | |
| } | |
| return "data:" + mime + ";base64," + base64Data | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/video_proxy_gemini.go` around lines 236 - 247, The
buildVideoDataURL function currently infers mime as "video/<encoding>" when
mimeType is empty, which incorrectly produces types like "video/base64"; update
buildVideoDataURL to treat encoding as a MIME subtype only if it looks like a
real mime (contains '/' ) or matches known video file extensions (e.g.,
"mp4","webm","mov","mkv","avi"); otherwise treat encodings like "base64" as
transport markers and fall back to a safe default MIME such as "video/mp4" (or
use encoding when it contains a slash), ensuring the logic around mimeType,
encoding and the enc variable in buildVideoDataURL is adjusted accordingly.
| func writeVideoDataURL(c *gin.Context, dataURL string) error { | ||
| parts := strings.SplitN(dataURL, ",", 2) | ||
| if len(parts) != 2 { | ||
| return fmt.Errorf("invalid data url") | ||
| } | ||
|
|
||
| header := parts[0] | ||
| payload := parts[1] | ||
| if !strings.HasPrefix(header, "data:") || !strings.Contains(header, ";base64") { | ||
| return fmt.Errorf("unsupported data url") | ||
| } | ||
|
|
||
| mimeType := strings.TrimPrefix(header, "data:") | ||
| mimeType = strings.TrimSuffix(mimeType, ";base64") | ||
| if mimeType == "" { | ||
| mimeType = "video/mp4" | ||
| } | ||
|
|
||
| videoBytes, err := base64.StdEncoding.DecodeString(payload) | ||
| if err != nil { | ||
| videoBytes, err = base64.RawStdEncoding.DecodeString(payload) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| c.Writer.Header().Set("Content-Type", mimeType) | ||
| c.Writer.Header().Set("Cache-Control", "public, max-age=86400") | ||
| c.Writer.WriteHeader(http.StatusOK) | ||
| _, err = c.Writer.Write(videoBytes) | ||
| return err |
There was a problem hiding this comment.
Guard decoded payload size before allocating full video bytes.
writeVideoDataURL currently decodes the whole base64 payload into memory. Large inline videos can cause high memory pressure or OOM under concurrency.
💡 Proposed fix
func writeVideoDataURL(c *gin.Context, dataURL string) error {
+ const maxInlineVideoBytes = 64 << 20 // 64 MiB decoded payload cap
parts := strings.SplitN(dataURL, ",", 2)
if len(parts) != 2 {
return fmt.Errorf("invalid data url")
}
@@
mimeType := strings.TrimPrefix(header, "data:")
mimeType = strings.TrimSuffix(mimeType, ";base64")
if mimeType == "" {
mimeType = "video/mp4"
}
+
+ if base64.StdEncoding.DecodedLen(len(payload)) > maxInlineVideoBytes {
+ return fmt.Errorf("data url payload too large")
+ }
videoBytes, err := base64.StdEncoding.DecodeString(payload)
if err != nil {
videoBytes, err = base64.RawStdEncoding.DecodeString(payload)
if err != nil {
return err
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func writeVideoDataURL(c *gin.Context, dataURL string) error { | |
| parts := strings.SplitN(dataURL, ",", 2) | |
| if len(parts) != 2 { | |
| return fmt.Errorf("invalid data url") | |
| } | |
| header := parts[0] | |
| payload := parts[1] | |
| if !strings.HasPrefix(header, "data:") || !strings.Contains(header, ";base64") { | |
| return fmt.Errorf("unsupported data url") | |
| } | |
| mimeType := strings.TrimPrefix(header, "data:") | |
| mimeType = strings.TrimSuffix(mimeType, ";base64") | |
| if mimeType == "" { | |
| mimeType = "video/mp4" | |
| } | |
| videoBytes, err := base64.StdEncoding.DecodeString(payload) | |
| if err != nil { | |
| videoBytes, err = base64.RawStdEncoding.DecodeString(payload) | |
| if err != nil { | |
| return err | |
| } | |
| } | |
| c.Writer.Header().Set("Content-Type", mimeType) | |
| c.Writer.Header().Set("Cache-Control", "public, max-age=86400") | |
| c.Writer.WriteHeader(http.StatusOK) | |
| _, err = c.Writer.Write(videoBytes) | |
| return err | |
| func writeVideoDataURL(c *gin.Context, dataURL string) error { | |
| const maxInlineVideoBytes = 64 << 20 // 64 MiB decoded payload cap | |
| parts := strings.SplitN(dataURL, ",", 2) | |
| if len(parts) != 2 { | |
| return fmt.Errorf("invalid data url") | |
| } | |
| header := parts[0] | |
| payload := parts[1] | |
| if !strings.HasPrefix(header, "data:") || !strings.Contains(header, ";base64") { | |
| return fmt.Errorf("unsupported data url") | |
| } | |
| mimeType := strings.TrimPrefix(header, "data:") | |
| mimeType = strings.TrimSuffix(mimeType, ";base64") | |
| if mimeType == "" { | |
| mimeType = "video/mp4" | |
| } | |
| if base64.StdEncoding.DecodedLen(len(payload)) > maxInlineVideoBytes { | |
| return fmt.Errorf("data url payload too large") | |
| } | |
| videoBytes, err := base64.StdEncoding.DecodeString(payload) | |
| if err != nil { | |
| videoBytes, err = base64.RawStdEncoding.DecodeString(payload) | |
| if err != nil { | |
| return err | |
| } | |
| } | |
| c.Writer.Header().Set("Content-Type", mimeType) | |
| c.Writer.Header().Set("Cache-Control", "public, max-age=86400") | |
| c.Writer.WriteHeader(http.StatusOK) | |
| _, err = c.Writer.Write(videoBytes) | |
| return err | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/video_proxy.go` around lines 164 - 194, writeVideoDataURL decodes
the entire base64 payload into memory (payload) which can OOM; before
allocating, estimate the decoded size from payload length (or set a maxBytes
constant) and reject/return an error if it exceeds the limit, and then stream
decode instead of decoding into []byte. Locate writeVideoDataURL and replace the
direct base64.StdEncoding.DecodeString(payload) usage with a guarded approach:
compute expectedDecodedSize := (len(payload) * 3) / 4 (adjust for padding) or
compare len(payload) against maxBase64Len, return an error if it exceeds allowed
size, and then use a streaming decoder (base64.NewDecoder) to copy into c.Writer
with an io.LimitedReader / io.CopyN to enforce the same limit rather than
allocating videoBytes. Ensure headers (Content-Type, Cache-Control) are set
before streaming and any decode errors are handled and returned.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/video_proxy_gemini.go`:
- Around line 250-255: Trim the extracted video string immediately and use that
trimmed value for emptiness and scheme checks and when returning or passing to
buildVideoDataURL; specifically, in the block that reads video from resp (the
variable named video) replace usages of the original untrimmed value with a
trimmedVideo (or reassign video = strings.TrimSpace(video)) so the HasPrefix
checks and the returned value (or call to buildVideoDataURL) use the normalized
string, and ensure encoding is read as before from resp["encoding"] for the
buildVideoDataURL call.
| if video, _ := resp["video"].(string); strings.TrimSpace(video) != "" { | ||
| if strings.HasPrefix(video, "data:") || strings.HasPrefix(video, "http://") || strings.HasPrefix(video, "https://") { | ||
| return video | ||
| } | ||
| enc, _ := resp["encoding"].(string) | ||
| return buildVideoDataURL("", enc, video) |
There was a problem hiding this comment.
Normalize video before scheme checks and return.
Whitespace is stripped for emptiness only; scheme checks and return values should use the trimmed value too.
💡 Proposed fix
- if video, _ := resp["video"].(string); strings.TrimSpace(video) != "" {
- if strings.HasPrefix(video, "data:") || strings.HasPrefix(video, "http://") || strings.HasPrefix(video, "https://") {
- return video
+ if video, _ := resp["video"].(string); strings.TrimSpace(video) != "" {
+ trimmedVideo := strings.TrimSpace(video)
+ if strings.HasPrefix(trimmedVideo, "data:") || strings.HasPrefix(trimmedVideo, "http://") || strings.HasPrefix(trimmedVideo, "https://") {
+ return trimmedVideo
}
enc, _ := resp["encoding"].(string)
- return buildVideoDataURL("", enc, video)
+ return buildVideoDataURL("", enc, trimmedVideo)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if video, _ := resp["video"].(string); strings.TrimSpace(video) != "" { | |
| if strings.HasPrefix(video, "data:") || strings.HasPrefix(video, "http://") || strings.HasPrefix(video, "https://") { | |
| return video | |
| } | |
| enc, _ := resp["encoding"].(string) | |
| return buildVideoDataURL("", enc, video) | |
| if video, _ := resp["video"].(string); strings.TrimSpace(video) != "" { | |
| trimmedVideo := strings.TrimSpace(video) | |
| if strings.HasPrefix(trimmedVideo, "data:") || strings.HasPrefix(trimmedVideo, "http://") || strings.HasPrefix(trimmedVideo, "https://") { | |
| return trimmedVideo | |
| } | |
| enc, _ := resp["encoding"].(string) | |
| return buildVideoDataURL("", enc, trimmedVideo) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/video_proxy_gemini.go` around lines 250 - 255, Trim the extracted
video string immediately and use that trimmed value for emptiness and scheme
checks and when returning or passing to buildVideoDataURL; specifically, in the
block that reads video from resp (the variable named video) replace usages of
the original untrimmed value with a trimmedVideo (or reassign video =
strings.TrimSpace(video)) so the HasPrefix checks and the returned value (or
call to buildVideoDataURL) use the normalized string, and ensure encoding is
read as before from resp["encoding"] for the buildVideoDataURL call.
fix: align Vertex content fetch flow with Gemini and handle base64
Summary by CodeRabbit