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

fix fetch of content scheme urls failing on Android #50122

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

Conversation

giantslogik
Copy link
Contributor

@giantslogik giantslogik commented Mar 19, 2025

Summary:

This PR fixes #48762

  • fix creating Blobs from Android 'content://' scheme urls was failing on the js side due to this check

Changelog:

[ANDROID] [FIXED] - fix fetch of content scheme uris failing on Android.

Test Plan:

Used the App here to test Android Blob creation. https://github.com/giantslogik/blob-large-file-fetch.

EDIT: Added tester to RNTester

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Mar 19, 2025
@giantslogik
Copy link
Contributor Author

PR created by splitting #48766 into two.

@javache @cortinico

@javache
Copy link
Member

javache commented Mar 19, 2025

Could you add or link a test-case to RNTester that shows what's broken here, and how this fixes it?

@giantslogik
Copy link
Contributor Author

giantslogik commented Mar 19, 2025

Could you add or link a test-case to RNTester that shows what's broken here, and how this fixes it?

@javache I got RNTester built and running. I wasn't able to find an existing test case that provides tests for Blob / networking. Ideally to add a test case i would need to use a third-party library to get a valid url for a Video/Photo on the Android device's gallery. It would be inappropriate to add a library like '@react-native-camera-roll/camera-roll' to the RNTester package.json.

Thought or advise on how to proceed ?

As noted in the issue #48762 , to show whats broken use the Reproducer:
https://github.com/giantslogik/blob-large-file-fetch .

Steps to reproduce:

  1. Install the application with yarn android
  2. Select a (non-large) file by granting limited permissions. This should provide a url using the 'content://' scheme on newer Androids (API 35 for me)
  3. Crashes in the js side of fetch

@cortinico
Copy link
Contributor

It would be inappropriate to add a library like '@react-native-camera-roll/camera-roll' to the RNTester package.json.
Thought or advise on how to proceed ?

Adding external libs to RNTester is something we tend to avoid as it makes our install time slower + there is some integration costs as RNTester lives inside our monorepo. So we'll have to make sure that all the deps imported by @react-native-camera-roll/camera-roll don't conflict with the one we already have.

Perhaps we can have a really simple module to pick an image. For example on Android we can use an Intent with Intent.ACTION_GET_CONTENT to get an image

@giantslogik
Copy link
Contributor Author

@javache @cortinico
Added a test case to RN tester:
Screenshot 2025-03-27 at 12 58 09 PMScreenshot 2025-03-27 at 12 58 28 PM

Screenshot 2025-03-27 at 1 04 53 PM

WITHOUT FIX: #48762
Screenshot 2025-03-27 at 12 58 47 PM

WITH FIX:
Screenshot 2025-03-27 at 1 00 01 PM

@giantslogik giantslogik force-pushed the content_url_fix branch 3 times, most recently from 1f85400 to f815598 Compare March 27, 2025 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fetch does not work with the android content:// uri scheme
4 participants