From 6347527cc036d646bb00560b6ba9ba3e5abc14b2 Mon Sep 17 00:00:00 2001 From: Raymond Lai Date: Tue, 28 Nov 2023 22:59:23 +0800 Subject: [PATCH 1/5] Fix NetworkOnMainThreadException on HybridFile.getUsableSpace() for SMB files Fixes #3982 --- .../filemanager/filesystem/HybridFile.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java b/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java index d67d7873fc..f245648642 100644 --- a/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java +++ b/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java @@ -45,6 +45,7 @@ import java.util.EnumSet; import java.util.List; import java.util.Locale; +import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -103,6 +104,7 @@ import androidx.documentfile.provider.DocumentFile; import androidx.preference.PreferenceManager; +import io.reactivex.Completable; import io.reactivex.Single; import io.reactivex.SingleObserver; import io.reactivex.android.schedulers.AndroidSchedulers; @@ -779,13 +781,15 @@ public long getUsableSpace() { long size = 0L; switch (mode) { case SMB: - try { - SmbFile smbFile = getSmbFile(); - size = smbFile != null ? smbFile.getDiskFreeSpace() : 0L; - } catch (SmbException e) { - size = 0L; - LOG.warn("failed to get usage space for smb file", e); - } + size = Single.fromCallable((Callable) () -> { + try { + SmbFile smbFile = getSmbFile(); + return smbFile != null ? smbFile.getDiskFreeSpace() : 0L; + } catch (SmbException e) { + LOG.warn("failed to get usage space for smb file", e); + return 0L; + } + }).subscribeOn(Schedulers.io()).blockingGet(); break; case FILE: case ROOT: From 72e5d4bfd2897938e36102f2c43d299937683e21 Mon Sep 17 00:00:00 2001 From: Raymond Lai Date: Tue, 28 Nov 2023 23:08:30 +0800 Subject: [PATCH 2/5] Add override for HybridFileParcelable.isDirectory(Context) To prevent unnecessary calls to superclass method when information is already available at HybridFileParcelable. This also fixes a problem that stops copying folders over SSH. --- .../amaze/filemanager/filesystem/HybridFileParcelable.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/src/main/java/com/amaze/filemanager/filesystem/HybridFileParcelable.java b/app/src/main/java/com/amaze/filemanager/filesystem/HybridFileParcelable.java index 20edfc696e..1cc78d20c2 100644 --- a/app/src/main/java/com/amaze/filemanager/filesystem/HybridFileParcelable.java +++ b/app/src/main/java/com/amaze/filemanager/filesystem/HybridFileParcelable.java @@ -147,6 +147,11 @@ public boolean isDirectory() { return isDirectory; } + @Override + public boolean isDirectory(Context context) { + return isDirectory; + } + public boolean isHidden() { return name.startsWith("."); } From 191e1e9f28c3b157d314f71a854bd3f43341c026 Mon Sep 17 00:00:00 2001 From: Raymond Lai Date: Tue, 28 Nov 2023 23:46:57 +0800 Subject: [PATCH 3/5] Fix code formatting --- .../filemanager/filesystem/HybridFile.java | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java b/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java index f245648642..046c9ed450 100644 --- a/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java +++ b/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java @@ -104,7 +104,6 @@ import androidx.documentfile.provider.DocumentFile; import androidx.preference.PreferenceManager; -import io.reactivex.Completable; import io.reactivex.Single; import io.reactivex.SingleObserver; import io.reactivex.android.schedulers.AndroidSchedulers; @@ -781,15 +780,20 @@ public long getUsableSpace() { long size = 0L; switch (mode) { case SMB: - size = Single.fromCallable((Callable) () -> { - try { - SmbFile smbFile = getSmbFile(); - return smbFile != null ? smbFile.getDiskFreeSpace() : 0L; - } catch (SmbException e) { - LOG.warn("failed to get usage space for smb file", e); - return 0L; - } - }).subscribeOn(Schedulers.io()).blockingGet(); + size = + Single.fromCallable( + (Callable) + () -> { + try { + SmbFile smbFile = getSmbFile(); + return smbFile != null ? smbFile.getDiskFreeSpace() : 0L; + } catch (SmbException e) { + LOG.warn("failed to get usage space for smb file", e); + return 0L; + } + }) + .subscribeOn(Schedulers.io()) + .blockingGet(); break; case FILE: case ROOT: From 311fd694785bfb107278fe2b93dcc667484c6477 Mon Sep 17 00:00:00 2001 From: Raymond Lai Date: Thu, 30 Nov 2023 23:10:38 +0800 Subject: [PATCH 4/5] Changes per PR feedback Restrict only SMB and SSH HybridFileParcelables returns the isDirectory attribute; others will use the superclass (HybridFile) implementation. Meanwhile, also fixes a potential flaw at HybridFile.isDirectory() for SMB files which is not wrapped in RxJava's Single in IO scheduler. --- .../com/amaze/filemanager/filesystem/HybridFile.java | 10 +--------- .../filemanager/filesystem/HybridFileParcelable.java | 3 ++- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java b/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java index 046c9ed450..ef4f157c2b 100644 --- a/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java +++ b/app/src/main/java/com/amaze/filemanager/filesystem/HybridFile.java @@ -592,16 +592,8 @@ public boolean isDirectory() { switch (mode) { case SFTP: case FTP: - return isDirectory(AppConfig.getInstance()); case SMB: - SmbFile smbFile = getSmbFile(); - try { - isDirectory = smbFile != null && smbFile.isDirectory(); - } catch (SmbException e) { - LOG.warn("failed to get isDirectory for smb file", e); - isDirectory = false; - } - break; + return isDirectory(AppConfig.getInstance()); case ROOT: isDirectory = NativeOperations.isDirectory(path); break; diff --git a/app/src/main/java/com/amaze/filemanager/filesystem/HybridFileParcelable.java b/app/src/main/java/com/amaze/filemanager/filesystem/HybridFileParcelable.java index 1cc78d20c2..c8ee8aeb4d 100644 --- a/app/src/main/java/com/amaze/filemanager/filesystem/HybridFileParcelable.java +++ b/app/src/main/java/com/amaze/filemanager/filesystem/HybridFileParcelable.java @@ -149,7 +149,8 @@ public boolean isDirectory() { @Override public boolean isDirectory(Context context) { - return isDirectory; + if (isSmb() || isSftp()) return isDirectory; + else return super.isDirectory(context); } public boolean isHidden() { From 11534165b07116ed9d783597037d4d2136468f28 Mon Sep 17 00:00:00 2001 From: Raymond Lai Date: Sat, 2 Dec 2023 14:33:50 +0800 Subject: [PATCH 5/5] Fix no feedback to user on problem listing files Toast the user if there is problem listing files. Hence LoadFilesListTask will have Throwable as progress update, for different list methods to feed exceptions to the UI. --- .../asynctasks/LoadFilesListTask.java | 42 ++++++++++++++++++- .../ui/dialogs/SmbConnectDialog.java | 3 +- app/src/main/res/values/strings.xml | 3 ++ build.gradle | 2 +- 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/amaze/filemanager/asynchronous/asynctasks/LoadFilesListTask.java b/app/src/main/java/com/amaze/filemanager/asynchronous/asynctasks/LoadFilesListTask.java index fdf51c6b98..00803c49d9 100644 --- a/app/src/main/java/com/amaze/filemanager/asynchronous/asynctasks/LoadFilesListTask.java +++ b/app/src/main/java/com/amaze/filemanager/asynchronous/asynctasks/LoadFilesListTask.java @@ -74,9 +74,11 @@ import android.os.Bundle; import android.provider.MediaStore; import android.text.format.Formatter; +import android.widget.Toast; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.appcompat.app.AlertDialog; import androidx.core.util.Pair; import jcifs.smb.SmbAuthException; @@ -85,7 +87,7 @@ import kotlin.collections.CollectionsKt; public class LoadFilesListTask - extends AsyncTask>> { + extends AsyncTask>> { private static final Logger LOG = LoggerFactory.getLogger(LoadFilesListTask.class); @@ -203,6 +205,41 @@ protected void onCancelled() { listener.onAsyncTaskFinished(null); } + @Override + protected void onProgressUpdate(Throwable... values) { + for (Throwable exception : values) { + if (exception instanceof SmbException) { + if ("/".equals(Uri.parse(path).getPath())) { + new AlertDialog.Builder(context.get()) + .setTitle(R.string.error_listfile_smb_title) + .setMessage( + context + .get() + .getString( + R.string.error_listfile_smb_noipcshare, + HybridFile.parseAndFormatUriForDisplay(path))) + .setPositiveButton( + android.R.string.ok, + (dialog, which) -> { + dialog.dismiss(); + }) + .show(); + } else { + Toast.makeText( + context.get(), + context + .get() + .getString( + R.string.error_listfile_smb, + HybridFile.parseAndFormatUriForDisplay(path), + exception.getMessage()), + Toast.LENGTH_LONG) + .show(); + } + } + } + } + @Override protected void onPostExecute(@Nullable Pair> list) { listener.onAsyncTaskFinished(list); @@ -615,7 +652,8 @@ private List listSmb( if (!e.getMessage().toLowerCase().contains("denied")) { mainFragment.reauthenticateSmb(); } - LOG.warn("failed to load smb list, authentication issue", e); + LOG.warn("failed to load smb list, authentication issue: ", e); + publishProgress(e); return null; } catch (SmbException | NullPointerException e) { LOG.warn("Failed to load smb files for path: " + path, e); diff --git a/app/src/main/java/com/amaze/filemanager/ui/dialogs/SmbConnectDialog.java b/app/src/main/java/com/amaze/filemanager/ui/dialogs/SmbConnectDialog.java index ab73e4c7b4..0942db2bca 100644 --- a/app/src/main/java/com/amaze/filemanager/ui/dialogs/SmbConnectDialog.java +++ b/app/src/main/java/com/amaze/filemanager/ui/dialogs/SmbConnectDialog.java @@ -346,7 +346,8 @@ public void afterTextChanged(@NonNull Editable s) { } SmbFile smbFile; String domaind = domain.getText().toString(); - if (chkSmbAnonymous.isChecked()) + if (chkSmbAnonymous.isChecked() + || (TextUtils.isEmpty(user.getText()) && TextUtils.isEmpty(pass.getText()))) smbFile = createSMBPath(new String[] {ipa, "", "", domaind, sShare}, true, false); else { String useraw = user.getText().toString(); diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 565e272446..329083bc0a 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -825,5 +825,8 @@ You only need to do this once, until the next time you select a new location for App Language Select Language System Default + Unable to load SMB file list + Error listing files at %s: %s + Error listing files at %s - probably the SMB server does not allow anonymous access to the IPC$ share. You may try specifying the share name directly in connection settings. diff --git a/build.gradle b/build.gradle index ab84a0d87c..97650bcd52 100644 --- a/build.gradle +++ b/build.gradle @@ -6,7 +6,7 @@ buildscript { robolectricVersion = '4.9' glideVersion = '4.11.0' sshjVersion = '0.35.0' - jcifsVersion = '2.1.6' + jcifsVersion = '2.1.9' fabSpeedDialVersion = '3.1.1' roomVersion = '2.5.0' bouncyCastleVersion = '1.70'