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 security exception (Nearby places not loading after permissions granted) #1440

Merged
merged 29 commits into from
Apr 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e9702a8
Merge remote-tracking branch 'refs/remotes/commons-app/2.7.x-release'…
misaochan Mar 28, 2018
b9beea4
Merge remote-tracking branch 'refs/remotes/commons-app/2.7.x-release'…
misaochan Mar 30, 2018
b73de9c
Update changelog.md
misaochan Mar 30, 2018
31e6606
Versioning for v2.7.0
misaochan Mar 30, 2018
b14de2c
Merge remote-tracking branch 'refs/remotes/commons-app/2.7.x-release'…
misaochan Apr 3, 2018
dd42d68
Merge branch '2.7.x-release-fork' of https://github.com/misaochan/app…
misaochan Apr 3, 2018
338189c
Merge remote-tracking branch 'refs/remotes/commons-app/2.7.x-release'…
misaochan Apr 4, 2018
45512e7
Merge remote-tracking branch 'refs/remotes/commons-app/2.7.x-release'…
misaochan Apr 5, 2018
ce5467c
Merge remote-tracking branch 'refs/remotes/commons-app/2.7.x-release'…
misaochan Apr 14, 2018
54bde96
Add logging to onPermissionsRequestResult
misaochan Apr 14, 2018
024ceeb
Request location updates onStatusChanged
misaochan Apr 14, 2018
c4a00e9
Copy onResume() actions into onPermissionsRequestResult
misaochan Apr 14, 2018
cce7ac6
Added getLastKnownLocation method and hooked it up to refreshView
misaochan Apr 14, 2018
0d114a6
Remove unnecessary calls, add more logging
misaochan Apr 14, 2018
f40875e
Add check to prevent NPE
misaochan Apr 14, 2018
6ce2b7c
Check that curLatLng exists before getting Mapbox instance
misaochan Apr 14, 2018
c0cf76e
Moar logging
misaochan Apr 14, 2018
542c372
Make curLatLang clearer
misaochan Apr 14, 2018
a6e8b1d
Not a good hack - put curLatLang into the bundle separately
misaochan Apr 14, 2018
1b8e841
Add TODO
misaochan Apr 14, 2018
3f013e1
Merge remote-tracking branch 'refs/remotes/commons-app/2.7.x-release'…
misaochan Apr 18, 2018
94b5482
Rename variables for clarity
misaochan Apr 18, 2018
b7cd435
Check for Network Provider as well, tidy up getLKL()
misaochan Apr 18, 2018
ba8c3b5
Add Javadocs
misaochan Apr 18, 2018
d6cec36
Remove unnecessary method in onStatusChanged
misaochan Apr 18, 2018
f0357fc
Add checkGPS comment
misaochan Apr 18, 2018
209cb53
Remove unnecessary logs
misaochan Apr 18, 2018
93ec7bc
Add TODO
misaochan Apr 18, 2018
b200a29
Call bundle.clear() before inserting CurLatLng
misaochan Apr 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,23 @@ public boolean isPermissionExplanationRequired(Activity activity) {
Manifest.permission.ACCESS_FINE_LOCATION);
}

/**
* Gets the last known location in cases where there wasn't time to register a listener
* (e.g. when Location permission just granted)
* @return last known LatLng
*/
public LatLng getLKL() {
if (ContextCompat.checkSelfPermission(context, Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED) {
Location lastKL = locationManager.getLastKnownLocation(LocationManager.GPS_PROVIDER);
if (lastKL == null) {
lastKL = locationManager.getLastKnownLocation(LocationManager.NETWORK_PROVIDER);
}
return LatLng.from(lastKL);
} else {
return null;
}
}

public LatLng getLastLocation() {
if (lastLocation == null) {
return null;
Expand Down Expand Up @@ -249,6 +266,7 @@ public void onProviderDisabled(String provider) {
public enum LocationChangeType{
LOCATION_SIGNIFICANTLY_CHANGED, //Went out of borders of nearby markers
LOCATION_SLIGHTLY_CHANGED, //User might be walking or driving
LOCATION_NOT_CHANGED
LOCATION_NOT_CHANGED,
PERMISSION_JUST_GRANTED
}
}
36 changes: 26 additions & 10 deletions app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@

import android.support.v4.app.FragmentTransaction;
import android.support.v7.app.AlertDialog;
import android.util.Log;
import android.view.Menu;
import android.view.MenuInflater;
import android.view.MenuItem;
import android.view.View;
import android.widget.LinearLayout;
import android.widget.ProgressBar;
import android.widget.Toast;

import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
Expand Down Expand Up @@ -81,6 +79,7 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp

private final String NETWORK_INTENT_ACTION = "android.net.conn.CONNECTIVITY_CHANGE";
private BroadcastReceiver broadcastReceiver;
private LatLng lastKnownLocation;

@Override
protected void onCreate(Bundle savedInstanceState) {
Expand Down Expand Up @@ -158,7 +157,11 @@ public void onRequestPermissionsResult(int requestCode, @NonNull String[] permis
switch (requestCode) {
case LOCATION_REQUEST: {
if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
refreshView(LocationServiceManager.LocationChangeType.LOCATION_SIGNIFICANTLY_CHANGED);
Timber.d("Location permission granted, refreshing view");
//Still need to check if GPS is enabled
checkGps();
lastKnownLocation = locationManager.getLKL();
refreshView(LocationServiceManager.LocationChangeType.PERMISSION_JUST_GRANTED);
} else {
//If permission not granted, go to page that says Nearby Places cannot be displayed
hideProgressBar();
Expand Down Expand Up @@ -347,21 +350,33 @@ private void refreshView(LocationServiceManager.LocationChangeType locationChang
}
curLatLang = lastLocation;

if (locationChangeType.equals(LocationServiceManager.LocationChangeType.PERMISSION_JUST_GRANTED)) {
curLatLang = lastKnownLocation;
}

if (curLatLang == null) {
Timber.d("Skipping update of nearby places as location is unavailable");
return;
}

if (locationChangeType
.equals(LocationServiceManager.LocationChangeType.LOCATION_SIGNIFICANTLY_CHANGED)) {
if (locationChangeType.equals(LocationServiceManager.LocationChangeType.LOCATION_SIGNIFICANTLY_CHANGED)
|| locationChangeType.equals(LocationServiceManager.LocationChangeType.PERMISSION_JUST_GRANTED)) {
progressBar.setVisibility(View.VISIBLE);

//TODO: This hack inserts curLatLng before populatePlaces is called (see #1440). Ideally a proper fix should be found
Gson gson = new GsonBuilder()
.registerTypeAdapter(Uri.class, new UriSerializer())
.create();
String gsonCurLatLng = gson.toJson(curLatLang);
bundle.clear();
bundle.putString("CurLatLng", gsonCurLatLng);

placesDisposable = Observable.fromCallable(() -> nearbyController
.loadAttractionsFromLocation(curLatLang))
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.subscribe(this::populatePlaces);
} else if (locationChangeType
.equals(LocationServiceManager.LocationChangeType.LOCATION_SLIGHTLY_CHANGED)) {
} else if (locationChangeType.equals(LocationServiceManager.LocationChangeType.LOCATION_SLIGHTLY_CHANGED)) {
Gson gson = new GsonBuilder()
.registerTypeAdapter(Uri.class, new UriSerializer())
.create();
Expand All @@ -384,21 +399,22 @@ private void populatePlaces(NearbyController.NearbyPlacesInfo nearbyPlacesInfo)
if (placeList.size() == 0) {
ViewUtil.showSnackbar(findViewById(R.id.container), R.string.no_nearby);
}

bundle.clear();

bundle.putString("PlaceList", gsonPlaceList);
bundle.putString("CurLatLng", gsonCurLatLng);
//bundle.putString("CurLatLng", gsonCurLatLng);
bundle.putString("BoundaryCoord", gsonBoundaryCoordinates);

// First time to init fragments
if (nearbyMapFragment == null) {
Timber.d("Init map fragment for the first time");
lockNearbyView(true);
setMapFragment();
setListFragment();
hideProgressBar();
lockNearbyView(false);
} else {
// There are fragments, just update the map and list
Timber.d("Map fragment already exists, just update the map and list");
updateMapFragment(false);
updateListFragment();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,11 @@ public void onViewCreated(View view, Bundle savedInstanceState) {
}

public void updateNearbyListSignificantly() {
adapterFactory.updateAdapterData(getPlaceListFromBundle(bundleForUpdates),
(RVRendererAdapter<Place>) recyclerView.getAdapter());
try {
adapterFactory.updateAdapterData(getPlaceListFromBundle(bundleForUpdates), (RVRendererAdapter<Place>) recyclerView.getAdapter());
} catch (NullPointerException e) {
Timber.e("Null pointer exception from calling recyclerView.getAdapter()");
}
}

private List<Place> getPlaceListFromBundle(Bundle bundle) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ public NearbyMapFragment() {
@Override
public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Timber.d("Nearby map fragment created");

controller = new ContributionController(this);
directUpload = new DirectUpload(this, controller);
Expand All @@ -151,17 +152,21 @@ public void onCreate(Bundle savedInstanceState) {
getActivity());
boundaryCoordinates = gson.fromJson(gsonBoundaryCoordinates, gsonBoundaryCoordinatesType);
}
Mapbox.getInstance(getActivity(),
getString(R.string.mapbox_commons_app_token));
MapboxTelemetry.getInstance().setTelemetryEnabled(false);
if (curLatLng != null) {
Mapbox.getInstance(getActivity(),
getString(R.string.mapbox_commons_app_token));
MapboxTelemetry.getInstance().setTelemetryEnabled(false);
}
setRetainInstance(true);
}

@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container,
Bundle savedInstanceState) {

Timber.d("onCreateView called");
if (curLatLng != null) {
Timber.d("curLatLng found, setting up map view...");
setupMapView(savedInstanceState);
}

Expand Down