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
This commit is contained in:
abraunegg 2019-06-30 10:12:25 +10:00 committed by GitHub
parent 2f3804a3c0
commit 4a4611ccea
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 129 additions and 110 deletions

View file

@ -306,8 +306,8 @@ final class SyncEngine
exit(-1); exit(-1);
} }
} }
if ((hasId(oneDriveDetails)) && (hasId(oneDriveRootDetails))) { if ((oneDriveDetails.type() == JSONType.object) && (oneDriveRootDetails.type() == JSONType.object) && (hasId(oneDriveDetails)) && (hasId(oneDriveRootDetails))) {
// JSON elements are valid // JSON elements are valid
// Debug OneDrive Account details response // Debug OneDrive Account details response
log.vdebug("OneDrive Account Details: ", oneDriveDetails); log.vdebug("OneDrive Account Details: ", oneDriveDetails);
@ -1120,7 +1120,7 @@ final class SyncEngine
} }
// fileDetails has to be a valid JSON object // fileDetails has to be a valid JSON object
if (fileDetails.object()){ if (fileDetails.type() == JSONType.object){
if (isMalware(fileDetails)){ if (isMalware(fileDetails)){
// OneDrive reports that this file is malware // OneDrive reports that this file is malware
log.error("ERROR: MALWARE DETECTED IN FILE - DOWNLOAD SKIPPED"); log.error("ERROR: MALWARE DETECTED IN FILE - DOWNLOAD SKIPPED");
@ -1138,7 +1138,7 @@ final class SyncEngine
if (!dryRun) { if (!dryRun) {
ulong fileSize = 0; ulong fileSize = 0;
string OneDriveFileHash; 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 // fileDetails is a valid JSON object with the elements we need
// Set the file size from the returned data // Set the file size from the returned data
fileSize = fileDetails["size"].integer; fileSize = fileDetails["size"].integer;
@ -2023,12 +2023,10 @@ final class SyncEngine
// https://github.com/OneDrive/onedrive-api-docs/issues/53 // https://github.com/OneDrive/onedrive-api-docs/issues/53
try { try {
response = onedrive.simpleUpload(path, parent.driveId, parent.id, baseName(path)); response = onedrive.simpleUpload(path, parent.driveId, parent.id, baseName(path));
writeln(" done.");
} catch (OneDriveException e) { } catch (OneDriveException e) {
// error uploading file // error uploading file
return; return;
} }
} else { } else {
// File is not a zero byte file // File is not a zero byte file
// Are we using OneDrive Personal or OneDrive Business? // Are we using OneDrive Personal or OneDrive Business?
@ -2052,13 +2050,11 @@ final class SyncEngine
} }
else throw e; else throw e;
} }
writeln(" done.");
} else { } else {
// File larger than threshold - use a session to upload // File larger than threshold - use a session to upload
writeln(""); writeln("");
try { try {
response = session.upload(path, parent.driveId, parent.id, baseName(path)); response = session.upload(path, parent.driveId, parent.id, baseName(path));
writeln(" done.");
} catch (OneDriveException e) { } catch (OneDriveException e) {
// error uploading file // error uploading file
log.vlog("Upload failed with OneDriveException: ", e.msg); log.vlog("Upload failed with OneDriveException: ", e.msg);
@ -2073,76 +2069,87 @@ final class SyncEngine
writeln(""); writeln("");
try { try {
response = session.upload(path, parent.driveId, parent.id, baseName(path)); response = session.upload(path, parent.driveId, parent.id, baseName(path));
writeln(" done.");
} catch (OneDriveException e) { } catch (OneDriveException e) {
// error uploading file // 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; return;
} }
} }
} }
// Log action to log file // response from OneDrive has to be a valid JSON object
log.fileOnly("Uploading new file ", path, " ... done."); if (response.type() == JSONType.object){
// Log action to log file
// The file was uploaded, or a 4xx / 5xx error was generated log.fileOnly("Uploading new file ", path, " ... done.");
if ("size" in response){ writeln(" done.");
// The response JSON contains size, high likelihood valid response returned // The file was uploaded, or a 4xx / 5xx error was generated
ulong uploadFileSize = response["size"].integer; if ("size" in response){
// The response JSON contains size, high likelihood valid response returned
// In some cases the file that was uploaded was not complete, but 'completed' without errors on OneDrive ulong uploadFileSize = response["size"].integer;
// 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 // In some cases the file that was uploaded was not complete, but 'completed' without errors on OneDrive
if (thisFileSize != uploadFileSize){ // This has been seen with PNG / JPG files mainly, which then contributes to generating a 412 error when we attempt to update the metadata
if(disableUploadValidation){ // Validate here that the file uploaded, at least in size, matches in the response to what the size is on disk
// Print a warning message if (thisFileSize != uploadFileSize){
log.log("WARNING: Uploaded file size does not match local file - skipping upload validation"); 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 { } else {
// OK .. the uploaded file does not match and we did not disable this validation // OneDrive Business Account - always use a session to upload
log.log("Uploaded file size does not match local file - upload failure - retrying"); // The session includes a Request Body element containing lastModifiedDateTime
// Delete uploaded bad file // which negates the need for a modify event against OneDrive
onedrive.deleteById(response["parentReference"]["driveId"].str, response["id"].str, response["eTag"].str); saveItem(response);
// Re-upload
uploadNewFile(path);
return; 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 { } else {
// we are --dry-run - simulate the file upload // 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. // Note that NTFS supports POSIX semantics for case sensitivity but this is not the default behavior.
// fileDetailsFromOneDrive has to be a valid object // 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 // 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))) { if (("name" in fileDetailsFromOneDrive) && (fileDetailsFromOneDrive["name"].str == baseName(path))) {
// OneDrive 'name' matches local path name // OneDrive 'name' matches local path name
@ -2377,7 +2384,7 @@ final class SyncEngine
private void saveItem(JSONValue jsonItem) private void saveItem(JSONValue jsonItem)
{ {
// jsonItem has to be a valid object // 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' // Check if the response JSON has an 'id', otherwise makeItem() fails with 'Key not found: id'
if (hasId(jsonItem)) { if (hasId(jsonItem)) {
// Takes a JSON input and formats to an item which can be used by the database // Takes a JSON input and formats to an item which can be used by the database

View file

@ -39,13 +39,14 @@ struct UploadSession
]) ])
]; ];
try { // Try to create the upload session for this file
// Try to create the upload session for this file session = onedrive.createUploadSession(parentDriveId, parentId, filename, eTag, fileSystemInfo);
session = onedrive.createUploadSession(parentDriveId, parentId, filename, eTag, fileSystemInfo);
if ("uploadUrl" in session){
session["localPath"] = localPath; session["localPath"] = localPath;
save(); save();
return upload(); return upload();
} catch (OneDriveException e) { } else {
// there was an error // there was an error
log.vlog("Create file upload session failed ... skipping file upload"); log.vlog("Create file upload session failed ... skipping file upload");
// return upload() will return a JSONValue response, create an empty JSONValue response to return // 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? // do we have a valid response from OneDrive?
if (response.object()){ if (response.type() == JSONType.object){
// JSON object // JSON object
if (("expirationDateTime" in response) && ("nextExpectedRanges" in response)){ if (("expirationDateTime" in response) && ("nextExpectedRanges" in response)){
// has the elements we need // has the elements we need
@ -143,6 +144,10 @@ struct UploadSession
JSONValue upload() JSONValue upload()
{ {
// Response for upload
JSONValue response;
// session JSON needs to contain valid elements
long offset; long offset;
long fileSize; long fileSize;
@ -154,45 +159,52 @@ struct UploadSession
fileSize = getSize(session["localPath"].str); fileSize = getSize(session["localPath"].str);
} }
// Upload Progress Bar if ("uploadUrl" in session){
size_t iteration = (roundTo!int(double(fileSize)/double(fragmentSize)))+1; // Upload file via session created
Progress p = new Progress(iteration); // Upload Progress Bar
p.title = "Uploading"; size_t iteration = (roundTo!int(double(fileSize)/double(fragmentSize)))+1;
Progress p = new Progress(iteration);
JSONValue response; p.title = "Uploading";
while (true) {
p.next(); while (true) {
long fragSize = fragmentSize < fileSize - offset ? fragmentSize : fileSize - offset; p.next();
// If the resume upload fails, we need to check for a return code here long fragSize = fragmentSize < fileSize - offset ? fragmentSize : fileSize - offset;
try { // If the resume upload fails, we need to check for a return code here
response = onedrive.uploadFragment( try {
session["uploadUrl"].str, response = onedrive.uploadFragment(
session["localPath"].str, session["uploadUrl"].str,
offset, session["localPath"].str,
fragSize, offset,
fileSize fragSize,
); fileSize
offset += fragmentSize; );
if (offset >= fileSize) break; offset += fragmentSize;
// update the session details if (offset >= fileSize) break;
session["expirationDateTime"] = response["expirationDateTime"]; // update the session details
session["nextExpectedRanges"] = response["nextExpectedRanges"]; session["expirationDateTime"] = response["expirationDateTime"];
save(); session["nextExpectedRanges"] = response["nextExpectedRanges"];
} catch (OneDriveException e) { save();
// there was an error remove session file } catch (OneDriveException e) {
if (exists(sessionFilePath)) { // there was an error remove session file
remove(sessionFilePath); 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() private void save()