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.

Sign up for GitHub

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

Jump to bottom

[ANDROID][BUGFIX][BlobModule] Switch equality check in BlobModule.java #18893

Closed
wants to merge 1 commit into from
Closed

[ANDROID][BUGFIX][BlobModule] Switch equality check in BlobModule.java #18893

wants to merge 1 commit into from

Conversation

shockdesign
Copy link
Contributor

Switch the equality check to avoid crash on the first item. The check can be on a null object and return the correct result.

Fixes #18709

Test Plan

Just a simple switch on equals, to make sure we're not bombing out by having a null scheme.

Related PRs

No related PRs and does not require a document change.

Release Notes

[ANDROID][BUGFIX][BlobModule] Switch equality check in BlobModule.java

@shockdesign
Switch the equality check to avoid crash on the first item. The check…
ee8abd2 
… can be on a null object and return the correct result.
@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 Apr 17, 2018
@pull-bot
Copy link

Warnings
⚠️

📋 Release Notes - This PR may have incorrectly formatted Release Notes.

Generated by 🚫 dangerJS

@react-native-bot react-native-bot added Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted. Platform: Android Android applications. labels Apr 17, 2018
@thymikee
Copy link
Contributor

I think the right fix for it would be to throw a descriptive error or warning (after applying this fix), something like:

A URL "${url}" passed to native networking module didn't have a scheme (e.g. "https") defined. This happens, when fetch is called with "/endpoint" instead of "https://host/endpoint"

@shockdesign
Copy link
Contributor Author

@thymikee The scheme passed in doesn't have to be legitimate as this would just be determining whether the Blob is remote or not, if it's not there, or doesn't match it isn't defined as remote

@janicduplessis
Copy link
Contributor

Thanks! LGTM

@facebook-github-bot shipit

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Apr 17, 2018
facebook-github-bot
facebook-github-bot approved these changes Apr 17, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@janicduplessis is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

grabbou pushed a commit that referenced this pull request Apr 27, 2018
@shockdesign @grabbou
Switch equality check in BlobModule.java
5bb2e83 
Summary:
Switch the equality check to avoid crash on the first item. The check can be on a null object and return the correct result.

Fixes #18709

Just a simple switch on equals, to make sure we're not bombing out by having a null scheme.

No related PRs and does not require a document change.

[ANDROID][BUGFIX][BlobModule] Switch equality check in BlobModule.java
Closes #18893

Differential Revision: D7658036

Pulled By: hramos

fbshipit-source-id: db61b98dae178dbbb645070f7b0d73ab43d30541
macdoum1 pushed a commit to macdoum1/react-native that referenced this pull request Jun 28, 2018
@shockdesign @macdoum1
Switch equality check in BlobModule.java
7e7fb60 
Summary:
Switch the equality check to avoid crash on the first item. The check can be on a null object and return the correct result.

Fixes facebook#18709

Just a simple switch on equals, to make sure we're not bombing out by having a null scheme.

No related PRs and does not require a document change.

[ANDROID][BUGFIX][BlobModule] Switch equality check in BlobModule.java
Closes facebook#18893

Differential Revision: D7658036

Pulled By: hramos

fbshipit-source-id: db61b98dae178dbbb645070f7b0d73ab43d30541
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janicduplessis janicduplessis janicduplessis approved these changes

@facebook-github-bot facebook-github-bot facebook-github-bot approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved. Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted. Platform: Android Android applications.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@shockdesign @pull-bot @thymikee @janicduplessis @facebook-github-bot @react-native-bot