From 8ffa25d373eb39046dbc9b06b6e11d86a371f58f Mon Sep 17 00:00:00 2001 From: abraunegg Date: Fri, 6 Mar 2026 07:11:52 +1100 Subject: [PATCH] Update util.d * Update PR --- src/util.d | 217 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 122 insertions(+), 95 deletions(-) diff --git a/src/util.d b/src/util.d index 1df6fd6f..8fddada5 100644 --- a/src/util.d +++ b/src/util.d @@ -66,9 +66,10 @@ shared static this() { // To assist with filesystem severity issues, configure an enum that can be used enum FsErrorSeverity { - warning, - error, - fatal + warning, + error, + fatal, + permission } // Creates a safe backup of the given item, and only performs the function if not in a --dry-run scenario. @@ -1066,8 +1067,9 @@ void displayFileSystemErrorMessage(string message, string callingFunction, strin // Header prefix for logging accuracy string headerPrefix = severity == FsErrorSeverity.warning ? "WARNING" - : severity == FsErrorSeverity.fatal ? "FATAL" - : "ERROR"; + : severity == FsErrorSeverity.permission ? "WARNING" + : severity == FsErrorSeverity.fatal ? "FATAL" + : "ERROR"; // Filesystem logging header addLogEntry(headerPrefix ~ ": The local file system returned an error with the following details:"); @@ -1114,65 +1116,75 @@ void displayFileSystemErrorMessage(string message, string callingFunction, strin addLogEntry(" Error Message: No error message available"); } - // Disk space diagnostics (best-effort) - // We intentionally probe both the current directory and the target path directory when possible. - try { - // Always check the current working directory as a baseline - ulong freeCwd = to!ulong(getAvailableDiskSpace(".")); - addLogEntry(" Disk Space (CWD): " ~ to!string(freeCwd) ~ " bytes available"); + // Disk space diagnostics (best-effort) - if this is not a permission issue + if (severity != FsErrorSeverity.permission) { + // We intentionally probe both the current directory and the target path directory when possible. + try { + // Always check the current working directory as a baseline + ulong freeCwd = to!ulong(getAvailableDiskSpace(".")); + addLogEntry(" Disk Space (CWD): " ~ to!string(freeCwd) ~ " bytes available"); - // If we have a context path, also check its parent directory when possible. - // We keep this conservative: if anything throws, just log the exception. - if (!contextPath.empty) { - string targetProbePath = contextPath; + // If we have a context path, also check its parent directory when possible. + // We keep this conservative: if anything throws, just log the exception. + if (!contextPath.empty) { + string targetProbePath = contextPath; - // If it's a file path, probe the parent directory (where writes/renames happen). - // Avoid throwing if parentDir isn't available or contextPath is weird. - try { - // std.path.dirName handles both file/dir paths; if it returns ".", keep as-is. - import std.path : dirName; - auto parent = dirName(contextPath); - if (!parent.empty) targetProbePath = parent; - } catch (Exception e) { - addLogEntry(" NOTE: Failed to derive parent directory from path: " ~ e.msg); - } - - ulong freeTarget = to!ulong(getAvailableDiskSpace(targetProbePath)); - addLogEntry(" Disk Space (Path): " ~ to!string(freeTarget) ~ " bytes available (parent path: " ~ targetProbePath ~ ")"); - - // Preserve existing behaviour: if disk space check returns 0, force exit. - // (Assumes getAvailableDiskSpace returns 0 on a hard failure in your implementation.) - if (freeTarget == 0 || freeCwd == 0) { - // Must force exit here, allow logging to be done - forceExit(); - } - } else { - // Preserve existing behaviour: if disk space check returns 0, force exit. - if (freeCwd == 0) { - forceExit(); + // If it's a file path, probe the parent directory (where writes/renames happen). + // Avoid throwing if parentDir isn't available or contextPath is weird. + try { + // std.path.dirName handles both file/dir paths; if it returns ".", keep as-is. + import std.path : dirName; + auto parent = dirName(contextPath); + if (!parent.empty) targetProbePath = parent; + } catch (Exception e) { + addLogEntry(" NOTE: Failed to derive parent directory from path: " ~ e.msg); + } + + ulong freeTarget = to!ulong(getAvailableDiskSpace(targetProbePath)); + addLogEntry(" Disk Space (Path): " ~ to!string(freeTarget) ~ " bytes available (parent path: " ~ targetProbePath ~ ")"); + + // Preserve existing behaviour: if disk space check returns 0, force exit. + // (Assumes getAvailableDiskSpace returns 0 on a hard failure in your implementation.) + if (freeTarget == 0 || freeCwd == 0) { + // Must force exit here, allow logging to be done + forceExit(); + } + } else { + // Preserve existing behaviour: if disk space check returns 0, force exit. + if (freeCwd == 0) { + forceExit(); + } } + } catch (Exception e) { + // Handle exceptions from disk space check or type conversion + addLogEntry(" NOTE: Exception during disk space check: " ~ e.msg); } - } catch (Exception e) { - // Handle exceptions from disk space check or type conversion - addLogEntry(" NOTE: Exception during disk space check: " ~ e.msg); } // Add note for WARNING messages - if (headerPrefix == "WARNING") { + if (severity == FsErrorSeverity.warning) { addLogEntry(); - addLogEntry("NOTE: This error is non-fatal; the client will continue to operate, but this may affect future operations if not resolved"); + addLogEntry("NOTE: This warning is non-fatal; the client will continue to operate, but this may affect future operations if not resolved"); + addLogEntry(); + } + + // Add note for filesystem permission messages + if (severity == FsErrorSeverity.permission) { + addLogEntry(); + addLogEntry("NOTE: Sync will continue. This file’s timestamps could not be updated because the effective user does not own the file."); + addLogEntry(" Fix: Run the client as the file owner, or change ownership of the sync tree so it is owned by the user running the client."); addLogEntry(); } // Add note for ERROR messages - if (headerPrefix == "ERROR") { + if (severity == FsErrorSeverity.error) { addLogEntry(); addLogEntry("NOTE: This error requires attention; the client may continue running, but functionality is impaired and the issue should be resolved."); addLogEntry(); } // Add note for FATAL messages - if (headerPrefix == "FATAL") { + if (severity == FsErrorSeverity.fatal) { addLogEntry(); addLogEntry("NOTE: This error is fatal; the client cannot continue and this issue must be corrected before retrying. The client will now attempt to exit in a safe and orderly manner."); addLogEntry(); @@ -2103,36 +2115,6 @@ private bool safeGetTimes(string path, out SysTime accessTime, out SysTime modTi return false; } -// Retry wrapper for setTimes() -private bool safeSetTimes_original(string path, SysTime accessTime, SysTime modTime, string thisFunctionName) { - int maxAttempts = 5; - - foreach (attempt; 0 .. maxAttempts) { - try { - setTimes(path, accessTime, modTime); - return true; - } catch (FileException e) { - // If the path disappeared before we could set, there's nothing useful to do - if (e.errno == ENOENT) { - return false; - } - - if (isTransientErrno(e.errno)) { - // slightly longer backoff here is fine too, but keep it simple/consistent - Thread.sleep(dur!"msecs"(15 * (attempt + 1))); - continue; - } - - displayFileSystemErrorMessage(e.msg, thisFunctionName, path); - return false; - } - } - - displayFileSystemErrorMessage("Failed to set file timestamps after retries", thisFunctionName, path); - return false; -} - - // Some errnos are 'expected' in the wild (permissions, RO mounts, immutable files) // What is this errno private bool isExpectedPermissionStyleErrno(int err) { @@ -2140,12 +2122,37 @@ private bool isExpectedPermissionStyleErrno(int err) { return err == EPERM || err == EACCES || err == EROFS; } +// Helper function to determine path mismatch against UID|GID and process effective UID +private bool getPathOwnerMismatch(string path, out uint fileUid, out uint effectiveUid) { + version (Posix) { + stat_t st; + + // Ensure we pass a NUL-terminated string to the C API + const(char)* cpath = toStringz(path); + + if (lstat(cpath, &st) != 0) { + if (debugLogging) { + addLogEntry("getPathOwnerMismatch(): lstat() failed for '" ~ path ~ "'", ["debug"]); + } + fileUid = 0; + effectiveUid = cast(uint) geteuid(); + return false; + } + + fileUid = cast(uint) st.st_uid; + effectiveUid = cast(uint) geteuid(); + return fileUid != effectiveUid; + } else { + fileUid = 0; + effectiveUid = 0; + return false; + } +} // Retry wrapper for setTimes() private bool safeSetTimes(string path, SysTime accessTime, SysTime modTime, string thisFunctionName) { enum int maxAttempts = 5; - bool permissionStyleWarningShown = false; foreach (attempt; 0 .. maxAttempts) { try { @@ -2166,24 +2173,47 @@ private bool safeSetTimes(string path, SysTime accessTime, SysTime modTime, stri continue; } - // Non-transient: special-case common permission / RO mount errors + // Non-transient: special-case common permission errors + // The user running the client needs to be the owner of the files if the client needs to set explicit timestamps + // See https://github.com/abraunegg/onedrive/issues/3651 for details if (isExpectedPermissionStyleErrno(e.errno)) { - // Only show a permission error message once, even though we may exit here, ensure this is only shown once per path attempt - if (!permissionStyleWarningShown) { - permissionStyleWarningShown = true; - - // Display applicable message for the user regarding permission error on path - displayFileSystemErrorMessage( - e.msg ~ - "\nNOTE: Unable to set local path timestamps (mtime/atime). The client can continue, " ~ - "but this item may be reprocessed more often. Common causes: path ownership/permissions, " ~ - "immutable attribute, or a filesystem/mount that disallows timestamp changes (e.g. CIFS/SMB, " ~ - "FUSE, NTFS/exFAT, read-only mounts).", - thisFunctionName, - path - ); + // Configure application message to display + string permissionErrorMessage = "Unable to set local file timestamps (mtime/atime): Operation not permitted"; + if (e.errno == EPERM) { + permissionErrorMessage = permissionErrorMessage ~ " (EPERM)"; } + + if (e.errno == EACCES) { + permissionErrorMessage = permissionErrorMessage ~ " (EACCES)"; + } + + if (e.errno == EROFS) { + permissionErrorMessage = permissionErrorMessage ~ " (EROFS)"; + } + + // Get extra details if required + string extraHint; + uint fileUid; + uint effectiveUid; + + if (e.errno == EPERM && getPathOwnerMismatch(path, fileUid, effectiveUid)) { + extraHint = + "\nThe client runtime user does not own this file. Effective UID=" ~ to!string(effectiveUid) ~ ", file owner UID=" ~ to!string(fileUid) ~ "." ~ + "\nOn Linux, setting explicit timestamps generally requires file ownership (or CAP_FOWNER)."; + + // Update permissionErrorMessage to add extraHint + permissionErrorMessage = permissionErrorMessage ~ extraHint; + } + + // Display applicable message for the user regarding permission error on path + displayFileSystemErrorMessage( + permissionErrorMessage, + thisFunctionName, + path, + FsErrorSeverity.permission + ); + // It is pointless attempting a re-try in this scenario as those conditions will not change by retrying 15ms later. return false; } @@ -2199,9 +2229,6 @@ private bool safeSetTimes(string path, SysTime accessTime, SysTime modTime, stri return false; } - - - // Set the timestamp of the provided path to ensure this is done in a consistent manner void setLocalPathTimestamp(bool dryRun, string inputPath, SysTime newTimeStamp) { // Set this function name