fix: reject non-200 image download responses

[VectorPeak]

Reject unsuccessful HTTP image-download responses before writing them as local image files.

What Problem This Solves

download_image_by_url() is used by the remote text-to-image path when NetworkRenderStrategy.render_custom_template(..., return_url=False) posts directly to a /generate endpoint and expects image bytes back.

Before this change, the helper read and saved the response body without checking resp.status. A failed HTTP response could therefore be persisted as if it were a valid image:

NetworkRenderStrategy(..., return_url=False)
  -> download_image_by_url(..., post=True)
  -> HTTP 404 / 500 response from /generate
  -> await resp.read()
  -> save_temp_img(...) or write path
  -> caller receives a local image path containing an error body

That can make the later failure look like an image rendering/parsing problem instead of a download-boundary failure. This is related to the real T2I failure path reported in #7940, where remote T2I endpoints returned HTTP 404 during long-text image rendering.

Change

This PR adds one shared image-response writer for download_image_by_url():

• validate the response with the existing _raise_for_download_status(resp, url) helper before reading bytes
• reuse the existing DownloadFileHTTPError raised by download_file() for HTTP download failures
• preserve successful 200 behavior for both temp-image saves and caller-provided output paths
• apply the same status check to GET, POST, and the insecure SSL fallback branches

This intentionally does not change endpoint selection, retry/fallback behavior in NetworkRenderStrategy, or the broader download_file() helper that was already fixed separately.

Evidence

Focused reproduction before the fix showed the POST path writing a JSON error body as a .jpg file:

path_result= C:\Users\MiaoXing\AppData\Local\Temp\...\bad-image.bin
path_exists= True
path_bytes= b'{"error":"not found"}'
temp_result= D:\ZXY\Github\AstrBot\data\temp\io_temp_img_...jpg
temp_exists= True
temp_suffix= .jpg
temp_bytes= b'{"error":"not found"}'

The regression tests now cover both GET and POST image-download paths:

• rejecting a non-200 GET or POST image response before creating the target file
• rejecting a non-200 GET or POST response after the SSL fallback path
• preserving successful 200 GET and POST image writes

Possible call chain / impact

Long message text-to-image rendering
  -> NetworkRenderStrategy.render(..., return_url=False)
  -> NetworkRenderStrategy.render_custom_template(...)
  -> download_image_by_url(f"{endpoint}/generate", post=True, post_data=...)
  -> _raise_for_download_status(resp, url)
  -> save/write image bytes only for HTTP 200

The affected helper currently has a narrow call surface: the in-repository caller is the remote T2I image-byte path. Non-200 responses now fail at the image download boundary so NetworkRenderStrategy can try the next endpoint or report that all endpoints failed. Successful image downloads keep the existing return value and file-writing behavior.

Validation

• uv run pytest tests/unit/test_io_download_file.py -q -> 9 passed in 1.26s
• uv run ruff check astrbot/core/utils/io.py tests/unit/test_io_download_file.py -> All checks passed!
• uv run ruff format --check astrbot/core/utils/io.py tests/unit/test_io_download_file.py -> 2 files already formatted
• git diff --check -> passed

---

Checklist / ????

• ?? If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  / No new feature is added; this is a narrow bug fix.

• ?? My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  / Focused pytest, Ruff check, Ruff format check, and git diff --check were run locally.

• ?? I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
  / No new dependencies are introduced.

• ?? My changes do not introduce malicious code.

Summary by Sourcery

Validate HTTP responses in image download helper before saving content and add coverage for POST and SSL-fallback scenarios.

Bug Fixes:

• Prevent non-200 HTTP responses from being saved as image files in download_image_by_url, instead raising the existing HTTP download error.

Enhancements:

• Introduce a shared helper to validate and save image HTTP responses used by all download_image_by_url code paths.

Tests:

• Extend download tests with GET, POST, and SSL-fallback cases to ensure non-200 responses are rejected and successful 200 responses are written correctly.

[sourcery-ai]

Hey - I've left some high level feedback:

• In _save_image_response, consider adding a more precise type for resp (e.g., aiohttp.ClientResponse) so that the helper’s contract is clearer and future refactors can rely on proper typing.
• The outer except Exception as e: raise e in download_image_by_url is redundant and can be removed to avoid obscuring the original traceback and to keep the function’s error behavior simpler.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `_save_image_response`, consider adding a more precise type for `resp` (e.g., `aiohttp.ClientResponse`) so that the helper’s contract is clearer and future refactors can rely on proper typing.
- The outer `except Exception as e: raise e` in `download_image_by_url` is redundant and can be removed to avoid obscuring the original traceback and to keep the function’s error behavior simpler.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[gemini-code-assist]

Code Review

This pull request refactors download_image_by_url in astrbot/core/utils/io.py by extracting common image-saving logic into a new helper function, _save_image_response. It also adds unit tests to verify POST requests, SSL fallback, and error handling. The reviewer suggested parameterizing these new tests to cover both GET and POST request paths, as the current tests only cover the POST path.

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.

[gemini-code-assist]

The new tests only cover the post=True (POST) path of download_image_by_url. Since post=False (GET) is the default behavior and also uses the new _save_image_response helper, we should ensure it is also covered by the tests. Parameterizing the tests with post allows us to cover both GET and POST methods cleanly without duplicating test code.

@pytest.mark.asyncio
@pytest.mark.parametrize("post", [True, False])
async def test_download_image_by_url_rejects_non_200_response(
    monkeypatch,
    tmp_path,
    post,
):
    target_path = tmp_path / "image.jpg"
    _patch_download_session(
        monkeypatch,
        _FakeResponse(status=404, chunks=[b'{"error":"not found"}']),
    )

    with pytest.raises(io.DownloadFileHTTPError, match="HTTP status code: 404"):
        await io.download_image_by_url(
            "https://example.test/generate",
            post=post,
            post_data={"json": False} if post else None,
            path=str(target_path),
        )

    assert not target_path.exists()


@pytest.mark.asyncio
@pytest.mark.parametrize("post", [True, False])
async def test_download_image_by_url_rejects_non_200_response_after_ssl_fallback(
    monkeypatch,
    tmp_path,
    post,
):
    class FakeSSLError(Exception):
        pass

    target_path = tmp_path / "image.jpg"
    _patch_download_sessions(
        monkeypatch,
        [
            FakeSSLError(),
            _FakeResponse(status=500, chunks=[b"server error"]),
        ],
    )
    monkeypatch.setattr(io.aiohttp, "ClientConnectorSSLError", FakeSSLError)
    monkeypatch.setattr(io.aiohttp, "ClientConnectorCertificateError", FakeSSLError)

    with pytest.raises(io.DownloadFileHTTPError, match="HTTP status code: 500"):
        await io.download_image_by_url(
            "https://example.test/generate",
            post=post,
            post_data={"json": False} if post else None,
            path=str(target_path),
        )

    assert not target_path.exists()


@pytest.mark.asyncio
@pytest.mark.parametrize("post", [True, False])
async def test_download_image_by_url_writes_successful_response(
    monkeypatch,
    tmp_path,
    post,
):
    target_path = tmp_path / "image.jpg"
    _patch_download_session(
        monkeypatch,
        _FakeResponse(status=200, chunks=[b"image bytes"]),
    )

    result = await io.download_image_by_url(
        "https://example.test/generate",
        post=post,
        post_data={"json": False} if post else None,
        path=str(target_path),
    )

    assert result == str(target_path)
    assert target_path.read_bytes() == b"image bytes"

[VectorPeak]

Addressed the review feedback in \21bbb3116\:

• Added the explicit \�iohttp.ClientResponse\ type to \_save_image_response()\.
• Removed the redundant outer \except Exception as e: raise e\ from \download_image_by_url()\.
• Parameterized the image download regression tests so GET and POST paths are both covered, including the SSL fallback branch and successful 200 writes.
• Updated the PR body evidence/validation to reflect the expanded GET/POST coverage.

Validation:

• \uv run pytest tests/unit/test_io_download_file.py -q\ -> \9 passed in 1.26s\
• \uv run ruff check astrbot/core/utils/io.py tests/unit/test_io_download_file.py\ -> \All checks passed!\
• \uv run ruff format --check astrbot/core/utils/io.py tests/unit/test_io_download_file.py\ -> \2 files already formatted\
• \git diff --check\ -> passed

Remote CI is also green on the latest head commit.