Yalantis / uCrop

Image Cropping Library for Android
https://yalantis.com/blog/introducing-ucrop-our-own-image-cropping-library-for-android/
11.86k stars 2.16k forks source link

uCrop doesn't handle Uri corrrectly and crashes in various circumstances on latest Android versions #390

Closed SMH17 closed 6 years ago

SMH17 commented 6 years ago

Unfortunately this library seems broken on more recent Android versions.

To reproduce the issue is enough to load a pic using share function from integrated Android File manager sample_screenshot

uCrop version: 2.2.1 Known affected Android versions: 7, 8.0, 8.1 (previous versions not tested)

Error:

java.lang.RuntimeException: An error occurred while executing doInBackground()
      at android.os.AsyncTask$3.done(AsyncTask.java:353)
      at java.util.concurrent.FutureTask.finishCompletion(FutureTask.java:383)
      at java.util.concurrent.FutureTask.setException(FutureTask.java:252)
      at java.util.concurrent.FutureTask.run(FutureTask.java:271)
      at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:245)
      at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
      at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
      at java.lang.Thread.run(Thread.java:764)
   Caused by: java.lang.NumberFormatException: For input string: "raw:/storage/emulated/0/Download/sampleimage.png"
      at java.lang.Long.parseLong(Long.java:590)
      at java.lang.Long.valueOf(Long.java:804)
      at com.yalantis.ucrop.util.FileUtils.getPath(FileUtils.java:159)
      at com.yalantis.ucrop.task.BitmapLoadTask.processInputUri(BitmapLoadTask.java:171)
      at com.yalantis.ucrop.task.BitmapLoadTask.doInBackground(BitmapLoadTask.java:90)
      at com.yalantis.ucrop.task.BitmapLoadTask.doInBackground(BitmapLoadTask.java:41)
      at android.os.AsyncTask$2.call(AsyncTask.java:333)
      at java.util.concurrent.FutureTask.run(FutureTask.java:266)
      at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:245) 
      at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162) 
      at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636) 
      at java.lang.Thread.run(Thread.java:764) 

Related issues:

318 #386

Suggestion: Use Android content resolver and InputStream rater than try to address manually filepaths, try to handle the Uri manually with FileUtils.java It's a very bad code practice that will leads to errors continuously and makes the code not well maintainable.

Legementarion commented 6 years ago

@SMH17 Hello! I saw your pull request. Could you say, after update dependencies in ucrop, crash still reproduce?

SMH17 commented 6 years ago

@Legementarion I have just pulled an update to dependencies that has nothing to do with this issue. The problem is related to the attempt to work directly on URI (that could have whatever form and you cannot hope to parse manually all of them) rater than using the content resolver and streams (that is the best practice that you should always apply when you need to work on paths in Android).

Legementarion commented 6 years ago

@SMH17 Ok, I understand you, I trying to reproduce it on android 7.0 with ucrop sample, but nothing happened, could you share me your code or project with this bug? Or if you want, I can create similar app for case written above in your issue, but it will happen on this weekends

SMH17 commented 6 years ago

@Legementarion Probably because you get the uri opening the image. To reproduce the issue you have to follow the screenshot exactly, passing the image Uri to the App with the share command from the stock Android file manager. The received Uri will have this format raw:/storage/emulated/0/Download/sampleimage.png that uCrop FileUtils is unable to process It correctly throwing this crash.

Legementarion commented 6 years ago

@SMH17 Ok, I change a little bit uCrop Sample, added intent filter and this at onCreate

   // Get intent, action and MIME type
        Intent intent = getIntent();
        String action = intent.getAction();
        String type = intent.getType();

        if (Intent.ACTION_SEND.equals(action) && type != null) {
            if (type.startsWith("image/")) {
                handleSendImage(intent); // Handle single image being sent
            }
        }

    private void handleSendImage(Intent intent) {
        Uri imageUri = intent.getParcelableExtra(Intent.EXTRA_STREAM);
        if (imageUri != null) {
            startCrop(imageUri);
        }
    }

and passing the image to the ucrop app, but I got uri like -

content://com.android.providers.downloads.documents/document/19

without any "raw" at start. Checked on lg g6 with android 7.0 and oneplus 3t with android 8.0 So everything alright. Do you do it same?

And could you say please your device model? Maybe it depend on it?

SMH17 commented 6 years ago

@Legementarion I have been able to reproduce this issue on Pixel (Oreo), G5 (Nougat), and also on Android Emulator (Oreo). But the Uri format changes according the section from where you share the picture in the "Files" App. Try to use the left side panel to change the section from recent, download ecc. .

Legementarion commented 6 years ago

@SMH17 Hi again! I have a good news for you. I checked your issue using uCrop Sample, but as you saw - to no avail. So I created new project and implement ucrop library, and tried again and yes, you are right, crash reproduced. The problem is in the different source code, last release was build 17 april of last year, so it doesnt contain last changes. So you can download source code of uCrop lib and add to project compile project(':ucrop') or just wait 2-3 days, until my pull request will approved and they update release build. Thank for patience and cooperation :)

Legementarion commented 6 years ago

Proudly want to say - Fixed at last version of Ucrop 2.2.2 :)