Skip to content

Commit

Permalink
Verifying the file descriptor permissions before passing it to `libki…
Browse files Browse the repository at this point in the history
…wix`.

* We have implemented a method to ensure that `libkiwix` can open the `file descriptor` successfully. This precautionary step helps us prevent runtime crashes triggered by `libkiwix` in case there is an issue accessing the `file descriptor` via its path.
  • Loading branch information
MohitMaliFtechiz committed Jan 8, 2024
1 parent 6b25d47 commit 658a2a5
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -239,21 +239,27 @@ class KiwixReaderFragment : CoreReaderFragment() {
} else {
zimReaderContainer?.zimFileReader?.let(::setUpBookmarks)
}
} else if (zimFile != null &&
getAssetFileDescriptorFromUri(requireActivity(), zimFile.toUri()) != null
) {
getAssetFileDescriptorFromUri(
requireActivity(),
zimFile.toUri()
)?.let { assetFileDescriptor ->
openZimFile(null, assetFileDescriptor = assetFileDescriptor, filePath = "$zimFile")
}
} else {
getCurrentWebView()?.snack(R.string.zim_not_opened)
zimFile?.let {
getAssetFileDescriptorFromUri(
requireActivity(),
zimFile.toUri()

Check warning on line 246 in app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt#L244-L246

Added lines #L244 - L246 were not covered by tests
)?.let { assetFileDescriptor ->
openZimFile(null, assetFileDescriptor = assetFileDescriptor, filePath = "$zimFile")
Log.d(
TAG_KIWIX,
"Kiwix normal start, Opened last used zimFile: -> $zimFile"

Check warning on line 251 in app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt#L248-L251

Added lines #L248 - L251 were not covered by tests
)
} ?: run(KiwixReaderFragment::showUnableToOpenFileToast)
} ?: run(KiwixReaderFragment::showUnableToOpenFileToast)

Check warning on line 254 in app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt#L253-L254

Added lines #L253 - L254 were not covered by tests
}
restoreTabs(zimArticles, zimPositions, currentTab)
}

private fun showUnableToOpenFileToast() {
getCurrentWebView()?.snack(R.string.zim_not_opened)
}

Check warning on line 261 in app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt#L261

Added line #L261 was not covered by tests

@Suppress("UnsafeCallOnNullableType")
override fun createWebView(attrs: AttributeSet?): ToolbarScrollingKiwixWebView {
return ToolbarScrollingKiwixWebView(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
import java.io.BufferedReader
import java.io.File
import java.io.FileInputStream
import java.io.FileNotFoundException
import java.io.IOException

Expand Down Expand Up @@ -410,13 +411,22 @@ object FileUtils {
fun getDemoFilePathForCustomApp(context: Context) =
"${ContextCompat.getExternalFilesDirs(context, null)[0]}/demo.zim"

@SuppressLint("Recycle")
@JvmStatic
fun getAssetFileDescriptorFromUri(
context: Context,
uri: Uri
): AssetFileDescriptor? {
return try {
context.contentResolver.openAssetFileDescriptor(uri, "r")
val assetFileDescriptor = context.contentResolver.openAssetFileDescriptor(uri, "r")

Check warning on line 421 in core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt#L421

Added line #L421 was not covered by tests
// Verify whether libkiwix can successfully open this file descriptor or not.
return if (

Check warning on line 423 in core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt#L423

Added line #L423 was not covered by tests
isFileDescriptorCanOpenWithLibkiwix(assetFileDescriptor?.parcelFileDescriptor?.fd)
) {
assetFileDescriptor

Check warning on line 426 in core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt#L426

Added line #L426 was not covered by tests
} else {
null

Check warning on line 428 in core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt#L428

Added line #L428 was not covered by tests
}
} catch (ignore: FileNotFoundException) {
null
} catch (ignore: Exception) {

Check warning on line 432 in core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt#L432

Added line #L432 was not covered by tests
Expand All @@ -427,4 +437,21 @@ object FileUtils {
null

Check warning on line 437 in core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt#L437

Added line #L437 was not covered by tests
}
}

@JvmStatic
fun isFileDescriptorCanOpenWithLibkiwix(fdNumber: Int?): Boolean {
return try {

Check warning on line 443 in core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt#L443

Added line #L443 was not covered by tests
// Attempt to create a FileInputStream object using the specified path.
// Since libkiwix utilizes this path to create the archive object internally,
// it is crucial to verify if we can successfully read the file descriptor (fd)
// via the given file path before passing it to libkiwix.
// This precaution helps prevent runtime crashes.
// For more details, refer to https://github.com/kiwix/kiwix-android/pull/3636.
FileInputStream("dev/fd/$fdNumber")
true
} catch (ignore: Exception) {
ignore.printStackTrace()
false

Check warning on line 454 in core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/utils/files/FileUtils.kt#L450-L454

Added lines #L450 - L454 were not covered by tests
}
}
}

0 comments on commit 658a2a5

Please sign in to comment.