bug: #3586730 textToImage() base64-decodes the image payload twice

Sven Decabooterrequested to merge
issue/ai-3586730:3586730-texttoimage-base64-decodes-the-image-payload-twice into 1.4.x

Description

Closes #3586730.

OpenAiBasedProviderClientBase::textToImage() decoded the b64_json payload twice, so the ImageFile it returned held bytes that are not an image. The second decode keeps roughly a quarter of the payload, and because the mime-type guard directly above it inspects the correctly decoded bytes and passes, the call still reports success — the corruption only surfaces when the file is written and opened.

The change is one line in src/Base/OpenAiBasedProviderClientBase.php:

- $images[] = new ImageFile(base64_decode($image_content), 'image/png', 'generated.png');
+ $file_ext = static::ALLOWED_IMAGE_MIME_TYPES[$mime_type];
+ $images[] = new ImageFile($image_content, $mime_type, 'generated.' . $file_ext);

$image_content is already the decoded binary, so it is passed through as-is. The same line also discarded $mime_type — computed and validated immediately above — in favour of a hardcoded image/png, so it now reports the type that was actually detected, and names the file with the matching extension from ALLOWED_IMAGE_MIME_TYPES. This is exactly what OpenAiProvider::textToImage() in ai_provider_openai already does; the base class was the outlier, which is also why the OpenAI provider itself was never affected.

Behaviour change worth noting for reviewers: for a JPEG or WebP response the returned ImageFile now carries image/jpeg / image/webp and generated.jpeg / generated.webp instead of image/png / generated.png. Consumers derive the file type from the filename (AbstractFileBase::getFileType()), so this makes saved files match their real format. PNG responses are unchanged. No API, signature or hook changes.

Regression range: introduced by bc860ac9 ("Let providers select allowed text-to-speech and text-to-image file formats", cherry-pick of 57dd477f), so it affects 1.4.4 and later, 1.3.10 and later, and the 1.2.x, 1.5.x, 1.x, 2.x and 2.0.x branches. This MR targets 1.4.x; the line is identical on every other branch, so it cherry-picks cleanly and needs a forward port.

Tests

textToImage() had no coverage on this class: the kernel TextToImageInterfaceTest exercises the echoai mock provider, and OpenAiBasedProviderClientBaseTest covered only exception handling and the Fiber branch. Three unit tests are added to the latter:

  • testTextToImageDecodesThePayloadOnce() — asserts the returned binary is byte-identical to the decoded payload. This is the regression assertion: it fails on the old code.
  • testTextToImageKeepsTheDetectedMimeType() — uses a JPEG payload, so it pins the mime type and filename. A PNG-only test would pass both before and after the change and leave the hardcoded image/png uncovered.
  • testTextToImageRejectsNonImagePayload() — asserts a payload that is not recognised as an allowed image still raises AiResponseErrorException.

Testing instructions

The unit test is the fastest check, and step 3 is what demonstrates the bug is really fixed rather than merely covered. To verify against a real API, with any provider that inherits this base implementation (LiteLLM, for example — not the OpenAI provider, which overrides the method):

  1. Set that provider as the Text-to-Image provider and generate an image in AI API Explorer → Text to Image, or call $provider->textToImage(new TextToImageInput('a red bicycle'), $model_id) and write getNormalized()[0]->getBinary() to a file.
  2. file reports PNG image data and xxd -l 8 shows 89504e470d0a1a0a. Before the fix the same call produced a file that file reports as data, starting 3cd1881c… — identical leading bytes for every prompt, since PNG magic is constant. Measured on one generation: 21,830 bytes of corrupt data before, 117,163 bytes of valid 1024×1024 PNG after.

Checklist

  • I have linked the related issue in the MR title or description
  • I have performed a self-review of my own code
  • I have added or updated tests, or explained in the description why this change is not covered by tests
  • I have updated documentation for any new or changed functionality — no documentation changes needed, this restores documented behaviour rather than adding any
  • I have written testing instructions and verified them locally
  • I have noted any required post-merge steps (config imports, cache rebuilds, manual changes) — none; a forward port to 1.5.x/1.x/2.x is the only follow-up
  • This MR contains no breaking API or hook changes, or they are explicitly documented in the description — no API or hook changes; the mime type and filename behaviour change is described above

AI Compliance

  • AI Assisted Code
    Mainly written by a human; AI used for autocomplete or partial generation under full human supervision.

  • AI Generated Code
    Mainly generated by AI, reviewed and approved by a human before this MR was created.

  • Vibe Coded
    Generated by AI and only functionally reviewed before this MR was created.

Merge request reports