Repository navigation
Take the photo token by position, not from the upload URL's photoIds - #107
Open
farafonoff wants to merge 2 commits into
Open
farafonoff wants to merge 2 commits into
farafonoff wants to merge 2 commits into
Conversation
MAX stopped returning a `photoIds` query parameter on the photo upload URL,
so `parse_qs(urlparse(url).query)["photoIds"][0]` raises KeyError and **every**
photo upload fails with `UploadError: Photo upload URL does not contain
photoIds`. Not intermittent, not rate limiting: no photo can upload at all.
Reported as happening against the live API on 2026-09-30, ~2 days after the
last successful photo forward.
Captured from the official MAX client, one photo:
REQUEST op=80 PHOTO_UPLOAD {"count": 1}
RESPONSE op=80 {"url": "https://iu.oneme.ru/uploadImage?r=<token>"}
POST that url -> 200 {"photos": {"0": {"token": "..."}}}
The official client annotates this itself ("phase 1/3 url has no media id").
The id was never needed. A multi-photo message is several *independent*
`count: 1` PHOTO_UPLOAD requests, each with its own one-shot URL and its own
single-entry result, whose tokens then go out together in one MSG_SEND --
verified with a two-photo capture (seq 80/81 issued in parallel, two POSTs,
two `photos["0"]` results, one MSG_SEND with both photoTokens). So the url's
photo id only ever served to index that one dict. Read it positionally.
Keying is deliberately not hardcoded to "0": whatever key comes back is fine,
as long as there is exactly one photo. A result with zero or several entries
now raises instead of guessing -- without an id in the URL there is no way to
tell which entry is ours, and silently attaching photo 1 of 2 to a message is
far worse than a failed upload.
Videos and files are untouched: they take url plus video_id/file_id and token
straight from their own response models and never parsed an id out of a URL.
Tests: the four new cases (no photoIds in the URL, a result keyed by photo id,
an ambiguous multi-photo result, an empty result, and a two-photo album being
two separate count=1 uploads) all fail against the previous code with
KeyError: 'photoIds' and pass here. The existing test was asserting the old
`?photoIds=photo-1` URL shape and no longer described the protocol.
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review. Walkthrough
ChangesЗагрузка фотографий
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to Photo uploads no longer depend on an ID in the upload URL. The inspected paths show no remaining issue that needs resolution before merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Photo uploads now reject empty or ambiguous results before publishing an attachment. The change is narrowly scoped, but correct photo association still depends on the remote upload service binding each response to its request. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Кролик проверил ответ, Comment |
Docstring coverage over the functions in this diff was 5/8. Adds them for `UploadService` (what the service is for, and that videos/files/voices only complete once the server emits their processing event), `UploadService.__init__`, `upload_photo` (a 105-line public method that had none -- what it requests, why the token is read positionally rather than by id, and everything that raises UploadError), and the empty-result test. 8/8 now.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MAX stopped returning a
photoIdsquery parameter on the photo upload URL, soparse_qs(urlparse(url).query)["photoIds"][0]raises KeyError and every photo upload fails withUploadError: Photo upload URL does not contain photoIds. Not intermittent, not rate limiting: no photo can upload at all. Reported as happening against the live API on 2026-09-30, ~2 days after the last successful photo forward.Captured from the official MAX client, one photo:
The official client annotates this itself ("phase 1/3 url has no media id").
The id was never needed. A multi-photo message is several independent
count: 1PHOTO_UPLOAD requests, each with its own one-shot URL and its own single-entry result, whose tokens then go out together in one MSG_SEND -- verified with a two-photo capture (seq 80/81 issued in parallel, two POSTs, twophotos["0"]results, one MSG_SEND with both photoTokens). So the url's photo id only ever served to index that one dict. Read it positionally.Keying is deliberately not hardcoded to "0": whatever key comes back is fine, as long as there is exactly one photo. A result with zero or several entries now raises instead of guessing -- without an id in the URL there is no way to tell which entry is ours, and silently attaching photo 1 of 2 to a message is far worse than a failed upload.
Videos and files are untouched: they take url plus video_id/file_id and token straight from their own response models and never parsed an id out of a URL.
Tests: the four new cases (no photoIds in the URL, a result keyed by photo id, an ambiguous multi-photo result, an empty result, and a two-photo album being two separate count=1 uploads) all fail against the previous code with KeyError: 'photoIds' and pass here. The existing test was asserting the old
?photoIds=photo-1URL shape and no longer described the protocol.Описание
Кратко, что делает этот PR. Например, добавляет новый метод, исправляет баг, улучшает документацию.
Тип изменений
Связанные задачи / Issue
Ссылка на issue, если есть: #
Тестирование
Покажите пример кода, который проверяет изменения: