Address remaining scoped storage regressions

- Accelerate common path used to check the storage scope for a given path
- Update the logic for the `get_as_text()` method - previous logic loads the content of a text file one byte at a time
This commit is contained in:
Fredia Huya-Kouadio 2022-07-25 15:47:31 -07:00
parent 68b8dcdf50
commit fc2c202312
7 changed files with 80 additions and 38 deletions

View file

@ -2091,16 +2091,10 @@ PoolVector<uint8_t> _File::get_buffer(int64_t p_length) const {
String _File::get_as_text() const {
ERR_FAIL_COND_V_MSG(!f, String(), "File must be opened before use.");
String text;
uint64_t original_pos = f->get_position();
f->seek(0);
String l = get_line();
while (!eof_reached()) {
text += l + "\n";
l = get_line();
}
text += l;
String text = f->get_as_utf8_string();
f->seek(original_pos);

View file

@ -166,6 +166,51 @@ uint8_t FileAccessFilesystemJAndroid::get_8() const {
return byte;
}
String FileAccessFilesystemJAndroid::get_line() const {
ERR_FAIL_COND_V_MSG(!is_open(), String(), "File must be opened before use.");
const size_t buffer_size_limit = 2048;
const uint64_t file_size = get_len();
const uint64_t start_position = get_position();
String result;
LocalVector<uint8_t> line_buffer;
size_t current_buffer_size = 0;
uint64_t line_buffer_position = 0;
while (true) {
size_t line_buffer_size = MIN(buffer_size_limit, file_size - get_position());
if (line_buffer_size <= 0) {
break;
}
current_buffer_size += line_buffer_size;
line_buffer.resize(current_buffer_size);
uint64_t bytes_read = get_buffer(&line_buffer[line_buffer_position], current_buffer_size - line_buffer_position);
if (bytes_read <= 0) {
break;
}
for (; bytes_read > 0; line_buffer_position++, bytes_read--) {
uint8_t elem = line_buffer[line_buffer_position];
if (elem == '\n' || elem == '\0') {
// Found the end of the line
const_cast<FileAccessFilesystemJAndroid *>(this)->seek(start_position + line_buffer_position + 1);
if (result.parse_utf8((const char *)line_buffer.ptr(), line_buffer_position)) {
return String();
}
return result;
}
}
}
if (result.parse_utf8((const char *)line_buffer.ptr(), line_buffer_position)) {
return String();
}
return result;
}
uint64_t FileAccessFilesystemJAndroid::get_buffer(uint8_t *p_dst, uint64_t p_length) const {
if (_file_read) {
ERR_FAIL_COND_V_MSG(!is_open(), 0, "File must be opened before use.");

View file

@ -73,6 +73,7 @@ public:
virtual bool eof_reached() const override; ///< reading passed EOF
virtual uint8_t get_8() const override; ///< get a byte
virtual String get_line() const override; ///< get a line
virtual uint64_t get_buffer(uint8_t *p_dst, uint64_t p_length) const override;
virtual Error get_error() const override; ///< get last error

View file

@ -54,11 +54,19 @@ internal enum class StorageScope {
*/
UNKNOWN;
companion object {
class Identifier(context: Context) {
private val internalAppDir: String? = context.filesDir.canonicalPath
private val internalCacheDir: String? = context.cacheDir.canonicalPath
private val externalAppDir: String? = context.getExternalFilesDir(null)?.canonicalPath
private val sharedDir : String? = Environment.getExternalStorageDirectory().canonicalPath
private val downloadsSharedDir: String? = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS).canonicalPath
private val documentsSharedDir: String? = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOCUMENTS).canonicalPath
/**
* Determines which [StorageScope] the given path falls under.
*/
fun getStorageScope(context: Context, path: String?): StorageScope {
fun identifyStorageScope(path: String?): StorageScope {
if (path == null) {
return UNKNOWN
}
@ -70,23 +78,19 @@ internal enum class StorageScope {
val canonicalPathFile = pathFile.canonicalPath
val internalAppDir = context.filesDir.canonicalPath ?: return UNKNOWN
if (canonicalPathFile.startsWith(internalAppDir)) {
if (internalAppDir != null && canonicalPathFile.startsWith(internalAppDir)) {
return APP
}
val internalCacheDir = context.cacheDir.canonicalPath ?: return UNKNOWN
if (canonicalPathFile.startsWith(internalCacheDir)) {
if (internalCacheDir != null && canonicalPathFile.startsWith(internalCacheDir)) {
return APP
}
val externalAppDir = context.getExternalFilesDir(null)?.canonicalPath ?: return UNKNOWN
if (canonicalPathFile.startsWith(externalAppDir)) {
if (externalAppDir != null && canonicalPathFile.startsWith(externalAppDir)) {
return APP
}
val sharedDir = Environment.getExternalStorageDirectory().canonicalPath ?: return UNKNOWN
if (canonicalPathFile.startsWith(sharedDir)) {
if (sharedDir != null && canonicalPathFile.startsWith(sharedDir)) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) {
// Before R, apps had access to shared storage so long as they have the right
// permissions (and flag on Q).
@ -95,13 +99,8 @@ internal enum class StorageScope {
// Post R, access is limited based on the target destination
// 'Downloads' and 'Documents' are still accessible
val downloadsSharedDir =
Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS).canonicalPath
?: return SHARED
val documentsSharedDir =
Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOCUMENTS).canonicalPath
?: return SHARED
if (canonicalPathFile.startsWith(downloadsSharedDir) || canonicalPathFile.startsWith(documentsSharedDir)) {
if ((downloadsSharedDir != null && canonicalPathFile.startsWith(downloadsSharedDir))
|| (documentsSharedDir != null && canonicalPathFile.startsWith(documentsSharedDir))) {
return APP
}

View file

@ -54,6 +54,7 @@ internal class FilesystemDirectoryAccess(private val context: Context):
private data class DirData(val dirFile: File, val files: Array<File>, var current: Int = 0)
private val storageScopeIdentifier = StorageScope.Identifier(context)
private val storageManager = context.getSystemService(Context.STORAGE_SERVICE) as StorageManager
private var lastDirId = STARTING_DIR_ID
private val dirs = SparseArray<DirData>()
@ -62,7 +63,7 @@ internal class FilesystemDirectoryAccess(private val context: Context):
// Directory access is available for shared storage on Android 11+
// On Android 10, access is also available as long as the `requestLegacyExternalStorage`
// tag is available.
return StorageScope.getStorageScope(context, path) != StorageScope.UNKNOWN
return storageScopeIdentifier.identifyStorageScope(path) != StorageScope.UNKNOWN
}
override fun hasDirId(dirId: Int) = dirs.indexOfKey(dirId) >= 0
@ -102,7 +103,7 @@ internal class FilesystemDirectoryAccess(private val context: Context):
}
}
override fun fileExists(path: String) = FileAccessHandler.fileExists(context, path)
override fun fileExists(path: String) = FileAccessHandler.fileExists(context, storageScopeIdentifier, path)
override fun dirNext(dirId: Int): String {
val dirData = dirs[dirId]
@ -199,7 +200,7 @@ internal class FilesystemDirectoryAccess(private val context: Context):
if (fromFile.isDirectory) {
fromFile.renameTo(File(to))
} else {
FileAccessHandler.renameFile(context, from, to)
FileAccessHandler.renameFile(context, storageScopeIdentifier, from, to)
}
} catch (e: SecurityException) {
false
@ -218,7 +219,7 @@ internal class FilesystemDirectoryAccess(private val context: Context):
if (deleteFile.isDirectory) {
deleteFile.delete()
} else {
FileAccessHandler.removeFile(context, filename)
FileAccessHandler.removeFile(context, storageScopeIdentifier, filename)
}
} else {
true

View file

@ -161,8 +161,9 @@ internal abstract class DataAccess(private val filePath: String) {
fun read(buffer: ByteBuffer): Int {
return try {
val readBytes = fileChannel.read(buffer)
endOfFile = readBytes == -1
|| (fileChannel.position() >= fileChannel.size() && fileChannel.size() > 0)
if (readBytes == -1) {
endOfFile = true
0
} else {
readBytes

View file

@ -49,8 +49,8 @@ class FileAccessHandler(val context: Context) {
private const val INVALID_FILE_ID = 0
private const val STARTING_FILE_ID = 1
fun fileExists(context: Context, path: String?): Boolean {
val storageScope = StorageScope.getStorageScope(context, path)
internal fun fileExists(context: Context, storageScopeIdentifier: StorageScope.Identifier, path: String?): Boolean {
val storageScope = storageScopeIdentifier.identifyStorageScope(path)
if (storageScope == StorageScope.UNKNOWN) {
return false
}
@ -62,8 +62,8 @@ class FileAccessHandler(val context: Context) {
}
}
fun removeFile(context: Context, path: String?): Boolean {
val storageScope = StorageScope.getStorageScope(context, path)
internal fun removeFile(context: Context, storageScopeIdentifier: StorageScope.Identifier, path: String?): Boolean {
val storageScope = storageScopeIdentifier.identifyStorageScope(path)
if (storageScope == StorageScope.UNKNOWN) {
return false
}
@ -75,8 +75,8 @@ class FileAccessHandler(val context: Context) {
}
}
fun renameFile(context: Context, from: String?, to: String?): Boolean {
val storageScope = StorageScope.getStorageScope(context, from)
internal fun renameFile(context: Context, storageScopeIdentifier: StorageScope.Identifier, from: String?, to: String?): Boolean {
val storageScope = storageScopeIdentifier.identifyStorageScope(from)
if (storageScope == StorageScope.UNKNOWN) {
return false
}
@ -89,13 +89,14 @@ class FileAccessHandler(val context: Context) {
}
}
private val storageScopeIdentifier = StorageScope.Identifier(context)
private val files = SparseArray<DataAccess>()
private var lastFileId = STARTING_FILE_ID
private fun hasFileId(fileId: Int) = files.indexOfKey(fileId) >= 0
fun fileOpen(path: String?, modeFlags: Int): Int {
val storageScope = StorageScope.getStorageScope(context, path)
val storageScope = storageScopeIdentifier.identifyStorageScope(path)
if (storageScope == StorageScope.UNKNOWN) {
return INVALID_FILE_ID
}
@ -162,10 +163,10 @@ class FileAccessHandler(val context: Context) {
files[fileId].flush()
}
fun fileExists(path: String?) = Companion.fileExists(context, path)
fun fileExists(path: String?) = Companion.fileExists(context, storageScopeIdentifier, path)
fun fileLastModified(filepath: String?): Long {
val storageScope = StorageScope.getStorageScope(context, filepath)
val storageScope = storageScopeIdentifier.identifyStorageScope(filepath)
if (storageScope == StorageScope.UNKNOWN) {
return 0L
}