-
Notifications
You must be signed in to change notification settings - Fork 406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add another attempt to check write permissions on a folder (fixes #1971) #2057
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -9,6 +9,8 @@ | |||||
import android.os.Build; | ||||||
import android.os.Bundle; | ||||||
import androidx.documentfile.provider.DocumentFile; | ||||||
|
||||||
import android.os.Environment; | ||||||
import android.text.Editable; | ||||||
import android.text.TextUtils; | ||||||
import android.text.TextWatcher; | ||||||
|
@@ -537,6 +539,10 @@ private void checkWriteAndUpdateUI() { | |||||
* Access level readwrite: folder can be configured "sendonly" or "sendreceive". | ||||||
*/ | ||||||
mCanWriteToPath = Util.nativeBinaryCanWriteToPath(FolderActivity.this, mFolder.path); | ||||||
if(!mCanWriteToPath){ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
final File externalStorageDirectory = Environment.getExternalStorageDirectory(); | ||||||
mCanWriteToPath = Util.nativeBinaryCanWriteToPath2(externalStorageDirectory, mFolder.path); | ||||||
} | ||||||
if (mCanWriteToPath) { | ||||||
binding.accessExplanationView.setText(R.string.folder_path_readwrite); | ||||||
binding.folderType.setEnabled(true); | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -7,7 +7,6 @@ | |||||
import android.content.Context; | ||||||
import android.content.pm.ApplicationInfo; | ||||||
import android.content.pm.PackageManager.NameNotFoundException; | ||||||
import android.os.Build; | ||||||
import android.preference.PreferenceManager; | ||||||
import androidx.appcompat.app.AlertDialog; | ||||||
import android.util.Log; | ||||||
|
@@ -172,6 +171,38 @@ public static boolean nativeBinaryCanWriteToPath(Context context, String absolut | |||||
return true; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Returns if the syncthing binary would be able to write a file into | ||||||
* the given folder given the configured access level. | ||||||
* | ||||||
* This uses the Android-native File API | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
*/ | ||||||
public static Boolean nativeBinaryCanWriteToPath2(File externalStorageDir, String absoluteFolderPath) { | ||||||
final String TOUCH_FILE_NAME = ".stwritetest"; | ||||||
|
||||||
// normalize path replacing ~ with external storage directory | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
// this is consistent with what $HOME is set to in SyncthingRunnable | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
final String normalizedPath = absoluteFolderPath.replaceFirst("^~", externalStorageDir.getAbsolutePath()); | ||||||
|
||||||
// Write permission test file | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
File touchFile = new File(normalizedPath, TOUCH_FILE_NAME); | ||||||
final String touchFilePath = touchFile.getAbsolutePath(); | ||||||
try{ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please double-check whether the code follows the same convention as the current one (spacing, new lines, etc.). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tomasz1986 thank you for the review. I addressed the formatting issues you highlighted. |
||||||
Boolean fileCreated = touchFile.createNewFile(); | ||||||
Log.d(TAG, String.format("%s createNewFile result %b", touchFilePath, fileCreated)); | ||||||
Boolean fileDeleted = touchFile.delete(); | ||||||
Log.d(TAG, String.format("%s delete result %b", touchFilePath, fileDeleted)); | ||||||
} catch (IOException e) { | ||||||
Log.e(TAG, String.format("Failed to write test file '%s'", touchFilePath), e); | ||||||
return false; | ||||||
} | ||||||
|
||||||
// Detected we have write permission. | ||||||
Log.i(TAG, String.format("Successfully wrote test file '%s'", touchFile)); | ||||||
|
||||||
return true; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Run command in a shell and return the exit code. | ||||||
*/ | ||||||
|
@@ -241,4 +272,5 @@ public static AlertDialog.Builder getAlertDialogBuilder(Context context) | |||||
{ | ||||||
return new MaterialAlertDialogBuilder(context); | ||||||
} | ||||||
|
||||||
wrobbins marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the new line.