added download file method by someqst · Pull Request #90 · love-apples/maxapi

someqst · 2026-04-07T22:57:38Z

Добавил метод для скачивания файлов
closes #64
Протестировано на:

Документах
Аудио
Видео

Очень хотел бы видеть этот метод.

Olegt0rr · 2026-04-08T06:57:57Z

                return await response.text()
+
+    async def download_file(
+        self, url: str, destination: Path | str, chunk_size: int | None = 65536


65536 - магическое число, лучше вынести в константу

Olegt0rr · 2026-04-08T07:06:54Z

+        if not bot.session:
+            bot.session = ClientSession(
+                base_url=bot.api_url,
+                timeout=bot.default_connection.timeout,
+                headers=bot.headers,
+                **bot.default_connection.kwargs,
+            )


Эта конструкция несколько раз встречается в коде. По принципам DRY лучше вынести её в отдельный метод Bot, позволяющий установить сессию и на выходе гарантировано её получить. Чтобы эта логика не была разбросана по всему приложению

Olegt0rr · 2026-04-08T07:07:15Z

                response = await temp_session.post(url=url, data=form)
                return await response.text()
+
+    async def download_file(


Нужно дописать тесты для покрытия этой функции

Olegt0rr · 2026-04-08T07:09:40Z

+                **bot.default_connection.kwargs,
+            )
+
+        response = await bot.session.get(url=url)


Если посмотреть на пример метода request в Bot, то там реализован backoff на случай отвала. Было бы здорово его здесь поддержать.

И в целом лучше в докстрингах указать явный комментарий почему данный метод работает не через общий request

Olegt0rr

Хороший и чистый код, но требует ещё доработок по комментариям.

Olegt0rr · 2026-04-08T07:19:14Z

@someqst добавь, пожалуйста, в описание PR: closes #64, чтобы связать с задачей

Copilot

Pull request overview

Adds a new download_file capability to the MAX API client so bots can download user-sent attachments (docs/audio/video), addressing issue #64.

Changes:

Introduces BaseConnection.download_file() to stream-download a file to a local destination.
Derives the output filename from Content-Disposition (with URL-decoding) or falls back to a UUID + MIME-based extension.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T11:15:23Z

+            bot.session = ClientSession(
+                base_url=bot.api_url,
+                timeout=bot.default_connection.timeout,
+                headers=bot.headers,
+                **bot.default_connection.kwargs,


bot.session создаётся с base_url=bot.api_url, но url для скачивания берётся из payload и может быть абсолютным (https://...). В aiohttp нельзя делать запросы с абсолютным URL через ClientSession с base_url — это приводит к ошибке сборки URL. Решение: для абсолютных URL использовать отдельную ClientSession без base_url (но с теми же headers/timeout), либо приводить url к относительному пути перед get().

Copilot · 2026-04-09T11:15:24Z

+        response = await bot.session.get(url=url)
+
+        cd = response.content_disposition
+        if cd and cd.filename:
+            filename = cd.filename
+            filename = unquote(cd.filename)
+        else:
+            ext = mimetypes.guess_extension(response.content_type) or ""
+            filename = str(uuid4()) + ext
+
+        path = Path(destination) / filename
+
+        async with aiofiles.open(path, "wb") as f:
+            async for chunk in response.content.iter_chunked(chunk_size):
+                await f.write(chunk)
+


Ответ GET не закрывается/не освобождается: используется await bot.session.get(...) без async with, и после чтения/записи файла не вызывается response.release()/response.close(). Это может привести к утечке соединений в пуле. Лучше оборачивать запрос в async with ... as response: и гарантировать освобождение ресурсов при исключениях.

Suggested change

response = await bot.session.get(url=url)

cd = response.content_disposition

if cd and cd.filename:

filename = cd.filename

filename = unquote(cd.filename)

else:

ext = mimetypes.guess_extension(response.content_type) or ""

filename = str(uuid4()) + ext

path = Path(destination) / filename

async with aiofiles.open(path, "wb") as f:

async for chunk in response.content.iter_chunked(chunk_size):

await f.write(chunk)

async with bot.session.get(url=url) as response:

cd = response.content_disposition

if cd and cd.filename:

filename = cd.filename

filename = unquote(cd.filename)

else:

ext = mimetypes.guess_extension(response.content_type) or ""

filename = str(uuid4()) + ext

path = Path(destination) / filename

async with aiofiles.open(path, "wb") as f:

async for chunk in response.content.iter_chunked(chunk_size):

await f.write(chunk)

Copilot · 2026-04-09T11:15:24Z

+        response = await bot.session.get(url=url)
+
+        cd = response.content_disposition
+        if cd and cd.filename:
+            filename = cd.filename
+            filename = unquote(cd.filename)
+        else:
+            ext = mimetypes.guess_extension(response.content_type) or ""
+            filename = str(uuid4()) + ext
+
+        path = Path(destination) / filename
+
+        async with aiofiles.open(path, "wb") as f:
+            async for chunk in response.content.iter_chunked(chunk_size):
+                await f.write(chunk)
+
+        return path


Перед сохранением на диск нет проверки response.status/response.ok. Сейчас при 401/404/5xx будет сохранён ответ об ошибке как «файл». Нужна явная обработка не-2xx статусов (например, 401 → InvalidToken, прочие → MaxApiError/NotAvailableForDownload) до начала записи.

Suggested change

response = await bot.session.get(url=url)

cd = response.content_disposition

if cd and cd.filename:

filename = cd.filename

filename = unquote(cd.filename)

else:

ext = mimetypes.guess_extension(response.content_type) or ""

filename = str(uuid4()) + ext

path = Path(destination) / filename

async with aiofiles.open(path, "wb") as f:

async for chunk in response.content.iter_chunked(chunk_size):

await f.write(chunk)

return path

async with bot.session.get(url=url) as response:

if response.status == 401:

message = await response.text()

raise InvalidToken(message or "Invalid token")

if response.status < 200 or response.status >= 300:

message = await response.text()

raise MaxApiError(

message or f"File is not available for download. HTTP status: {response.status}"

)

cd = response.content_disposition

if cd and cd.filename:

filename = cd.filename

filename = unquote(cd.filename)

else:

ext = mimetypes.guess_extension(response.content_type) or ""

filename = str(uuid4()) + ext

path = Path(destination) / filename

async with aiofiles.open(path, "wb") as f:

async for chunk in response.content.iter_chunked(chunk_size):

await f.write(chunk)

return path

Copilot · 2026-04-09T11:15:24Z

+    async def download_file(
+        self, url: str, destination: Path | str, chunk_size: int | None = 65536
+    ) -> Path | None:


chunk_size объявлен как int | None, но response.content.iter_chunked() требует int. При chunk_size=None скачивание упадёт с исключением. Либо сделайте параметр всегда int (с дефолтом), либо при None используйте другой способ чтения.

Copilot · 2026-04-09T11:15:24Z

+            filename = cd.filename
+            filename = unquote(cd.filename)


Имя файла берётся из Content-Disposition и используется без валидации. Если сервер вернёт filename с ../ или разделителями пути, то Path(destination) / filename позволит записать файл вне destination (path traversal). Нужна нормализация/санитизация имени (например, брать только Path(filename).name, запрещать разделители/нулевые байты) перед формированием итогового пути.

Suggested change

filename = cd.filename

filename = unquote(cd.filename)

raw_filename = unquote(cd.filename)

if "\x00" in raw_filename:

raise MaxApiError("Invalid filename in Content-Disposition")

filename = Path(raw_filename.replace("\\", "/")).name

if filename in {"", ".", ".."}:

ext = mimetypes.guess_extension(response.content_type) or ""

filename = str(uuid4()) + ext

Copilot · 2026-04-09T11:15:25Z

+            destination (Path | str): место назначения для скачанного файла.
+            chunk_size (bytes): Размер чанков файла, по умолчанию 64.
+        Returns:
+            Path: Ссылка на файл в файловой системе
+        """


Докстринг не совпадает с реальным поведением: chunk_size по умолчанию 65536 (64 KiB), но описан как «по умолчанию 64». Также в Returns указан Path, тогда как аннотация метода — Path | None, хотя None не возвращается. Стоит привести документацию и типы к одному контракту.

Copilot · 2026-04-09T11:15:25Z

+        cd = response.content_disposition
+        if cd and cd.filename:
+            filename = cd.filename
+            filename = unquote(cd.filename)


Лишнее присваивание: filename = cd.filename, а следующей строкой переменная сразу перезаписывается. Можно оставить одно присваивание (и вызывать unquote от уже присвоенной переменной), чтобы избежать дублирования.

Suggested change

filename = unquote(cd.filename)

filename = unquote(filename)

Copilot · 2026-04-09T11:15:25Z

+    async def download_file(
+        self, url: str, destination: Path | str, chunk_size: int | None = 65536
+    ) -> Path | None:
+        """
+        Скачивает файл с сервера MAX.


В репозитории есть покрытие тестами для connection/utils, но для нового метода скачивания тестов нет. Желательно добавить тест(ы), которые проверяют: выбор имени файла (с/без Content-Disposition), обработку не-2xx статусов и запись чанками в destination (с мокнутым ClientSession/response).

bish-x · 2026-04-12T10:11:27Z

Привет! Я реализовал расширенную версию этого функционала в PR #96, где учтены все замечания @Olegt0rr:

Магическое число 65536 вынесено в константу DOWNLOAD_CHUNK_SIZE
Логика создания сессии — через Bot.ensure_session() (DRY)
Retry/backoff через библиотеку backoff вместо ручной реализации
Защита от path traversal (Path(filename).name)
Тесты наа скачивание, retry, path traversal, ensure_session
async with / try/finally для освобождения response

Если этот PR больше не планируется дорабатывать, возможно имеет смысл его закрыть в пользу #96?

added downlod file method

654c54c

someqst changed the title ~~added downlod file method~~ added download file method Apr 8, 2026

Olegt0rr reviewed Apr 8, 2026

View reviewed changes

Olegt0rr requested changes Apr 8, 2026

View reviewed changes

bish-x mentioned this pull request Apr 8, 2026

feat: добавлен метод download_file для скачивания файлов #96

Merged

Olegt0rr requested a review from Copilot April 9, 2026 11:10

Copilot started reviewing on behalf of Olegt0rr April 9, 2026 11:11 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Olegt0rr assigned someqst Apr 12, 2026

someqst closed this Apr 13, 2026

-            filename = cd.filename
-            filename = unquote(cd.filename)
+            raw_filename = unquote(cd.filename)
+            if "\x00" in raw_filename:
+                raise MaxApiError("Invalid filename in Content-Disposition")
+            filename = Path(raw_filename.replace("\\", "/")).name
+            if filename in {"", ".", ".."}:
+                ext = mimetypes.guess_extension(response.content_type) or ""
+                filename = str(uuid4()) + ext

Conversation

someqst commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Olegt0rr Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Olegt0rr Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Olegt0rr Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Olegt0rr Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Olegt0rr left a comment

Choose a reason for hiding this comment

Uh oh!

Olegt0rr commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

bish-x commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

someqst commented Apr 7, 2026 •

edited

Loading

Olegt0rr Apr 8, 2026 •

edited

Loading

Olegt0rr commented Apr 8, 2026 •

edited

Loading

bish-x commented Apr 12, 2026 •

edited

Loading