From 4a4611ccea82e9088192932f8b26ac6bf315f066 Mon Sep 17 00:00:00 2001 From: abraunegg Date: Sun, 30 Jun 2019 10:12:25 +1000 Subject: [PATCH] Update fix for Issue #555 (#559) * Update fix for #555 as try | catch block for session creation appears to miss error response codes * Update how JSONValue object is determined to be valid * Add error logging when response is not a valid JSON object --- src/sync.d | 145 +++++++++++++++++++++++++++------------------------ src/upload.d | 94 ++++++++++++++++++--------------- 2 files changed, 129 insertions(+), 110 deletions(-) diff --git a/src/sync.d b/src/sync.d index 4d743149..2620d842 100644 --- a/src/sync.d +++ b/src/sync.d @@ -306,8 +306,8 @@ final class SyncEngine exit(-1); } } - - if ((hasId(oneDriveDetails)) && (hasId(oneDriveRootDetails))) { + + if ((oneDriveDetails.type() == JSONType.object) && (oneDriveRootDetails.type() == JSONType.object) && (hasId(oneDriveDetails)) && (hasId(oneDriveRootDetails))) { // JSON elements are valid // Debug OneDrive Account details response log.vdebug("OneDrive Account Details: ", oneDriveDetails); @@ -1120,7 +1120,7 @@ final class SyncEngine } // fileDetails has to be a valid JSON object - if (fileDetails.object()){ + if (fileDetails.type() == JSONType.object){ if (isMalware(fileDetails)){ // OneDrive reports that this file is malware log.error("ERROR: MALWARE DETECTED IN FILE - DOWNLOAD SKIPPED"); @@ -1138,7 +1138,7 @@ final class SyncEngine if (!dryRun) { ulong fileSize = 0; string OneDriveFileHash; - if ( (hasFileSize(fileDetails)) && (hasQuickXorHash(fileDetails)) && (fileDetails.object()) ) { + if ( (hasFileSize(fileDetails)) && (hasQuickXorHash(fileDetails)) && (fileDetails.type() == JSONType.object) ) { // fileDetails is a valid JSON object with the elements we need // Set the file size from the returned data fileSize = fileDetails["size"].integer; @@ -2023,12 +2023,10 @@ final class SyncEngine // https://github.com/OneDrive/onedrive-api-docs/issues/53 try { response = onedrive.simpleUpload(path, parent.driveId, parent.id, baseName(path)); - writeln(" done."); } catch (OneDriveException e) { // error uploading file return; } - } else { // File is not a zero byte file // Are we using OneDrive Personal or OneDrive Business? @@ -2052,13 +2050,11 @@ final class SyncEngine } else throw e; } - writeln(" done."); } else { // File larger than threshold - use a session to upload writeln(""); try { response = session.upload(path, parent.driveId, parent.id, baseName(path)); - writeln(" done."); } catch (OneDriveException e) { // error uploading file log.vlog("Upload failed with OneDriveException: ", e.msg); @@ -2073,76 +2069,87 @@ final class SyncEngine writeln(""); try { response = session.upload(path, parent.driveId, parent.id, baseName(path)); - writeln(" done."); } catch (OneDriveException e) { // error uploading file + log.vlog("Upload failed with OneDriveException: ", e.msg); + return; + } catch (FileException e) { + log.vlog("Upload failed with File Exception: ", e.msg); return; } } } - // Log action to log file - log.fileOnly("Uploading new file ", path, " ... done."); - - // The file was uploaded, or a 4xx / 5xx error was generated - if ("size" in response){ - // The response JSON contains size, high likelihood valid response returned - ulong uploadFileSize = response["size"].integer; - - // In some cases the file that was uploaded was not complete, but 'completed' without errors on OneDrive - // This has been seen with PNG / JPG files mainly, which then contributes to generating a 412 error when we attempt to update the metadata - // Validate here that the file uploaded, at least in size, matches in the response to what the size is on disk - if (thisFileSize != uploadFileSize){ - if(disableUploadValidation){ - // Print a warning message - log.log("WARNING: Uploaded file size does not match local file - skipping upload validation"); + // response from OneDrive has to be a valid JSON object + if (response.type() == JSONType.object){ + // Log action to log file + log.fileOnly("Uploading new file ", path, " ... done."); + writeln(" done."); + // The file was uploaded, or a 4xx / 5xx error was generated + if ("size" in response){ + // The response JSON contains size, high likelihood valid response returned + ulong uploadFileSize = response["size"].integer; + + // In some cases the file that was uploaded was not complete, but 'completed' without errors on OneDrive + // This has been seen with PNG / JPG files mainly, which then contributes to generating a 412 error when we attempt to update the metadata + // Validate here that the file uploaded, at least in size, matches in the response to what the size is on disk + if (thisFileSize != uploadFileSize){ + if(disableUploadValidation){ + // Print a warning message + log.log("WARNING: Uploaded file size does not match local file - skipping upload validation"); + } else { + // OK .. the uploaded file does not match and we did not disable this validation + log.log("Uploaded file size does not match local file - upload failure - retrying"); + // Delete uploaded bad file + onedrive.deleteById(response["parentReference"]["driveId"].str, response["id"].str, response["eTag"].str); + // Re-upload + uploadNewFile(path); + return; + } + } + + // File validation is OK + if ((accountType == "personal") || (thisFileSize == 0)){ + // Update the item's metadata on OneDrive + string id = response["id"].str; + string cTag; + + // Is there a valid cTag in the response? + if ("cTag" in response) { + // use the cTag instead of the eTag because OneDrive may update the metadata of files AFTER they have been uploaded + cTag = response["cTag"].str; + } else { + // Is there an eTag in the response? + if ("eTag" in response) { + // use the eTag from the response as there was no cTag + cTag = response["eTag"].str; + } else { + // no tag available - set to nothing + cTag = ""; + } + } + + if (exists(path)) { + SysTime mtime = timeLastModified(path).toUTC(); + uploadLastModifiedTime(parent.driveId, id, cTag, mtime); + } else { + // will be removed in different event! + log.log("File disappeared after upload: ", path); + } + return; } else { - // OK .. the uploaded file does not match and we did not disable this validation - log.log("Uploaded file size does not match local file - upload failure - retrying"); - // Delete uploaded bad file - onedrive.deleteById(response["parentReference"]["driveId"].str, response["id"].str, response["eTag"].str); - // Re-upload - uploadNewFile(path); + // OneDrive Business Account - always use a session to upload + // The session includes a Request Body element containing lastModifiedDateTime + // which negates the need for a modify event against OneDrive + saveItem(response); return; } - } - - // File validation is OK - if ((accountType == "personal") || (thisFileSize == 0)){ - // Update the item's metadata on OneDrive - string id = response["id"].str; - string cTag; - - // Is there a valid cTag in the response? - if ("cTag" in response) { - // use the cTag instead of the eTag because OneDrive may update the metadata of files AFTER they have been uploaded - cTag = response["cTag"].str; - } else { - // Is there an eTag in the response? - if ("eTag" in response) { - // use the eTag from the response as there was no cTag - cTag = response["eTag"].str; - } else { - // no tag available - set to nothing - cTag = ""; - } - } - - if (exists(path)) { - SysTime mtime = timeLastModified(path).toUTC(); - uploadLastModifiedTime(parent.driveId, id, cTag, mtime); - } else { - // will be removed in different event! - log.log("File disappeared after upload: ", path); - } - return; - } else { - // OneDrive Business Account - always use a session to upload - // The session includes a Request Body element containing lastModifiedDateTime - // which negates the need for a modify event against OneDrive - saveItem(response); - return; } + } else { + // response is not valid JSON, an error was returned from OneDrive + log.fileOnly("Uploading new file ", path, " ... error"); + writeln(" error"); + return; } } else { // we are --dry-run - simulate the file upload @@ -2168,7 +2175,7 @@ final class SyncEngine // Note that NTFS supports POSIX semantics for case sensitivity but this is not the default behavior. // fileDetailsFromOneDrive has to be a valid object - if (fileDetailsFromOneDrive.object()){ + if (fileDetailsFromOneDrive.type() == JSONType.object){ // Check that 'name' is in the JSON response (validates data) and that 'name' == the path we are looking for if (("name" in fileDetailsFromOneDrive) && (fileDetailsFromOneDrive["name"].str == baseName(path))) { // OneDrive 'name' matches local path name @@ -2377,7 +2384,7 @@ final class SyncEngine private void saveItem(JSONValue jsonItem) { // jsonItem has to be a valid object - if (jsonItem.object()){ + if (jsonItem.type() == JSONType.object){ // Check if the response JSON has an 'id', otherwise makeItem() fails with 'Key not found: id' if (hasId(jsonItem)) { // Takes a JSON input and formats to an item which can be used by the database diff --git a/src/upload.d b/src/upload.d index d93a499e..a2a67d64 100644 --- a/src/upload.d +++ b/src/upload.d @@ -39,13 +39,14 @@ struct UploadSession ]) ]; - try { - // Try to create the upload session for this file - session = onedrive.createUploadSession(parentDriveId, parentId, filename, eTag, fileSystemInfo); + // Try to create the upload session for this file + session = onedrive.createUploadSession(parentDriveId, parentId, filename, eTag, fileSystemInfo); + + if ("uploadUrl" in session){ session["localPath"] = localPath; save(); return upload(); - } catch (OneDriveException e) { + } else { // there was an error log.vlog("Create file upload session failed ... skipping file upload"); // return upload() will return a JSONValue response, create an empty JSONValue response to return @@ -94,7 +95,7 @@ struct UploadSession } // do we have a valid response from OneDrive? - if (response.object()){ + if (response.type() == JSONType.object){ // JSON object if (("expirationDateTime" in response) && ("nextExpectedRanges" in response)){ // has the elements we need @@ -143,6 +144,10 @@ struct UploadSession JSONValue upload() { + // Response for upload + JSONValue response; + + // session JSON needs to contain valid elements long offset; long fileSize; @@ -154,45 +159,52 @@ struct UploadSession fileSize = getSize(session["localPath"].str); } - // Upload Progress Bar - size_t iteration = (roundTo!int(double(fileSize)/double(fragmentSize)))+1; - Progress p = new Progress(iteration); - p.title = "Uploading"; - - JSONValue response; - while (true) { - p.next(); - long fragSize = fragmentSize < fileSize - offset ? fragmentSize : fileSize - offset; - // If the resume upload fails, we need to check for a return code here - try { - response = onedrive.uploadFragment( - session["uploadUrl"].str, - session["localPath"].str, - offset, - fragSize, - fileSize - ); - offset += fragmentSize; - if (offset >= fileSize) break; - // update the session details - session["expirationDateTime"] = response["expirationDateTime"]; - session["nextExpectedRanges"] = response["nextExpectedRanges"]; - save(); - } catch (OneDriveException e) { - // there was an error remove session file - if (exists(sessionFilePath)) { - remove(sessionFilePath); + if ("uploadUrl" in session){ + // Upload file via session created + // Upload Progress Bar + size_t iteration = (roundTo!int(double(fileSize)/double(fragmentSize)))+1; + Progress p = new Progress(iteration); + p.title = "Uploading"; + + while (true) { + p.next(); + long fragSize = fragmentSize < fileSize - offset ? fragmentSize : fileSize - offset; + // If the resume upload fails, we need to check for a return code here + try { + response = onedrive.uploadFragment( + session["uploadUrl"].str, + session["localPath"].str, + offset, + fragSize, + fileSize + ); + offset += fragmentSize; + if (offset >= fileSize) break; + // update the session details + session["expirationDateTime"] = response["expirationDateTime"]; + session["nextExpectedRanges"] = response["nextExpectedRanges"]; + save(); + } catch (OneDriveException e) { + // there was an error remove session file + if (exists(sessionFilePath)) { + remove(sessionFilePath); + } + return response; } - return response; } + // upload complete + p.next(); + writeln(); + if (exists(sessionFilePath)) { + remove(sessionFilePath); + } + return response; + } else { + // session elements were not present + log.vlog("Session has no valid upload URL ... skipping this file upload"); + // return an empty JSON response + return response; } - // upload complete - p.next(); - writeln(); - if (exists(sessionFilePath)) { - remove(sessionFilePath); - } - return response; } private void save()