Fixed and added support for dataUri in form data #33675
Conversation
This reverts commit 0ef27c2.
facebook-github-bot
added
CLA Signed
react-native-bot
added
Platform: Android
Base commit: 018d5cf |
Base commit: 018d5cf |
|
@cortinico Can you please review this PR |
| byte[] decodedDataUrString = Base64.decode(fileContentUriStr.split(",")[1], Base64.DEFAULT); | ||
| Bitmap bitMap = BitmapFactory.decodeByteArray(decodedDataUrString, 0, decodedDataUrString.length); | ||
| ByteArrayOutputStream bytes = new ByteArrayOutputStream(); | ||
| bitMap.compress(Bitmap.CompressFormat.JPEG, 100, bytes); |
There was a problem hiding this comment.
This won't work as it as you're compressing with JPG an image that could be potentially a PNG/GIF or other formats.
There was a problem hiding this comment.
@cortinico compressFormat also supports webp and PNG! We can add them too!?
There was a problem hiding this comment.
It seems like the correct approach would be:
bitMap.compress(CompressFormat.PNG, 0 /* ignored for PNG */, bytes);
There was a problem hiding this comment.
@cortinico Thank you for the review!, Do you mean something like this?
if(format is PNG)
bitMap.compress(CompressFormat.PNG, 0 /* ignored for PNG */, bytes);
if(format is JPG)
bitMap.compress(CompressFormat.JPG, 0 /* ignored for JPG */, bytes);
if(format is WEBP)
bitMap.compress(CompressFormat.WEBP, 0 /* ignored for WEBP */, bytes);
There was a problem hiding this comment.
It seems like the correct approach would be:
bitMap.compress(CompressFormat.PNG, 0 /* ignored for PNG */, bytes);
Or should we do this just for PNG format, Sorry I didn’t completely get your approach
There was a problem hiding this comment.
The snippet I suggested essentially ignores compression, so you don't need to check the format at all
There was a problem hiding this comment.
@cortinico I've made the changes. does the changes looks good?
|
A test is failing, I don’t think the failure is related to my changes! Is there anything else I need to do? |
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cortinico Internal tests are failing. Do i need to do make any corrections!? Or it's nothing to worry about |
|
Nope I'll take care of it next week 👍 |
|
@cortinico Okay Thank you |
|
This pull request was successfully merged by @hetanthakkar1 in c663c0e. When will my fix make it into a release? | Upcoming Releases |
Summary
Continuation of #33548
Fixes #25790. The issue resulted because the getFileInputStream() method of RequestBodyUtil failed to return an inputStream of the given dataUri This happened because the openInputStream() method of context.getContentResolver() expects a content or a file in its parameter but instead received dataUri which resulted in FileNotFoundException.
Solution:
I've now converted the dataUri to bitmap and then attached an inputStream to the compressed bitmap. This way we won't have to store the image temporarily. I think converting the dataUri to Bitmap is necessary as there is no method that lets us convert the dataUri to inputStream directly. And regarding large size images, converting them to bitmap is the only(efficient) step I can think of, as the conversion is handled by in-built java function.
This issue has been unresolved for quite some time now, so resolving this PR would be greatly appreciated.
Changelog
[Android] [Added] - Support for dataUri in form data
Test Plan