Skip to content

zulip: Add Client.fetch_upload() to download uploaded files - #932

Open
Ompatel75 wants to merge 1 commit into
zulip:mainfrom
Ompatel75:fetch-upload-method
Open

Ompatel75 wants to merge 1 commit into
zulip:mainfrom
Ompatel75:fetch-upload-method

Conversation

@Ompatel75

Copy link
Copy Markdown

Fixes #628

Problem

client.call_endpoint() unconditionally prepends /api/v1/ to every
URL, making it impossible to request endpoints that live outside that
namespace. The most important case is Zulip's uploaded-file endpoints:

  • GET /user_uploads/<realm>/<filename> — downloads the raw file
  • GET /user_uploads/temporary/<token>/<filename> — temporary S3 link

A bot that tries to download an attachment via call_endpoint() silently
hits /api/v1/user_uploads/... instead (which returns a JSON
temporary-URL response), and then crashes when do_api_query() tries to
decode binary file bytes as JSON.

Solution

Add a dedicated Client.fetch_upload(path) method that:

  1. Does NOT prepend /api/v1/ — it builds the target URL from the
    server root (stripping the /api/ suffix from self.base_url).

  2. Reuses the authenticated session (self.session, which already
    carries HTTP Basic Auth with the bot's email and API key), so no
    separate credential plumbing is needed.

  3. Accepts both forms of path:

    • A relative path (/user_uploads/2/ab/cat.png) taken directly from
      a Zulip message's markdown link
    • A fully-qualified URL on the same server
  4. Returns a raw requests.Response (not Dict[str, Any]), because
    the response body is binary file content, not a JSON API envelope.

Example usage (from a bot)

import re, zulip

client = zulip.Client(config_file="~/.zuliprc")

# message['content'] == "[cat.png](/user_uploads/2/ab/cat.png)"
match = re.search(r'\((/user_uploads/[^)]+)\)', message['content'])
if match:
    response = client.fetch_upload(match.group(1))
    if response.status_code == 200:
        with open("cat.png", "wb") as f:
            f.write(response.content)

Zulip's uploaded-file endpoints (/user_uploads/...) live outside the
/api/v1/ namespace, so client.call_endpoint() cannot be used to reach
them -- it unconditionally prepends /api/v1/ to every path.

This commit adds a dedicated Client.fetch_upload(path) method that:

* Accepts a relative path (e.g. /user_uploads/2/ab/cat.png) extracted
  directly from a Zulip message's markdown link, or a fully-qualified
  URL on the same Zulip server.

* Sends the request through the existing authenticated session
  (HTTP Basic Auth with the bot's email and API key), so no separate
  credential management is required.

* Returns a raw requests.Response object -- not JSON -- because the
  response is binary file content, not a Zulip API envelope.

A corresponding example script (examples/download-file) and a full
test suite (zulip/tests/test_fetch_upload.py, 11 tests) are included.

Fixes zulip#628.
@Ompatel75
Ompatel75 force-pushed the fetch-upload-method branch from 0fc8388 to a4e87a7 Compare October 25, 2026 11:35
@knowprasad

Copy link
Copy Markdown

Took a look through the implementation and the static-analysis CI failure.

A couple of quick observations:

  1. In CI, static-analysis failed on Ruff EXE001 for zulip/zulip/examples/download-file because the file has a shebang (#!/usr/bin/env python3) but is committed as mode 100644. You can fix that locally with git update-index --chmod=+x zulip/zulip/examples/download-file and amending the commit.

  2. On the URL handling: if a caller passes an arbitrary absolute URL starting with http:// or https://, self.session.get() will send the bot's HTTP Basic Auth credentials (self.email, self.api_key) to that target host. If a bot extracts an external link from user message markdown and passes it to fetch_upload(), that could leak credentials. It might be safer to either assert that the netloc matches self.base_url or strip auth when fetching non-realm URLs.

  3. It might also be worth exposing timeout as a parameter (defaulting to 15.0 or None, similar to call_endpoint) so callers downloading larger files over slower connections don't hit unexpected read timeouts.

The tests in test_fetch_upload.py are really thorough, especially mocking out get_server_settings cleanly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add function for downloading an uploaded file

3 participants