react-native-documents / document-picker

Document Picker for React Native
https://react-native-documents.github.io/
MIT License
1.35k stars 437 forks source link

`fileCopyUri` has `file://` prefix on iOS but doesn't on Android #526

Closed kidroca closed 2 years ago

kidroca commented 2 years ago

Bug report

Summary

Using RN document picker to select a document with the copyTo option doesn't return the URI scheme on Android while it does for iOS

If you try to use the fileCopyUri to upload a file (using vanilla fetch) it works on iOS, but does not work on Android

Adding the file:// prefix, fixes the issue on android

Reproducible sample code

import RNDocumentPicker from 'react-native-document-picker';

const documentPickerOptions = {
    type: [RNDocumentPicker.types.allFiles],
    copyTo: 'cachesDirectory',
};

RNDocumentPicker.pick(documentPickerOptions)
  .then(documents => documents.map(({ fileCopyUri, ...doc })=> {
    const payload = {
        ...doc,
    };

    // When the URI is lacking uri scheme - file upload would fail
    const hasScheme = /^.+:\/\//.test(fileResult.uri);
    if (!hasScheme) {
        payload.uri = `file://${fileCopyUri}`;
    }

   return payload;
  });

Steps to reproduce

  1. Use Document Picker with copyTo: 'cachesDirectory' option
  2. Inspect fileCopyUri under Android - no file:/// prefix
  3. Inpsect fileCopyUri under iOS - has file:/// prefix

Describe what you expected to happen:

  1. Use Document Picker with copyTo: 'cachesDirectory' option
  2. fileCopyUri should behave the same way on all Platforms
    • preferable all should have file:/// prefix
    • alternatively none should have it

Environment info

npx react-native info output:

System:
    OS: Windows 10 10.0.19044
    CPU: (16) x64 AMD Ryzen 7 3700X 8-Core Processor
    Memory: 20.20 GB / 31.93 GB
  Binaries:
    Node: 14.18.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.15 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 6.14.15 - C:\Program Files\nodejs\npm.CMD
    Watchman: 20210110.135312.0 - D:\watchman-v2021.01.11.00-windows\bin\watchman.EXE
  SDKs:
    Android SDK:
      API Levels: 27, 28, 29, 30, 31
      Build Tools: 29.0.2, 30.0.2, 31.0.0
      System Images: android-29 | Google APIs Intel x86 Atom
      Android NDK: Not Found
    Windows SDK: Not Found
  IDEs:
    Android Studio: Not Found
    Visual Studio: 16.9.31025.194 (Visual Studio Community 2019)
  Languages:
    Java: Not Found
  npmPackages:
    @react-native-community/cli: 6.2.0 => 6.2.0 
    react: ^17.0.2 => 17.0.2 
    react-native: 0.66.4 => 0.66.4 
    react-native-windows: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

library version: 7.1.3

iOS / Android version: ios 15 / Android 10

kidroca commented 2 years ago

Perhaps we can use [toURI](https://developer.android.com/reference/java/io/File#toURI()) here instead of getAbsolutePath()

https://github.com/rnmods/react-native-document-picker/blob/c52e7a6d2a08f5506c23de86c1401775419f772c/android/src/main/java/com/reactnativedocumentpicker/DocumentPickerModule.java#L329

Ultimately this is returned in place of fileCopyUri

vonovak commented 2 years ago

Hello, thanks for reporting.

preferable all should have file:/// prefix

this sounds like the right solution, wdyt? Feel free to send over a PR. Having some before + after screenshots will likely speed up the review as I won't have to spend so much time testing and will be nice as documentation for everyone. Thank you :)

kidroca commented 2 years ago

preferable all should have file:/// prefix

this sounds like the right solution, wdyt?

I agree From what I've seen having file:/// works for image srcs and http calls Not having it still works for images but doesn't work for http calls

Having some before + after screenshots will likely speed up the review as I won't have to spend so much time testing and will be nice as documentation for everyone. Thank you :)

Alright, I'll prepare a PR tomorrow and include shots of fileCopyUri before and after the change

kidroca commented 2 years ago

PR ready: https://github.com/rnmods/react-native-document-picker/pull/527