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

Only execute WifiManager.getConnectionInfo if ACCESS_WIFI_STATE is granted #328

Conversation

sweggersen
Copy link
Contributor

@sweggersen sweggersen commented Mar 19, 2020

Overview

The motivation behind this PR is to allow apps to remove ACCESS_WIFI_STATE permission in their app, without the app crashing. We have a case in one of our partner apps where we use netinfo for some network logging, but we don't use all the data collected by netinfo, e.g. wifi data.
We shouldn't crash because of missing permissions, just reduce the data collected.

Test Plan

Before this change:
AndroidManifest.xml in the Android app

 <uses-permission android:name="android.permission.ACCESS_WIFI_STATE" tools:node="remove" />

logcat:

 java.lang.SecurityException: WifiService: Neither user 10403 nor current process has android.permission.ACCESS_WIFI_STATE.
at android.os.Parcel.createException(Parcel.java:2088)
at android.os.Parcel.readException(Parcel.java:2056)
at android.os.Parcel.readException(Parcel.java:2004)
at android.net.wifi.IWifiManager$Stub$Proxy.getConnectionInfo(IWifiManager.java:3405)
at android.net.wifi.WifiManager.getConnectionInfo(WifiManager.java:2727)
at com.reactnativecommunity.netinfo.ConnectivityReceiver.d(SourceFile:127)
at com.reactnativecommunity.netinfo.ConnectivityReceiver.getCurrentState(SourceFile:56)
at com.reactnativecommunity.netinfo.NetInfoModule.getCurrentState(SourceFile:50)
at java.lang.reflect.Method.invoke(Native Method)
at com.facebook.react.bridge.JavaMethodWrapper.invoke(SourceFile:371)
at com.facebook.react.bridge.JavaModuleWrapper.invoke(SourceFile:150)
at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
at android.os.Handler.handleCallback(Handler.java:883)
at android.os.Handler.dispatchMessage(Handler.java:100)
at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(SourceFile:26)
at android.os.Looper.loop(Looper.java:237)
at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(SourceFile:225)
at java.lang.Thread.run(Thread.java:919)

After this change:
🟢

@sweggersen
Copy link
Contributor Author

cc @tido64

@matt-oakes
Copy link
Collaborator

Thanks for the PR. This looks great! Merging and releasing now 👍

@matt-oakes matt-oakes merged commit ba16e0a into react-native-netinfo:master Mar 30, 2020
react-native-community-bot pushed a commit that referenced this pull request Mar 30, 2020
# [5.7.0](v5.6.2...v5.7.0) (2020-03-30)

### Bug Fixes

* **macos:** Ensure macOS files are included in the NPM package ([#335](#335) by [@matt-oakes](https://github.com/matt-oakes)) ([742c79a](742c79a))

### Features

* **android:** Make the ACCESS_WIFI_STATE permission optional ([#328](#328) by [@sweggersen](https://github.com/sweggersen)) ([ba16e0a](ba16e0a))
@react-native-community-bot
Copy link
Collaborator

🎉 This PR is included in version 5.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants