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

Bugfix change shares database USER_ID type to String #12753

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

JonasMayerDev
Copy link
Collaborator

The database type for the owners userid for shares was int instead of String (which was assumed everywhere, like "getString" etc.). It seemd to have worked well for short userids but for large userids the data will not be correctly saved. Ids like "12320078498127907419284723" would be converted to "1.23200e24" which leads to following misbehavior for example:

Screen_recording_20240327_220052.webm

Now database type is String. To test this fix / to experience the issue, you need to be logged in as a user with a really long userid (=username / loginname) and share a file or folder with another person on the instance.

Screen_recording_20240327_220139.webm
  • Tests written, or not not needed

Copy link

Codacy

Lint

TypemasterPR
Warnings7171
Errors33

SpotBugs

CategoryBaseNew
Bad practice6767
Correctness6868
Dodgy code350350
Experimental11
Internationalization77
Malicious code vulnerability22
Multithreaded correctness66
Performance5858
Security1919
Total578578

Copy link

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12753.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

Copy link
Collaborator

@ZetaTom ZetaTom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works and fixes the issue.

Copy link

blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

Copy link
Collaborator

@alperozturk96 alperozturk96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on Android 14 emulator. Didn't crash.

@tobiasKaminsky tobiasKaminsky merged commit e036471 into master Apr 2, 2024
18 of 20 checks passed
@delete-merged-branch delete-merged-branch bot deleted the change_userid_type branch April 2, 2024 08:14
@JonasMayerDev
Copy link
Collaborator Author

/backport to stable-3.28

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.

4 participants