search

apache / cordova-plugin-camera

Apache Cordova Plugin camera
https://cordova.apache.org/
Apache License 2.0
966 stars 1.55k forks source link

(android) catch RecoverableSecurityException #896

Closed byronaltice closed 1 month ago

byronaltice commented 3 months ago

Platforms affected

Android

Motivation and Context

781 (forked this PR and added additional changes)

679

660

656

Closes #679 Closes #660 Closes #679

Description

This is just to resolve the issues with #781 - there were formattting issues requested to be resolved, and fork needed synced with latest. That is the only thing additional changes with this PR over #781. Original PR text included below:

Catch the RecoverableSecurityException and Requests permission to delete the media from the media store. see https://developer.android.com/training/data-storage/shared/media#update-other-apps-files

Testing

Using a

Checklist

cfrank19 commented 1 month ago

I believe my team is running into this issue (extremely challenging to reproduce across different hardware) - any way we can get a release with this update to eliminate it as an option? @erisu @breautek

breautek commented 1 month ago

@byronaltice

Looking for some opinions... I have drafted https://github.com/apache/cordova-plugin-camera/pull/907 (https://github.com/apache/cordova-plugin-camera/pull/907/commits/242cd45be8e81bece437902176160ebc24a4119e for the actual changes specific to that PR) which completely removes checkForDuplicateImage and tracking the numpics. The details are in #907 but I'll summarise the rationale here.

The code obviously is attempting to determine if capturing an image is resulting in a duplicate entry in the gallery. I'm assuming this is if the user is also using saveToPhotoAlbum option on image captures, but the camera intent also stores the captured image in the gallery. It's also ancient, this was in the code base since the initial commit, so it's hard to say for certain. I can say in my testing, I'm not seeing any duplicates.

Overall the code works is it queries the media store and counts how many images are available when you start capturing a picture. And then it does it again at the end of processing the captured photo to determine if there was a duplicate.

The problem I have though is if you think about it logically, any app at any time can insert images or media into the gallery. If this occurs, it will obviously trip up these count checks. I don't see how this guarantees that it is even selecting the "duplicate" image to delete. I think it is possible that the camera could be attempting to delete a completely unrelated photo. Even if some camera apps insert the captured photo themselves, which would cause a duplicate if saveToPhotoAlbum is used, the way this handling that "duplicate" is unsafe, in my opinion.

Wondering if you have some time to analyse and see if you can come to the same conclusion?

breautek commented 1 month ago

This is made obsolete by #907 which removed the code that required handling RecoverablySecurityException