From aa47f1119ff8933f9522ae9f299100731df1c11f Mon Sep 17 00:00:00 2001 From: abraunegg Date: Thu, 12 Nov 2020 21:35:06 +1100 Subject: [PATCH 1/8] Fix download failure due to incorrect filesystem permissions * Add try block for opening new file for writing when downloading a new file to catch any read only file systems * Move set file attributes to master function, incase there is a download failure, exit scope cannot set attributes on a file that is non existent --- src/onedrive.d | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/onedrive.d b/src/onedrive.d index eb2cfff2..a66cdf5b 100644 --- a/src/onedrive.d +++ b/src/onedrive.d @@ -501,7 +501,13 @@ final class OneDriveApi // Configure the applicable permissions for the folder newPath.setAttributes(cfg.returnRequiredDirectoryPermisions()); const(char)[] url = driveByIdUrl ~ driveId ~ "/items/" ~ id ~ "/content?AVOverride=1"; + // Download file download(url, saveToPath, fileSize); + // Does path exist? + if (exists(saveToPath)) { + // File was downloaded sucessfully - configure the applicable permissions for the file + saveToPath.setAttributes(cfg.returnRequiredFilePermisions()); + } } // https://docs.microsoft.com/en-us/onedrive/developer/rest-api/api/driveitem_put_content @@ -797,8 +803,14 @@ final class OneDriveApi { // Threshold for displaying download bar long thresholdFileSize = 4 * 2^^20; // 4 MiB - // open file as write in binary mode - auto file = File(filename, "wb"); + + try { + // open file as write in binary mode + auto file = File(filename, "wb"); + } catch (FileException e) { + // display the error message + displayFileSystemErrorMessage(e.msg); + } // function scopes scope(exit) { @@ -818,8 +830,6 @@ final class OneDriveApi // close open file file.close(); } - // Configure the applicable permissions for the file - filename.setAttributes(cfg.returnRequiredFilePermisions()); } http.method = HTTP.Method.get; From 68cc51ab4d24c74aa9ed49b7c4ecf70d41c2483f Mon Sep 17 00:00:00 2001 From: abraunegg Date: Thu, 12 Nov 2020 21:51:07 +1100 Subject: [PATCH 2/8] Update onedrive.d * Remove try block - incompatible with auto designation needed to open file ... --- src/onedrive.d | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/onedrive.d b/src/onedrive.d index a66cdf5b..0d8d5157 100644 --- a/src/onedrive.d +++ b/src/onedrive.d @@ -803,14 +803,7 @@ final class OneDriveApi { // Threshold for displaying download bar long thresholdFileSize = 4 * 2^^20; // 4 MiB - - try { - // open file as write in binary mode - auto file = File(filename, "wb"); - } catch (FileException e) { - // display the error message - displayFileSystemErrorMessage(e.msg); - } + auto file = File(filename, "wb"); // function scopes scope(exit) { From cc2b8085f5e314f0de7099ee50a1c30214bf755f Mon Sep 17 00:00:00 2001 From: abraunegg Date: Fri, 13 Nov 2020 05:42:24 +1100 Subject: [PATCH 3/8] Update error catching and handling * Update error catching and handling --- src/onedrive.d | 12 +++++++++++- src/sync.d | 7 ++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/onedrive.d b/src/onedrive.d index 0d8d5157..467cfbe8 100644 --- a/src/onedrive.d +++ b/src/onedrive.d @@ -493,8 +493,17 @@ final class OneDriveApi { checkAccessTokenExpired(); scope(failure) { - if (exists(saveToPath)) remove(saveToPath); + if (exists(saveToPath)) { + // try and remove the file, catch error + try { + remove(saveToPath); + } catch (FileException e) { + // display the error message + displayFileSystemErrorMessage(e.msg); + } + } } + // Create the directory string newPath = dirName(saveToPath); mkdirRecurse(newPath); @@ -803,6 +812,7 @@ final class OneDriveApi { // Threshold for displaying download bar long thresholdFileSize = 4 * 2^^20; // 4 MiB + // open file as write in binary mode auto file = File(filename, "wb"); // function scopes diff --git a/src/sync.d b/src/sync.d index 446b47a2..012600dc 100644 --- a/src/sync.d +++ b/src/sync.d @@ -2565,7 +2565,6 @@ final class SyncEngine log.vdebug("onedrive.downloadById(item.driveId, item.id, path, fileSize); generated a OneDriveException"); // 408 = Request Time Out // 429 = Too Many Requests - need to delay - if (e.httpStatusCode == 408) { // 408 error handling - request time out // https://github.com/abraunegg/onedrive/issues/694 @@ -2637,6 +2636,12 @@ final class SyncEngine } } } + } catch (FileException e) { + // There was a file system error + // display the error message + displayFileSystemErrorMessage(e.msg); + downloadFailed = true; + return; } catch (std.exception.ErrnoException e) { // There was a file system error // display the error message From ac02b768d3cb771f7698dfc67bb1bd9022843ded Mon Sep 17 00:00:00 2001 From: abraunegg Date: Fri, 13 Nov 2020 07:34:52 +1100 Subject: [PATCH 4/8] catch folder creation errors due to file system permissions error * catch folder creation errors due to file system permissions error --- src/onedrive.d | 8 +++++++- src/sync.d | 13 +++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/onedrive.d b/src/onedrive.d index 467cfbe8..baf31487 100644 --- a/src/onedrive.d +++ b/src/onedrive.d @@ -506,7 +506,13 @@ final class OneDriveApi // Create the directory string newPath = dirName(saveToPath); - mkdirRecurse(newPath); + try { + mkdirRecurse(newPath); + } catch (FileException e) { + // display the error message + displayFileSystemErrorMessage(e.msg); + } + // Configure the applicable permissions for the folder newPath.setAttributes(cfg.returnRequiredDirectoryPermisions()); const(char)[] url = driveByIdUrl ~ driveId ~ "/items/" ~ id ~ "/content?AVOverride=1"; diff --git a/src/sync.d b/src/sync.d index 012600dc..30768ef0 100644 --- a/src/sync.d +++ b/src/sync.d @@ -2423,10 +2423,15 @@ final class SyncEngine } if (!dryRun) { - // Create the new directory - mkdirRecurse(path); - // Configure the applicable permissions for the folder - path.setAttributes(cfg.returnRequiredDirectoryPermisions()); + try { + // Create the new directory + mkdirRecurse(path); + // Configure the applicable permissions for the folder + path.setAttributes(cfg.returnRequiredDirectoryPermisions()); + } catch (FileException e) { + // display the error message + displayFileSystemErrorMessage(e.msg); + } } else { // we dont create the directory, but we need to track that we 'faked it' idsFaked ~= [item.driveId, item.id]; From dd989b73d1003fe029419be62002aa3332246158 Mon Sep 17 00:00:00 2001 From: abraunegg Date: Fri, 13 Nov 2020 07:48:14 +1100 Subject: [PATCH 5/8] Update sync.d * Flag that this download failed --- src/sync.d | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sync.d b/src/sync.d index 30768ef0..525a209d 100644 --- a/src/sync.d +++ b/src/sync.d @@ -2431,6 +2431,9 @@ final class SyncEngine } catch (FileException e) { // display the error message displayFileSystemErrorMessage(e.msg); + // flag that this download failed, otherwise the 'item' is added to the database - then, as not present on the local disk, would get deleted from OneDrive + downloadFailed = true; + return; } } else { // we dont create the directory, but we need to track that we 'faked it' From b57bb3fde5d2c3259cda2c8379174e9256d32e15 Mon Sep 17 00:00:00 2001 From: abraunegg Date: Fri, 13 Nov 2020 08:02:28 +1100 Subject: [PATCH 6/8] Update sync.d * Remove this --- src/sync.d | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/sync.d b/src/sync.d index 525a209d..30768ef0 100644 --- a/src/sync.d +++ b/src/sync.d @@ -2431,9 +2431,6 @@ final class SyncEngine } catch (FileException e) { // display the error message displayFileSystemErrorMessage(e.msg); - // flag that this download failed, otherwise the 'item' is added to the database - then, as not present on the local disk, would get deleted from OneDrive - downloadFailed = true; - return; } } else { // we dont create the directory, but we need to track that we 'faked it' From b59596523e87bc1abeb54aae63e12dfa9c047d45 Mon Sep 17 00:00:00 2001 From: abraunegg Date: Fri, 13 Nov 2020 08:27:09 +1100 Subject: [PATCH 7/8] Update sync.d * Full fix --- src/sync.d | 45 +++++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/src/sync.d b/src/sync.d index 30768ef0..e250271b 100644 --- a/src/sync.d +++ b/src/sync.d @@ -2006,25 +2006,31 @@ final class SyncEngine // - full path + combination of any above two - /path/name*.txt // - full path to file - /path/to/file.txt - // need to compute the full path for this file - path = itemdb.computePath(item.driveId, item.parentId) ~ "/" ~ item.name; - - // The path that needs to be checked needs to include the '/' - // This due to if the user has specified in skip_file an exclusive path: '/path/file' - that is what must be matched - if (!startsWith(path, "/")){ - // Add '/' to the path - path = '/' ~ path; + // is the parent id in the database? + if (itemdb.idInLocalDatabase(item.driveId, item.parentId)){ + // need to compute the full path for this file + path = itemdb.computePath(item.driveId, item.parentId) ~ "/" ~ item.name; + + // The path that needs to be checked needs to include the '/' + // This due to if the user has specified in skip_file an exclusive path: '/path/file' - that is what must be matched + if (!startsWith(path, "/")){ + // Add '/' to the path + path = '/' ~ path; + } + + log.vdebug("skip_file item to check: ", path); + unwanted = selectiveSync.isFileNameExcluded(path); + log.vdebug("Result: ", unwanted); + if (unwanted) log.vlog("Skipping item - excluded by skip_file config: ", item.name); + } else { + // parent id is not in the database + unwanted = true; + log.vlog("Skipping item - parent not present in local database"); } - - log.vdebug("skip_file item to check: ", path); - unwanted = selectiveSync.isFileNameExcluded(path); - log.vdebug("Result: ", unwanted); - if (unwanted) log.vlog("Skipping item - excluded by skip_file config: ", item.name); } } // check the item type - if (!unwanted) { if (isItemFile(driveItem)) { log.vdebug("The item we are syncing is a file"); @@ -2238,6 +2244,14 @@ final class SyncEngine } // What was the item that was saved log.vdebug("item details: ", item); + } else { + // flag was tripped, which was it + if (downloadFailed) { + log.vdebug("Download or creation of local directory failed"); + } + if (malwareDetected) { + log.vdebug("OneDrive reported that file contained malware"); + } } } @@ -2431,6 +2445,9 @@ final class SyncEngine } catch (FileException e) { // display the error message displayFileSystemErrorMessage(e.msg); + // flag that this failed + downloadFailed = true; + return; } } else { // we dont create the directory, but we need to track that we 'faked it' From aa533ef5a4850749d4e65673651725cbf0df5045 Mon Sep 17 00:00:00 2001 From: abraunegg Date: Fri, 13 Nov 2020 09:52:48 +1100 Subject: [PATCH 8/8] Update sync.d * tweak skip message output --- src/sync.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sync.d b/src/sync.d index e250271b..ce4130f8 100644 --- a/src/sync.d +++ b/src/sync.d @@ -2025,7 +2025,7 @@ final class SyncEngine } else { // parent id is not in the database unwanted = true; - log.vlog("Skipping item - parent not present in local database"); + log.vlog("Skipping file - parent path not present in local database"); } } }