Skip to content
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

Choose a reasonable and valid number for the intent code #790

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

matan-h
Copy link

@matan-h matan-h commented Dec 11, 2023

This PR makes the Android file chooser use a reasonable and valid number in the intent code.
As this Stack Overflow post explains, the maximum value for the requestCode is 2^16 (65536).

Previously, the code used arbitrary numbers (123456 and 654321), which is making some phones encounter a Java Exception: java.lang.IllegalArgumentException: Can only use lower 16 bits for requestCode.

@Julian-O
Copy link
Contributor

I am not 100% sure I understand the context on this code, so let me explain what I think:

  • The save_code and result_code are meant to be unique identifiers, that show that the callback is related to this particular call.
  • The Pythonic way would be to use an object() call to guarantee uniqueness within the Python application. I believe the other callers are not coming from Python, so this technique isn't guaranteed.
  • Using a uuid would be another standard approach to avoid clashes. I am not clear whether there is a hardware limit that makes this impossible.
  • The old code just generates two random 6 digit numbers and just hopes there isn't a clash.
  • The old code just generates two random 5 digit numbers and just hopes there isn't a clash.

My comments:

  • 5 digits doesn't feel rare enough. There will be clashes in the field.
  • Your edit says 65536 is the maximum, but doesn't explain what is imposing this maximum. Given it is 2^16, it sounds like it must be an 16-bit OS limitation. If so, you should explain (link?) to why it is so limited. If not, how about 2^32 instead?
  • save_code and result_code can randomly be generated as the same number, which will cause problems. How about make the top end of the range of the first one one smaller, and then define the second one as the first one + 1? That way they never clash.

@matan-h matan-h changed the title Choose a reasonable and valid number for the intent code and use the uri variable Choose a reasonable and valid number for the intent code Dec 12, 2023
@matan-h
Copy link
Author

matan-h commented Dec 12, 2023

Thank you, @Julian-O. I've added an explanation to the code and edited the description of this pull request to make it cleaner.

I've also removed the other minor change as that would probably require a design change.
Here is the android code which creating this error in the new API:

@Julian-O
Copy link
Contributor

Thanks. I understand now.

[Just to be clear, I am not an official reviewer with merge rights. We'll need to wait for one of them.]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants