fix(bedrock): decode base64 data URL to bytes for Converse image source - #33536
fix(bedrock): decode base64 data URL to bytes for Converse image source#33536liuhao1024 wants to merge 2 commits into
Conversation
The _convert_content_to_converse() function passed the base64-encoded ASCII string directly as ImageBlock.source.bytes. Bedrock's Converse API expects raw bytes (boto3 base64-encodes on the wire), so the image was double-encoded and rejected with 'Invalid or unsupported image format'. Fix: base64.b64decode(data) before passing to source.bytes. Fixes NousResearch#33317
|
Solid work. We hit the same double-encoding issue and fixed it in #34742. The root cause is identical — base64 string passed to source.bytes instead of decoded raw bytes. boto3 re-encodes at the wire layer. Your fix and ours are the same two-line change. Whichever lands first should close the other. |
|
@alt-glitch @JiaDe-Wu Thanks for the context. I see #28085 and #34742 both address the same base64 double-encoding issue. Since all three PRs (#28085, #34742, #33536) are still open, I'll wait for the maintainer to decide which approach to merge. If either #28085 or #34742 merges first, I'm happy to close this PR as a duplicate. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused Bedrock regression fix. The premise is confirmed on current main: agent/bedrock_adapter.py:525 extracts the data-URL payload and agent/bedrock_adapter.py:534 currently forwards that base64 string as image.source.bytes.
Problems
- The proposed decode at
agent/bedrock_adapter.py:535is unguarded and non-strict. The converter accepts anydata:URL, so malformed payloads may either be silently converted to junk bytes or raise while building the request. The related open implementation in #28085 usesbase64.b64decode(..., validate=True), catches decode errors, and skips empty/malformed image blocks.
Suggested changes
- Use strict decoding, catch
binascii.Error/ValueError, and preserve adjacent text when discarding an invalid image block. - Add a malformed-data-URL regression case alongside the existing conversion test in
tests/agent/test_bedrock_adapter.py:332.
This is an automated hermes-sweeper review.
| "image": { | ||
| "format": media_type.split("/")[-1] if "/" in media_type else "jpeg", | ||
| "source": {"bytes": data}, | ||
| "source": {"bytes": base64.b64decode(data)}, |
There was a problem hiding this comment.
Please use strict decoding and handle malformed payloads here. This branch accepts every data: URL, while plain b64decode() may accept junk characters or raise for invalid padding. Use validate=True, catch decode errors, and skip empty/malformed image blocks so an invalid attachment does not fail conversion of the whole message.
What does this PR do?
_convert_content_to_converse()inagent/bedrock_adapter.pypasses the base64-encoded ASCII string from a data URL directly asImageBlock.source.bytes. Bedrock's Converse API expects rawbytes— boto3 base64-encodes them at the wire layer. The result is double-encoding, and Bedrock rejects every image with:Root Cause
Line 482 of
agent/bedrock_adapter.py:datais the portion after the comma indata:image/png;base64,iVBORw0KGgo...— a base64 ASCII string. It needs to be decoded to raw bytes first.Related Issue
Fixes #33317
Type of Change
Changes Made
How to Test
pytest tests/ -q— all tests should passChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture and workflows — or N/ACode Intelligence
agent/bedrock_adapter.py:_convert_content_to_converse(called fromconvert_messages_to_converse, used by Bedrock transport for all multimodal messages)base64.b64decode()(correct), openai passes string (correct for its API). Bedrock is the only adapter that needs raw bytes.Fixes #33317