Skip to content

Commit

Permalink
Fix issue #3057
Browse files Browse the repository at this point in the history 
* When --disable-download-validation is used, we still need to set the file timestamp correctly to avoid integrity checking issues
* Implement setFileTimestamp() as a common function so that when setting a file timestamp this is done in a consistent manner
  • Loading branch information
@abraunegg
abraunegg committed Jan 11, 2025
1 parent 5a20154 commit 2483cb3
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 93 deletions.
151 changes: 58 additions & 93 deletions src/sync.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2649,9 +2649,8 @@ class SyncEngine {
// Otherwise when we do the DB check, the move on the file system, the file technically has a newer timestamp
// which is 'correct' .. but we need to report locally the online timestamp here as the move was made online
if (changedOneDriveItem.type == ItemType.file) {
// Set the timestamp
if (debugLogging) {addLogEntry("Calling setTimes() for this file: " ~ changedItemPath, ["debug"]);}
setTimes(changedItemPath, changedOneDriveItem.mtime, changedOneDriveItem.mtime);
// Set the timestamp, logging and error handling done within function
setFileTimestamp(dryRun, changedItemPath, changedOneDriveItem.mtime);
}
} else {
// --dry-run situation - the actual rename did not occur - but we need to track like it did
Expand Down Expand Up @@ -2914,7 +2913,40 @@ class SyncEngine {
// When downloading some files from SharePoint, the OneDrive API reports one file size,
// but the SharePoint HTTP Server sends a totally different byte count for the same file
// we have implemented --disable-download-validation to disable these checks


// Regardless of --disable-download-validation we still need to set the file timestamp correctly
// Get the mtime from the JSON data
SysTime itemModifiedTime;
string lastModifiedTimestamp;
if (isItemRemote(onedriveJSONItem)) {
// remote file item
lastModifiedTimestamp = strip(onedriveJSONItem["remoteItem"]["fileSystemInfo"]["lastModifiedDateTime"].str);
// is lastModifiedTimestamp valid?
if (isValidUTCDateTime(lastModifiedTimestamp)) {
// string is a valid timestamp
itemModifiedTime = SysTime.fromISOExtString(lastModifiedTimestamp);
} else {
// invalid timestamp from JSON file
addLogEntry("WARNING: Invalid timestamp provided by the Microsoft OneDrive API: " ~ lastModifiedTimestamp);
// Set mtime to Clock.currTime(UTC()) given that the time in the JSON should be a UTC timestamp
itemModifiedTime = Clock.currTime(UTC());
}
} else {
// not a remote item
lastModifiedTimestamp = strip(onedriveJSONItem["fileSystemInfo"]["lastModifiedDateTime"].str);
// is lastModifiedTimestamp valid?
if (isValidUTCDateTime(lastModifiedTimestamp)) {
// string is a valid timestamp
itemModifiedTime = SysTime.fromISOExtString(lastModifiedTimestamp);
} else {
// invalid timestamp from JSON file
addLogEntry("WARNING: Invalid timestamp provided by the Microsoft OneDrive API: " ~ lastModifiedTimestamp);
// Set mtime to Clock.currTime(UTC()) given that the time in the JSON should be a UTC timestamp
itemModifiedTime = Clock.currTime(UTC());
}
}

// Did the user configure --disable-download-validation ?
if (!disableDownloadValidation) {
// A 'file' was downloaded - does what we downloaded = reported jsonFileSize or if there is some sort of funky local disk compression going on
// Does the file hash OneDrive reports match what we have locally?
Expand All @@ -2936,52 +2968,8 @@ class SyncEngine {
// Downloaded file matches size and hash
if (debugLogging) {addLogEntry("Downloaded file matches reported size and reported file hash", ["debug"]);}

try {
// get the mtime from the JSON data
SysTime itemModifiedTime;
string lastModifiedTimestamp;
if (isItemRemote(onedriveJSONItem)) {
// remote file item
lastModifiedTimestamp = strip(onedriveJSONItem["remoteItem"]["fileSystemInfo"]["lastModifiedDateTime"].str);
// is lastModifiedTimestamp valid?
if (isValidUTCDateTime(lastModifiedTimestamp)) {
// string is a valid timestamp
itemModifiedTime = SysTime.fromISOExtString(lastModifiedTimestamp);
} else {
// invalid timestamp from JSON file
addLogEntry("WARNING: Invalid timestamp provided by the Microsoft OneDrive API: " ~ lastModifiedTimestamp);
// Set mtime to Clock.currTime(UTC()) given that the time in the JSON should be a UTC timestamp
itemModifiedTime = Clock.currTime(UTC());
}
} else {
// not a remote item
lastModifiedTimestamp = strip(onedriveJSONItem["fileSystemInfo"]["lastModifiedDateTime"].str);
// is lastModifiedTimestamp valid?
if (isValidUTCDateTime(lastModifiedTimestamp)) {
// string is a valid timestamp
itemModifiedTime = SysTime.fromISOExtString(lastModifiedTimestamp);
} else {
// invalid timestamp from JSON file
addLogEntry("WARNING: Invalid timestamp provided by the Microsoft OneDrive API: " ~ lastModifiedTimestamp);
// Set mtime to Clock.currTime(UTC()) given that the time in the JSON should be a UTC timestamp
itemModifiedTime = Clock.currTime(UTC());
}
}

// set the correct time on the downloaded file
if (!dryRun) {
if (debugLogging) {addLogEntry("Calling setTimes() for this file: " ~ newItemPath, ["debug"]);}
try {
setTimes(newItemPath, itemModifiedTime, itemModifiedTime);
} catch (FileException e) {
// display the error message
displayFileSystemErrorMessage(e.msg, getFunctionName!({}));
}
}
} catch (FileException e) {
// display the error message
displayFileSystemErrorMessage(e.msg, getFunctionName!({}));
}
// Set the timestamp, logging and error handling done within function
setFileTimestamp(dryRun, newItemPath, itemModifiedTime);
} else {
// Downloaded file does not match size or hash .. which is it?
bool downloadValueMismatch = false;
Expand Down Expand Up @@ -3056,7 +3044,11 @@ class SyncEngine {
// Download validation checks were disabled
if (debugLogging) {addLogEntry("Downloaded file validation disabled due to --disable-download-validation", ["debug"]);}
if (verboseLogging) {addLogEntry("WARNING: Skipping download integrity check for: " ~ newItemPath, ["verbose"]);}
} // end of (!disableDownloadValidation)

// Whilst the download integrity checks were disabled, we still have to set the correct timestamp on the file
// Set the timestamp, logging and error handling done within function
setFileTimestamp(dryRun, newItemPath, itemModifiedTime);
} // end of (!disableDownloadValidation)
} else {
addLogEntry("ERROR: File failed to download. Increase logging verbosity to determine why.");
// Was this item previously in-sync with the local system?
Expand Down Expand Up @@ -3148,14 +3140,8 @@ class SyncEngine {
// --resync was used
// The source of the out-of-date timestamp was the local item and needs to be corrected ... but why is it newer - indexing application potentially changing the timestamp ?
if (verboseLogging) {addLogEntry("The source of the incorrect timestamp was the local file - correcting timestamp locally due to --resync", ["verbose"]);}
// Fix the local file timestamp
if (debugLogging) {addLogEntry("Calling setTimes() for this file: " ~ path, ["debug"]);}
try {
setTimes(path, item.mtime, item.mtime);
} catch (FileException e) {
// display the error message
displayFileSystemErrorMessage(e.msg, getFunctionName!({}));
}
// Fix the timestamp, logging and error handling done within function
setFileTimestamp(dryRun, path, item.mtime);
} else {
// The source of the out-of-date timestamp was OneDrive and this needs to be corrected to avoid always generating a hash test if timestamp is different
if (verboseLogging) {addLogEntry("The source of the incorrect timestamp was OneDrive online - correcting timestamp online", ["verbose"]);}
Expand All @@ -3172,26 +3158,15 @@ class SyncEngine {
} else if (!dryRun) {
// --download-only is being used ... local file needs to be corrected ... but why is it newer - indexing application potentially changing the timestamp ?
if (verboseLogging) {addLogEntry("The source of the incorrect timestamp was the local file - correcting timestamp locally due to --download-only", ["verbose"]);}
// Fix the local file timestamp
if (debugLogging) {addLogEntry("Calling setTimes() for this file: " ~ path, ["debug"]);}
try {
setTimes(path, item.mtime, item.mtime);
} catch (FileException e) {
// display the error message
displayFileSystemErrorMessage(e.msg, getFunctionName!({}));
}
// Fix the timestamp, logging and error handling done within function
setFileTimestamp(dryRun, path, item.mtime);
}
} else if (!dryRun) {
// The source of the out-of-date timestamp was the local file and this needs to be corrected to avoid always generating a hash test if timestamp is different
if (verboseLogging) {addLogEntry("The source of the incorrect timestamp was the local file - correcting timestamp locally", ["verbose"]);}
// Fix the local file timestamp
if (debugLogging) {addLogEntry("Calling setTimes() for this file: " ~ path, ["debug"]);}
try {
setTimes(path, item.mtime, item.mtime);
} catch (FileException e) {
// display the error message
displayFileSystemErrorMessage(e.msg, getFunctionName!({}));
}

// Fix the timestamp, logging and error handling done within function
setFileTimestamp(dryRun, path, item.mtime);
}
return false;
} else {
Expand Down Expand Up @@ -3842,27 +3817,16 @@ class SyncEngine {
} else {
// Remote file, remote values need to be used, we may not even have permission to change timestamp, update local file
if (verboseLogging) {addLogEntry("The local item has the same hash value as the item online, however file is a OneDrive Business Shared File - correcting local timestamp", ["verbose"]);}
if (debugLogging) {addLogEntry("Calling setTimes() for this file: " ~ localFilePath, ["debug"]);}
try {
setTimes(localFilePath, dbItem.mtime, dbItem.mtime);
} catch (FileException e) {
// display the error message
displayFileSystemErrorMessage(e.msg, getFunctionName!({}));
}

// Set the timestamp, logging and error handling done within function
setFileTimestamp(dryRun, localFilePath, dbItem.mtime);
}
}
} else {
// --download-only being used
if (verboseLogging) {addLogEntry("The local item has the same hash value as the item online - correcting local timestamp due to --download-only being used to ensure local file matches timestamp online", ["verbose"]);}
if (!dryRun) {
if (debugLogging) {addLogEntry("Calling setTimes() for this file: " ~ localFilePath, ["debug"]);}
try {
setTimes(localFilePath, dbItem.mtime, dbItem.mtime);
} catch (FileException e) {
// display the error message
displayFileSystemErrorMessage(e.msg, getFunctionName!({}));
}
}
// Set the timestamp, logging and error handling done within function
setFileTimestamp(dryRun, localFilePath, dbItem.mtime);
}
}
} else {
Expand Down Expand Up @@ -4963,8 +4927,9 @@ class SyncEngine {
// create an applicable item
Item onlineItem;
onlineItem = makeItem(uploadResponse);
// correct the local file timestamp to avoid creating a new version online
setTimes(localFilePath, onlineItem.mtime, onlineItem.mtime);
// Correct the local file timestamp to avoid creating a new version online
// Set the timestamp, logging and error handling done within function
setFileTimestamp(dryRun, localFilePath, onlineItem.mtime);
} else {
// Create a new online version of the file by updating the metadata, which negates the need to download the file
uploadLastModifiedTime(dbItem, targetDriveId, targetItemId, localModifiedTime, etagFromUploadResponse);
Expand Down
24 changes: 24 additions & 0 deletions src/util.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1533,3 +1533,27 @@ void checkOpenSSLVersion() {
}
}
}

// Set the timestamp of the provided file path to ensure this is done in a consistent manner
void setFileTimestamp(bool dryRun, string filePath, SysTime newTimeStamp) {
// Try and set the local file timestamp, catch filesystem error
try {
// set the correct time on the requested filepath
if (!dryRun) {
if (debugLogging) {
addLogEntry("Calling setTimes() for this file: " ~ filePath, ["debug"]);
addLogEntry("* new timestamp for this file will be: " ~ to!string(newTimeStamp), ["debug"]);
}
// make the change
try {
setTimes(filePath, newTimeStamp, newTimeStamp);
} catch (FileException e) {
// display the error message
displayFileSystemErrorMessage(e.msg, getFunctionName!({}));
}
}
} catch (FileException e) {
// display the error message
displayFileSystemErrorMessage(e.msg, getFunctionName!({}));
}
}

0 comments on commit 2483cb3

Please sign in to comment.