Merge pull request #1815 from bugsnag/PLAT-9797/root-detector-improve…

…ments Trivial improvements to RootDetector
bugsnag · Mar 9, 2023 · 9e2f32a · 9e2f32a
2 parents 7e45499 + cb8ef44
commit 9e2f32a
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@
 
 * Added numeric range annotations to Configuration
   [#1808](https://github.com/bugsnag/bugsnag-android/pull/1808)
+* Small improvements to the root detection overhead
+  [#1815](https://github.com/bugsnag/bugsnag-android/pull/1815)
 
 ## 5.28.4 (2023-02-08)
 

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt
@@ -1,10 +1,9 @@
 package com.bugsnag.android
 
 import androidx.annotation.VisibleForTesting
-import java.io.BufferedReader
 import java.io.File
 import java.io.IOException
-import java.util.concurrent.atomic.AtomicBoolean
+import java.io.Reader
 
 /**
  * Attempts to detect whether the device is rooted. Root detection errs on the side of false
@@ -41,12 +40,13 @@ internal class RootDetector @JvmOverloads constructor(
         )
     }
 
-    private val libraryLoaded = AtomicBoolean(false)
+    @Volatile
+    private var libraryLoaded = false
 
     init {
         try {
             System.loadLibrary("bugsnag-root-detection")
-            libraryLoaded.set(true)
+            libraryLoaded = true
         } catch (ignored: UnsatisfiedLinkError) {
             // library couldn't load. This could be due to root detection countermeasures,
             // or down to genuine OS level bugs with library loading - in either case
@@ -107,7 +107,7 @@ internal class RootDetector @JvmOverloads constructor(
                         line.replace("\\s".toRegex(), "")
                     }.filter { line ->
                         line.startsWith("ro.debuggable=[1]") || line.startsWith("ro.secure=[0]")
-                    }.count() > 0
+                    }.any()
             }
         }
         return false
@@ -120,8 +120,7 @@ internal class RootDetector @JvmOverloads constructor(
         var process: Process? = null
         return try {
             process = processBuilder.start()
-            val output = process.inputStream.bufferedReader().use(BufferedReader::readText)
-            output.isNotBlank()
+            process.inputStream.bufferedReader().use { it.isNotBlank() }
         } catch (ignored: IOException) {
             false
         } finally {
@@ -131,11 +130,21 @@ internal class RootDetector @JvmOverloads constructor(
 
     private external fun performNativeRootChecks(): Boolean
 
+    private fun Reader.isNotBlank(): Boolean {
+        while (true) {
+            val ch = read()
+            when {
+                ch == -1 -> return false
+                !ch.toChar().isWhitespace() -> return true
+            }
+        }
+    }
+
     /**
      * Performs root checks which require native code.
      */
     private fun nativeCheckRoot(): Boolean = when {
-        libraryLoaded.get() -> performNativeRootChecks()
+        libraryLoaded -> performNativeRootChecks()
         else -> false
     }
 }
diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/RootDetectorTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/RootDetectorTest.kt
@@ -52,6 +52,18 @@ class RootDetectorTest {
         verify(process, times(1)).destroy()
     }
 
+    /**
+     * The method returns false if 'which su' returns a blank string
+     */
+    @Test
+    fun checkSuNotFoundBlank() {
+        val emptyStream = ByteArrayInputStream("\n  \n".toByteArray())
+        `when`(process.inputStream).thenReturn(emptyStream)
+        assertFalse(rootDetector.checkSuExists(processBuilder))
+        verify(processBuilder, times(1)).command(listOf("which", "su"))
+        verify(process, times(1)).destroy()
+    }
+
     /**
      * The method returns true if 'which su' returns a non-empty string
      */