Критические исправления сокета/входа #37
Conversation
📝 WalkthroughWalkthroughRemoved language from auth payloads and public methods, added internal phone-splitting helper, changed SSL-wrapped socket to blocking during handshake, and bumped default app version and build number. (45 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pymax/mixins/auth.py (2)
33-72: Critical: Phone formatting assumes fixed 12-character length.Lines 52-54 format the phone number by hardcoding slice indices for exactly 12 characters:
split_phone = phone[0:2] + " " + phone[2:5] + " " + phone[5:8] + " " + phone[8:10] + " " + phone[10:12]However,
PHONE_REGEXfrom constant.py allows phone numbers with 10-15 digits (with optional+). This will cause anIndexErrorfor phone numbers that don't have exactly 12 characters, or produce incorrect formatting for numbers with more than 12 characters.The example in the PR uses
+79221231234(12 characters), but this logic won't handle other valid formats.🔎 Proposed fix with length validation
async def request_code(self, phone: str) -> str: """ Запрашивает код аутентификации для указанного номера телефона и возвращает временный токен. Метод отправляет запрос на получение кода верификации на переданный номер телефона. Используется в процессе аутентификации или регистрации. :param phone: Номер телефона в международном формате. :type phone: str :return: Временный токен для дальнейшей аутентификации. :rtype: str :raises ValueError: Если полученные данные имеют неверный формат. :raises Error: Если сервер вернул ошибку. .. note:: Используется только в пользовательском flow аутентификации. """ self.logger.info("Requesting auth code") - split_phone = phone[0:2] + " " + phone[2:5] + " " + phone[5:8] + " " + phone[8:10] + " " + phone[10:12] + if len(phone) != 12: + raise ValueError(f"Phone number must be exactly 12 characters, got {len(phone)}") + split_phone = f"{phone[0:2]} {phone[2:5]} {phone[5:8]} {phone[8:10]} {phone[10:12]}" payload = RequestCodePayload( phone=split_phone, type=AuthType.START_AUTH ).model_dump(by_alias=True)Alternative: If the server requires this specific format, consider creating a helper method to validate and format phone numbers, then reuse it in both
request_codeandresend_code.
74-107: Critical: Duplicate phone formatting logic with same IndexError risk.The same hardcoded 12-character phone formatting logic from
request_codeis duplicated here on line 87. This has the identical bug: it will fail withIndexErrorfor phone numbers that aren't exactly 12 characters.🔎 Proposed fix: Extract formatting logic into a helper method
Add a helper method to avoid duplication:
def _format_phone_for_api(self, phone: str) -> str: """Format phone number for API request (requires exactly 12 characters).""" if len(phone) != 12: raise ValueError(f"Phone number must be exactly 12 characters, got {len(phone)}") return f"{phone[0:2]} {phone[2:5]} {phone[5:8]} {phone[8:10]} {phone[10:12]}"Then use it in both methods:
async def request_code(self, phone: str) -> str: self.logger.info("Requesting auth code") - split_phone = phone[0:2] + " " + phone[2:5] + " " + phone[5:8] + " " + phone[8:10] + " " + phone[10:12] + split_phone = self._format_phone_for_api(phone) payload = RequestCodePayload( phone=split_phone, type=AuthType.START_AUTH ).model_dump(by_alias=True)async def resend_code(self, phone: str) -> str: self.logger.info("Resending auth code") - split_phone = phone[0:2] + " " + phone[2:5] + " " + phone[5:8] + " " + phone[8:10] + " " + phone[10:12] + split_phone = self._format_phone_for_api(phone) payload = RequestCodePayload( phone=split_phone, type=AuthType.RESEND ).model_dump(by_alias=True)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/pymax/mixins/auth.pysrc/pymax/mixins/socket.pysrc/pymax/payloads.pysrc/pymax/static/constant.py
💤 Files with no reviewable changes (1)
- src/pymax/payloads.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/pymax/mixins/auth.py (2)
src/pymax/payloads.py (1)
RequestCodePayload(56-58)src/pymax/static/enum.py (1)
AuthType(173-177)
🪛 Ruff (0.14.10)
src/pymax/static/constant.py
80-80: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
81-81: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
🔇 Additional comments (4)
src/pymax/mixins/socket.py (1)
197-221: SSL handshake blocking mode: defensive but reasonable.The explicit
setblocking(True)on line 215 after wrapping ensures the SSL socket operates in blocking mode, even though the raw socket was already configured this way on line 204. While this appears redundant, it's defensive coding that guarantees consistency regardless ofwrap_socketimplementation details.Since all socket I/O operations use
run_in_executor(lines 256, 300, 321, 490), blocking mode is the correct choice and aligns with the PR's goal of improving socket stability.src/pymax/static/constant.py (2)
83-83: Verify the new build number is valid.The build number has been updated from 0x97CB (38859) to 0x9E2A (40490). Confirm this build number is recognized by the server and aligns with the updated app version.
79-79: Update README examples to reflect the new app version.The version "25.21.0" passes all internal validation checks (requirement is ≥ "25.12.13"), but the documentation examples in README.md (lines 68 and 83) still reference the outdated "25.12.13" version and should be updated to match.
src/pymax/mixins/auth.py (1)
33-33: No language parameter removal detected in current code.The review comment asserts that a
languageparameter has been removed fromrequest_codeandresend_code, but the current method signatures at lines 33 and 74 show only aphone: strparameter. TheRequestCodePayloadclass contains onlyphoneandtypefields with nolanguagefield. Additionally, there are zero references to "language" throughout the file, and all existing calls to these methods use only the phone parameter.Likely an incorrect or invalid review comment.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
2 participants
Описание
Делает сокет более стабильным и обновляет вход до последнего API.
Тип изменений
Связанные задачи / Issue
Ссылка на issue, если есть: #
Тестирование
Покажите пример кода, который проверяет изменения:
Summary by CodeRabbit
Updates
New Features
Bug Fixes
Breaking Changes
✏️ Tip: You can customize this high-level summary in your review settings.