-
Notifications
You must be signed in to change notification settings - Fork 47
Fix Google Play signed APK download with smart checking #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 18 commits
0b89f48
12bdf8e
3f09f0d
6919b9d
306caff
058d1ab
75eea7c
38a66f9
5597c7d
d838b78
6446464
78f6ab6
05d0401
65e5dc2
308c13a
50a115b
5c09b10
54dd603
f325d5f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -386,8 +386,8 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||||||
| download-signed-apk: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| name: Download Google Play Signed APK | ||||||||||||||||||||||||||||||||||||||||||||||||||
| needs: [prepare, play-store-upload] | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if: ${{ vars.ENABLE_PLAY_STORE_UPLOAD == 'true' && vars.ENABLE_SIGNING == 'true' }} | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if: ${{ vars.ENABLE_PLAY_STORE_UPLOAD == 'true' && vars.ENABLE_SIGNING == 'true' }} | |
| if: vars.ENABLE_PLAY_STORE_UPLOAD == 'true' && vars.ENABLE_SIGNING == 'true' |
Outdated
Copilot
AI
Sep 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded variantId=2 is based on "previous observations" which makes the code fragile and unpredictable. This magic number should be documented with a clear explanation of why variant 2 is preferred, or the logic should be generalized to handle any variant.
Copilot
AI
Sep 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Direct URI manipulation is fragile and could break if the Google API client changes its URL format. Consider using the proper Google API client method for setting the alt=media parameter or using URL parsing libraries to handle query parameters safely.
| # Add alt=media parameter correctly (URL already has query params, so use &) | |
| if '?' in download_request.uri: | |
| download_request.uri += '&alt=media' | |
| else: | |
| download_request.uri += '?alt=media' | |
| # Add alt=media parameter robustly using urllib.parse | |
| from urllib.parse import urlparse, parse_qs, urlencode, urlunparse | |
| parsed_url = urlparse(download_request.uri) | |
| query = parse_qs(parsed_url.query) | |
| query['alt'] = ['media'] | |
| new_query = urlencode(query, doseq=True) | |
| download_request.uri = urlunparse(parsed_url._replace(query=new_query)) |
Copilot
AI
Sep 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Direct URI manipulation is fragile and error-prone. Consider using URL parsing libraries like urllib.parse to properly handle query parameters instead of string concatenation.
| # Add alt=media parameter correctly (URL already has query params, so use &) | |
| if '?' in download_request.uri: | |
| download_request.uri += '&alt=media' | |
| else: | |
| download_request.uri += '?alt=media' | |
| # Add alt=media parameter correctly using urllib.parse | |
| from urllib.parse import urlparse, parse_qs, urlencode, urlunparse | |
| parsed = urlparse(download_request.uri) | |
| query = parse_qs(parsed.query) | |
| query['alt'] = ['media'] | |
| new_query = urlencode(query, doseq=True) | |
| download_request.uri = urlunparse(parsed._replace(query=new_query)) |
Copilot
AI
Sep 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The imports are placed in the middle of the function rather than at the top of the script. Move these imports to the top of the script with the other imports for better code organization and readability.
Copilot
AI
Sep 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import statements should be placed at the top of the script with other imports (lines 414-419) rather than in the middle of the function for better code organization and readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition is checking for both
ENABLE_PLAY_STORE_UPLOADandENABLE_SIGNINGbut this job specifically downloads APKs from Play Store, not signing them. TheENABLE_SIGNINGcondition may be incorrect here since Play Store handles the signing.