-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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 port as -1 if dev server without specifying port on Android #34705
Conversation
# Why when load js bundle from a URL without an explicit port, e.g. http://www.example.com/ , the app will crash. the root cause is that `android.net.Uri.getPort()` will return -1 when there's no port. related facebook/react-native#34705 and ENG-5932 # How use `okhttp3.HttpUrl.defaultPort(scheme)` as fallback when port is -1. since the parameter in `injectReactInterceptor` is already android.net.Uri, i don't want to use HttpUrl to parse it again. only change the default port would be lower risk. # Test Plan test on bare-expo 1. [enable dev-launcher](https://github.com/expo/expo/blob/7331e13192c5b798ec67ec180949e1ef4e4da228/apps/bare-expo/android/app/src/main/java/dev/expo/payments/MainApplication.java#L25) 2. `EXPO_NO_DEFAULT_PORT=1 npx expo start --tunnel` 3. in dev-launcher, load the bundle url, e.g. `exp://nquwmbi.kudochien.19000.exp.direct`
# Why when load js bundle from a URL without an explicit port, e.g. http://www.example.com/ , the app will crash. the root cause is that `android.net.Uri.getPort()` will return -1 when there's no port. related facebook/react-native#34705 and ENG-5932 # How use `okhttp3.HttpUrl.defaultPort(scheme)` as fallback when port is -1. since the parameter in `injectReactInterceptor` is already android.net.Uri, i don't want to use HttpUrl to parse it again. only change the default port would be lower risk. # Test Plan test on bare-expo 1. [enable dev-launcher](https://github.com/expo/expo/blob/7331e13192c5b798ec67ec180949e1ef4e4da228/apps/bare-expo/android/app/src/main/java/dev/expo/payments/MainApplication.java#L25) 2. `EXPO_NO_DEFAULT_PORT=1 npx expo start --tunnel` 3. in dev-launcher, load the bundle url, e.g. `exp://nquwmbi.kudochien.19000.exp.direct`
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Hey @Kudo, sadly this PR is failing internally for some of our apps with:
I believe that |
thanks @cortinico! any chances for you to update okhttp3 internally? i think the version is older than okhttp 3.11.0 and that was four years ago. i don't want to convert String -> {URL,Uri} -> HttpUrl parse (that would call {URL,Uri}.toString() internally). |
i find there's |
That's great! I could look into updating OkHTTP but the failure is happening in 3/4 different apps and it could take me some time. I'll re-import your change and give it a go 👍 |
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This pull request was successfully merged by @Kudo in 3d7e138. When will my fix make it into a release? | Upcoming Releases |
thanks @cortinico for landing this 🚀 |
# Why when load js bundle from a URL without an explicit port, e.g. http://www.example.com/ , the app will crash. the root cause is that `android.net.Uri.getPort()` will return -1 when there's no port. related facebook/react-native#34705 and ENG-5932 # How use `okhttp3.HttpUrl.defaultPort(scheme)` as fallback when port is -1. since the parameter in `injectReactInterceptor` is already android.net.Uri, i don't want to use HttpUrl to parse it again. only change the default port would be lower risk. # Test Plan test on bare-expo 1. [enable dev-launcher](https://github.com/expo/expo/blob/7331e13192c5b798ec67ec180949e1ef4e4da228/apps/bare-expo/android/app/src/main/java/dev/expo/payments/MainApplication.java#L25) 2. `EXPO_NO_DEFAULT_PORT=1 npx expo start --tunnel` 3. in dev-launcher, load the bundle url, e.g. `exp://nquwmbi.kudochien.19000.exp.direct`
Summary: when specifying dev server without port, e.g. http://www.example.com/, there are some issues. 1. redbox error <img src="https://user-images.githubusercontent.com/46429/190540390-8ee420f2-7642-427b-9f2e-e0c6d31015f8.png" width="30%"> 2. showing -1 in loading view <img src="https://user-images.githubusercontent.com/46429/190540727-158f35ad-359f-443a-a4b0-768dd2f7e400.png" width="50%"> the root cause is coming from [`java.net.URL.getPort()` will return -1 when the url doesn't have a port](https://developer.android.com/reference/java/net/URL#getPort()). this pr replaces the parser to [`okhttp3.HttpUrl`](https://square.github.io/okhttp/4.x/okhttp/okhttp3/-http-url/#port) that it will have default port 80 for http or port 443 for https. the two call paths should only serve http/https address, not file:// address. it should be safe to change from java.net.URL to okhttp3.HttpUrl. not fully related, in the case above, android will connect to `ws://www.example.com/:8097` for react-devtools we should strip the trailing slash in *setUpReactDevTools.js* ## Changelog [Android] [Fixed] - Fix port as -1 if dev server without specifying port on Android Pull Request resolved: #34705 Test Plan: test on rn-tester with the following steps 1. `yarn start` 2. open another terminal and run `ngrok http 8081` and it will return a tunnel url, e.g. `71a1-114-36-194-97.jp.ngrok.io` 3. open dev setting in app and change the dev server to `71a1-114-36-194-97.jp.ngrok.io` 5. reload the app Reviewed By: cipolleschi Differential Revision: D39573988 Pulled By: cortinico fbshipit-source-id: 397df90ab30533207bd87a3f069132d97c22c7fd
…book#34705) Summary: when specifying dev server without port, e.g. http://www.example.com/, there are some issues. 1. redbox error <img src="https://user-images.githubusercontent.com/46429/190540390-8ee420f2-7642-427b-9f2e-e0c6d31015f8.png" width="30%"> 2. showing -1 in loading view <img src="https://user-images.githubusercontent.com/46429/190540727-158f35ad-359f-443a-a4b0-768dd2f7e400.png" width="50%"> the root cause is coming from [`java.net.URL.getPort()` will return -1 when the url doesn't have a port](https://developer.android.com/reference/java/net/URL#getPort()). this pr replaces the parser to [`okhttp3.HttpUrl`](https://square.github.io/okhttp/4.x/okhttp/okhttp3/-http-url/#port) that it will have default port 80 for http or port 443 for https. the two call paths should only serve http/https address, not file:// address. it should be safe to change from java.net.URL to okhttp3.HttpUrl. not fully related, in the case above, android will connect to `ws://www.example.com/:8097` for react-devtools we should strip the trailing slash in *setUpReactDevTools.js* ## Changelog [Android] [Fixed] - Fix port as -1 if dev server without specifying port on Android Pull Request resolved: facebook#34705 Test Plan: test on rn-tester with the following steps 1. `yarn start` 2. open another terminal and run `ngrok http 8081` and it will return a tunnel url, e.g. `71a1-114-36-194-97.jp.ngrok.io` 3. open dev setting in app and change the dev server to `71a1-114-36-194-97.jp.ngrok.io` 5. reload the app Reviewed By: cipolleschi Differential Revision: D39573988 Pulled By: cortinico fbshipit-source-id: 397df90ab30533207bd87a3f069132d97c22c7fd
Summary
when specifying dev server without port, e.g. http://www.example.com/, there are some issues.
the root cause is coming from
java.net.URL.getPort()
will return -1 when the url doesn't have a port.this pr replaces the parser tookhttp3.HttpUrl
that it will have default port 80 for http or port 443 for https. the two call paths should only serve http/https address, not file:// address. it should be safe to change from java.net.URL to okhttp3.HttpUrl.Update: adding
java.net.URL.getDefaultPort
fallback when the default port is -1not fully related, in the case above, android will connect to
ws://www.example.com/:8097
for react-devtoolswe should strip the trailing slash in setUpReactDevTools.js
Changelog
[Android] [Fixed] - Fix port as -1 if dev server without specifying port on Android
Test Plan
test on rn-tester with the following steps
yarn start
ngrok http 8081
and it will return a tunnel url, e.g.71a1-114-36-194-97.jp.ngrok.io
71a1-114-36-194-97.jp.ngrok.io