fix: sanitize unsupported image MIME types (e.g. GIF) in context cleaner#9335
fix: sanitize unsupported image MIME types (e.g. GIF) in context cleaner#9335xiaoyuyu6420 wants to merge 2 commits into
Conversation
Fixes AstrBotDevs#9295. When a provider declares image modality support, the context sanitizer previously kept every image block as-is. Animated GIFs can then be persisted into session history and later cause continuous failures on OpenAI-compatible Gemini gateways that reject image/gif. - Drop image/gif data URLs and .gif http(s) URLs to a [Image] placeholder even when the provider supports image modality. - Keep the existing full image-strip path when image modality is absent. - Preserve JPEG/PNG/WebP and unknown-MIME images when image is supported. - Add unit coverage for the reporter scenario and related edge cases.
dosubot
Bot
added
size:M
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
_extract_image_mime, consider usingurllib.parse.urlsplit(or similar) instead of manualsplit('?', '#')to make URL path handling more robust for edge cases like encoded?/#in the path. - Given that
_UNSUPPORTED_IMAGE_MIMESis meant as a safety net for multiple gateways, you might want to expose a simple extension point (e.g., optional config injection or an overridable helper) so providers that truly support GIF can opt out without code changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_extract_image_mime`, consider using `urllib.parse.urlsplit` (or similar) instead of manual `split('?', '#')` to make URL path handling more robust for edge cases like encoded `?`/`#` in the path.
- Given that `_UNSUPPORTED_IMAGE_MIMES` is meant as a safety net for multiple gateways, you might want to expose a simple extension point (e.g., optional config injection or an overridable helper) so providers that truly support GIF can opt out without code changes.
## Individual Comments
### Comment 1
<location path="astrbot/core/provider/modalities.py" line_range="85-93" />
<code_context>
+ if path.endswith(ext):
+ return mime
+
+ source = part.get("source")
+ if isinstance(source, dict):
+ media_type = source.get("media_type")
+ if isinstance(media_type, str):
+ return media_type.lower()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Normalize `media_type` by stripping parameters (e.g. `image/gif; charset=binary`) before comparison.
In the current branch, `media_type` is lowercased but not stripped of parameters. If an API returns values like `image/gif; charset=binary` or `image/jpeg;quality=80`, `_is_unsupported_image_mime` will not match entries such as `image/gif` in `_UNSUPPORTED_IMAGE_MIMES`.
To align with the data-URL handling and make matching robust across providers, normalize by stripping parameters before lowercasing, e.g. `media_type.split(";", 1)[0].strip().lower()`.
```suggestion
source = part.get("source")
if isinstance(source, dict):
media_type = source.get("media_type")
if isinstance(media_type, str):
# Normalize media type by stripping parameters before comparison,
# e.g. "image/gif; charset=binary" -> "image/gif"
return media_type.split(";", 1)[0].strip().lower()
mime_type = part.get("mimeType") or part.get("mime_type")
if isinstance(mime_type, str):
# Normalize MIME type by stripping parameters before comparison
return mime_type.split(";", 1)[0].strip().lower()
```
</issue_to_address>
### Comment 2
<location path="astrbot/core/provider/modalities.py" line_range="91-93" />
<code_context>
+ if isinstance(media_type, str):
+ return media_type.lower()
+
+ mime_type = part.get("mimeType") or part.get("mime_type")
+ if isinstance(mime_type, str):
+ return mime_type.lower()
+
</code_context>
<issue_to_address>
**suggestion:** Consider stripping parameters from `mimeType`/`mime_type` as well to align with other MIME parsing.
Because `mimeType`/`mime_type` is returned as `mime_type.lower()`, a value like `image/gif;codec=xyz` won’t match `_UNSUPPORTED_IMAGE_MIMES` even though the base type is `image/gif`. Normalizing it with `split(";", 1)[0].strip().lower()` would align this path with data URLs and ensure unsupported image types are correctly detected.
```suggestion
mime_type = part.get("mimeType") or part.get("mime_type")
if isinstance(mime_type, str):
# Strip any parameters (e.g. "image/gif;codec=xyz" → "image/gif")
# to align with other MIME parsing and ensure unsupported types are detected.
return mime_type.split(";", 1)[0].strip().lower()
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to filter out unsupported image MIME types (specifically image/gif) during context sanitization, preventing issues with gateways (such as Gemini-compatible proxies) that reject them even when the model claims image support. It includes robust extraction of MIME types from data URLs, path extensions, and various payload structures, accompanied by comprehensive unit tests. The review feedback suggests enhancing robustness by stripping leading and trailing whitespace from URLs, media types, and MIME types before processing.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if isinstance(url, str): | ||
| # data URLs look like "data:image/gif;base64,...." | ||
| if url.lower().startswith("data:"): |
There was a problem hiding this comment.
To make the MIME extraction more robust against leading or trailing whitespace in URLs (which can occasionally occur in raw or user-submitted contexts), consider stripping the url string before processing.
| if isinstance(url, str): | |
| # data URLs look like "data:image/gif;base64,...." | |
| if url.lower().startswith("data:"): | |
| if isinstance(url, str): | |
| url = url.strip() | |
| # data URLs look like "data:image/gif;base64,...." | |
| if url.lower().startswith("data:"): |
| source = part.get("source") | ||
| if isinstance(source, dict): | ||
| media_type = source.get("media_type") | ||
| if isinstance(media_type, str): | ||
| return media_type.lower() | ||
|
|
||
| mime_type = part.get("mimeType") or part.get("mime_type") | ||
| if isinstance(mime_type, str): | ||
| return mime_type.lower() |
There was a problem hiding this comment.
Similar to how the data URL MIME extraction strips whitespace (on line 75), it is safer to strip leading/trailing whitespace from media_type and mime_type to ensure robust matching against _UNSUPPORTED_IMAGE_MIMES.
| source = part.get("source") | |
| if isinstance(source, dict): | |
| media_type = source.get("media_type") | |
| if isinstance(media_type, str): | |
| return media_type.lower() | |
| mime_type = part.get("mimeType") or part.get("mime_type") | |
| if isinstance(mime_type, str): | |
| return mime_type.lower() | |
| source = part.get("source") | |
| if isinstance(source, dict): | |
| media_type = source.get("media_type") | |
| if isinstance(media_type, str): | |
| return media_type.strip().lower() | |
| mime_type = part.get("mimeType") or part.get("mime_type") | |
| if isinstance(mime_type, str): | |
| return mime_type.strip().lower() |
Address Sourcery/Gemini review feedback on AstrBotDevs#9335 without adding new abstractions. - Strip parameters (e.g. "image/gif; charset=binary") and whitespace from source.media_type and mimeType/mime_type before comparing against _UNSUPPORTED_IMAGE_MIMES, so parameterized GIF values are still detected. - Strip surrounding whitespace from image URLs defensively. - Switch the http(s) URL path extraction from a manual split('?', '#') to urllib.parse.urlsplit, which keeps percent-encoded delimiters inside the path and is more robust for edge cases. - Inline the normalization at both call sites instead of introducing a helper, keeping it consistent with the existing data-URL branch and honoring the repo's No-Unnecessary-Helpers rule. Adds 5 unit tests covering parameterized media_type/mimeType, the encoded path case, and a non-GIF-with-params regression guard.
|
Pushed an iteration addressing the review feedback. Summary of decisions: Adopted
Not adopted — extension point / config injection for Note on scope (issue #9295)
If maintainers want either, I can follow up in a separate PR rather than expand this one. Tests: 16 passed (11 existing + 5 new). |
1 participant
Issue / 问题
Fixes #9295.
When a quoted animated GIF is added to conversation history, it can be
persisted as
data:image/gif;base64,.... Some OpenAI-compatible Geminigateways declare
imagemodality support but rejectimage/gif, so everylater request fails with
mime type is not supported/convert_request_faileduntil the session history is cleared.sanitize_contexts_by_modalities()previously only checked coarse modalityflags (
image/audio/tool_use) and short-circuited when all three wereenabled, so GIF MIME cleanup never ran for the reporter's configuration.
Modifications / 改动点
Keep a narrow unsupported MIME list (
image/gif) inastrbot/core/provider/modalities.py.Extract image MIME best-effort from OpenAI-style
image_urldata URLs and.gifHTTP URLs.Replace unsupported GIF image blocks with
[Image]even when the providerclaims image support.
Preserve existing full image stripping when image modality is absent, and
preserve JPEG/PNG/WebP / unknown-MIME images when image is supported.
Add unit tests for the reporter scenario and related edge cases.
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Results:
Trade-off note
This intentionally uses a narrow hard-coded MIME blacklist (
image/gif)instead of a new provider config switch, to keep the fix minimal and focused
on the reported failure mode. Providers that truly accept GIF will lose GIF
image blocks and receive a text placeholder instead. If maintainers prefer a
configurable MIME whitelist/blacklist, that can be layered on later without
reverting this safety net.
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new warnings or errors are introduced.
/ 我已确保没有引入新的警告或错误。
Summary by Sourcery
Sanitize conversation contexts to strip unsupported image MIME types (notably GIF) even when providers claim image support, preventing incompatible gateways from rejecting subsequent requests.
Bug Fixes:
Tests: