Fix security exception crash while accessing network location provider #1498
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1498 +/- ##
=========================================
- Coverage 3.21% 3.2% -0.02%
=========================================
Files 132 132
Lines 7091 7116 +25
Branches 682 684 +2
=========================================
Hits 228 228
- Misses 6848 6873 +25
Partials 15 15
Continue to review full report at Codecov.
|
|
@maskaravivek I dont get security exception with steps now. But still I do get crash with meaningless logs:/
|
|
@neslihanturan Is this happening consistently on a real test phone? Or did you test on an emulator? |
|
Testing on a Samsung physical device with Android 7.0, API 24. Consistently crashes prior to changes and consistently crashes after changes. Have not been able to confirm stacktraces are the same since Android Studio keeps cleaning them up (let me know of a way to save them if further testing is needed). Just to clarify repro: Open app, go to nearby, log out, log in, go to nearby -> crash |
|
@albendz Logs would be quite helpful to debug further. Can you please try using |
|
@albendz Are you sure that this PR doesn't fix the issue for you. I tried reproducing it and was able to successfully reproduce it without the fix. After this fix, the error was no longer occurring for me. |
|
I'll retest and hopefully get back to you soon. |
|
Sorry for the slow response @maskaravivek - I will try to help test as well, today or tomorrow. |
|
Yes, I'm sure. Though, I still can't confirm the error I'm seeing is the same as what you fixed. I don't see an exception, only "05-13 20:49:25.156 18785-18890/fr.free.nrw.commons.debug A/libc: /usr/local/google/buildbot/src/android/ndk-r15-release/external/libcxx/../../external/libcxxabi/src/abort_message.cpp:74: void abort_message(const char *, ...): assertion "terminating with uncaught exception of type mapbox::sqlite::Exception: attempt to write a readonly database" failed |
|
Thanks for helping in testing @albendz. The error that you are facing seems to be different than what I have fixed in the PR. The important logline seems to be: And the possible reasons could be:
In any case, IMO this PR can be merged as it fixes an issue where location permissions were not being checked before registering for location updates. |
|
I am experiencing the same issue when following the steps at #1480 - logs below. However, I am OK with merging this PR since it fixes a different issue, but not closing #1480 . @neslihanturan , what do you think? and |
|
Based on what Vivek sent, the SIGABRT 6 issue is related to something else. If you're seeing that, it's not the issue. Since that seems to be caused by overloading the UI thread, we might be doing the repro too quickly... |
|
I waited for over 10 seconds after logging in before I loaded Nearby, to be sure that speed wasn't the issue. :) However, I personally think it would be best to focus on fixing the jacoco.exec error instead of SIGABRT, as the latter is much more general and can potentially be caused by library errors. After we fix jacoco, we can see if the crash still occurs. |
|
@misaochan I agree with you. My tests gave same results so we should leave #1480 open but also merge this PR |
#1498) * Fix security exception crash while accessing network location provider * Added java docs
…rby (#1495) * Localisation updates from https://translatewiki.net. * Integrate API for displaying featured images (#1456) * Integrate API for displaying featured images * Add pagination and refactor code so that it can be reused for category images * Add license info to the images * Fix author view * Remove unused values * Fix minor issues with featured images * Fix null license url issue * Remove some log lines * Fix back navigation issue * fix tests * fix test inits * Gracefully handling various error situations * Added java docs * Update pull_request_template.md (#1476) * Update pull_request_template.md * Remove Javadocs mention * Added required/optional notes * resolves #1464 : MediaDataExtractor is making inefficient (redundant) server calls (#1496) * Open map of place where picture was taken (#1360) * Intent to map added * Merge conflicts resolved * Added the functionality to hide FAB incase of null coordinate * Merge Conflict resolved * Improve pr quality * Improve Quality * Added nested FAB animations * Nested FAB implemented * Improve Quality * Added up arrow * Javadocs Added * Add nearby tutorial (#1467) * Add dependency for MaterialShowcase * Add actionview class to get a reference to material showcase * Create a NearbyMaterialShowcaseSequence class * Apply sequence steps * Add first three steps of nearby showcase * Add sequence id constants to make sure they will be displayed only once * Add last step of sequence to explain plus fab * Create an object to prevent customize all sequences every time * Fix typo * Code cleanup * Add strings to strings.xml * Code cleanup * Revert irrelevant change * Revert irrelevant change * Remove showcaseview for recenter button * Use single showcaseView instead of sequence * Add single showcase view insted of sequence to be able to edit text style * Make sure it will be displayed only once * Cleanup * Update strings * Change dismiss text style * CONTRIBUTING: fix formatting of the gist of the guidelines (#1453) * CONTRIBUTING: fix formatting of the gist of the guidelines First level headings for a gist seems to be overkill. So, replace first level headings with an ordered-list which sounds more meaningful. * CONTRIBUTING: specify clearly that 'blame' is a feature of "Git" The contributing file specifies about the ability to know who wrote something without the need of @author javadoc tags but incorrectly attributes the feature to GitHub. Correctly attribute the feature to where it belongs, Git, and specify the name of the feature to help users easily take advantage of it. * Feature/switch to butterknife (#1494) * Implemented butterknife in MediaDetailFragment [issue #1491] * Implemented butterknife in MediaDetailPagerFragment [[issue #1491]] * post merge upstream master wip [[issue #1491]] * Localisation updates from https://translatewiki.net. * Bug fix #1504 (#1506) * Bug fix #1504 * Filtered messages with ConnectException [issue #1504] * A generalised message for exceptions in Nearby Activity [issue #1504] * Localisation updates from https://translatewiki.net. * Fix security exception crash while accessing network location provider (#1498) * Fix security exception crash while accessing network location provider * Added java docs * Localisation updates from https://translatewiki.net. * Log P18 edits to wikidata corresponding wikidata entity on uploading a nearby image * Added java docs * Fix test build * Refresh nearby * Refresh nearby list on successful edit * Java docs * Make authenticated wikidata edits * Updated toast message to show entity name that was edited
Description
Fixes #1480
The app was not asking for permissions before subscribing to location updates from the network provider. This was leading to a security exception.
Tests performed
Unable to reproduce the crash using the steps mentioned in the issue.
@neslihanturan @misaochan Can you guys help in testing if its reproducible on your end.