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 RequestBodyUtil for older version of Android (M or before) #28399

Closed

Conversation

daisy1754
Copy link
Contributor

@daisy1754 daisy1754 commented Mar 26, 2020

Summary:

This is bugfix for following code in RequestBodyUtil.java.

stream.getChannel().transferFrom(channel, 0, Long.MAX_VALUE);

This throws IllegalArgumentException in Android M or before. It seems in old version of JVM it internally casts third argument to integer so when you pass value that is larger than Integer.MAX_VALUE app would crash.

See:
https://bugs.openjdk.java.net/browse/JDK-5105464
https://stackoverflow.com/questions/55696035/thrown-illegalargumentexception-when-using-java-nio

Changelog:

[ANDROID] [FIXED] - Fix issue downloading request body via remote URL

Test Plan

Run following code on Android M or before. It would crash w/o this PR but it won't with this PR.

const body = new FormData();
const image = { uri: "https://reactnative.dev/img/showcase/facebook.png", path: null };
body.append('user[img]', fileBody(image));	
fetch("https://example.com", {
    method: 'POST',
    headers: {
    Accept: 'application/json',
    'Content-Type': 'multipart/form-data;',
    },
    body: body
});

@facebook-github-bot
Copy link
Contributor

Hi @daisy1754!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 26, 2020
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@analysis-bot
Copy link

analysis-bot commented Mar 26, 2020

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,620,845 +58
android hermes armeabi-v7a 7,933,518 +52
android hermes x86 9,106,846 +53
android hermes x86_64 8,961,653 +58
android jsc arm64-v8a 9,186,941 +98
android jsc armeabi-v7a 8,377,235 +99
android jsc x86 9,244,542 +97
android jsc x86_64 9,503,018 +102

Base commit: eb83356
Branch: main

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal 10,800,224 0

Base commit: 25f7aea

@daisy1754
Copy link
Contributor Author

This 7 line PR has been open for more than a month with no response. can someone review please? @mdvacca @fkgozali (randomly picked you two since you seemed to touch android native code recently)

@daisy1754
Copy link
Contributor Author

@axe-fb @RSNara can either of you help? (wanted to clean up my open PRs)

@github-actions
Copy link

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Apr 15, 2023
@daisy1754
Copy link
Contributor Author

:/ it's sad that this is being ignored, IMO such a simple fix that fixes easy-to-reproduce crash

@github-actions github-actions bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Apr 15, 2023
@cortinico cortinico force-pushed the feature/fix_request_body_util_m branch from 195f202 to fb951fd Compare April 17, 2023 18:38
@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Apr 18, 2023
@facebook-github-bot
Copy link
Contributor

@cortinico merged this pull request in 4b39f44.

jeongshin pushed a commit to jeongshin/react-native that referenced this pull request May 7, 2023
…ook#28399)

Summary:
This is bugfix for following code in RequestBodyUtil.java.

```
stream.getChannel().transferFrom(channel, 0, Long.MAX_VALUE);
```

This throws IllegalArgumentException in Android M or before. It seems in old version of JVM it internally casts third argument to integer so when you pass value that is larger than Integer.MAX_VALUE app would crash.

See:
https://bugs.openjdk.java.net/browse/JDK-5105464
https://stackoverflow.com/questions/55696035/thrown-illegalargumentexception-when-using-java-nio

## Changelog

[Android] [Fixed] Fix issue downloading request body via remote URL

Pull Request resolved: facebook#28399

Test Plan:
Run following code on Android M or before. It would crash w/o this PR but it won't with this PR.

```
const body = new FormData();
const image = { uri: "https://reactnative.dev/img/showcase/facebook.png", path: null };
body.append('user[img]', fileBody(image));
fetch("https://example.com", {
    method: 'POST',
    headers: {
    Accept: 'application/json',
    'Content-Type': 'multipart/form-data;',
    },
    body: body
});
```

Reviewed By: christophpurrer, cipolleschi

Differential Revision: D45057263

Pulled By: cortinico

fbshipit-source-id: e0306eb157a5aa92619ac51e67d106b8651a0ba7
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…ook#28399)

Summary:
This is bugfix for following code in RequestBodyUtil.java.

```
stream.getChannel().transferFrom(channel, 0, Long.MAX_VALUE);
```

This throws IllegalArgumentException in Android M or before. It seems in old version of JVM it internally casts third argument to integer so when you pass value that is larger than Integer.MAX_VALUE app would crash.

See:
https://bugs.openjdk.java.net/browse/JDK-5105464
https://stackoverflow.com/questions/55696035/thrown-illegalargumentexception-when-using-java-nio

## Changelog

[Android] [Fixed] Fix issue downloading request body via remote URL

Pull Request resolved: facebook#28399

Test Plan:
Run following code on Android M or before. It would crash w/o this PR but it won't with this PR.

```
const body = new FormData();
const image = { uri: "https://reactnative.dev/img/showcase/facebook.png", path: null };
body.append('user[img]', fileBody(image));
fetch("https://example.com", {
    method: 'POST',
    headers: {
    Accept: 'application/json',
    'Content-Type': 'multipart/form-data;',
    },
    body: body
});
```

Reviewed By: christophpurrer, cipolleschi

Differential Revision: D45057263

Pulled By: cortinico

fbshipit-source-id: e0306eb157a5aa92619ac51e67d106b8651a0ba7
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. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants