fix: preserve whitespace in sync paths, surface unresolved paths, O(1) lookups

- FileSyncCompareItem._validate_file_path no longer trims whitespace off
  the client-provided path. This endpoint is a path-identity protocol
  and some filesystems (notably Linux) treat leading/trailing spaces
  as significant; stripping collapsed " report.txt" and "report.txt"
  to the same key and made the compare response fail to round-trip the
  client's directoryFiles lookup. Blank values (empty or whitespace-
  only) are still rejected, but the returned path matches the input
  character-for-character.
- syncDirectoryHandler now builds a filesByPath Map once and uses it
  for O(1) lookups in the new-files and changed-files loops instead
  of a per-iteration directoryFiles.find() (quadratic for large
  directories).
- Both loops now have explicit else branches when a planned path
  doesn't resolve to a collected file: they count it as a failure,
  log the path, surface a toast, and still bump processedCount so the
  summary reports real outcomes. Previously the iteration was a
  silent no-op and a server-planned file that didn't resolve locally
  would not even register as missed.
This commit is contained in:
DrMelone 2026-04-13 00:37:24 +02:00
parent 624b360eb1
commit a59beeaaf9
2 changed files with 86 additions and 47 deletions

View file

@ -1225,13 +1225,19 @@ class FileSyncCompareItem(BaseModel):
def _validate_file_path(cls, value: str) -> str:
if '\x00' in value:
raise ValueError('file_path must not contain NUL bytes')
stripped = value.strip()
if not stripped:
# Reject paths that are effectively blank (empty or whitespace-only)
# without mutating the input: this endpoint is a path-identity
# protocol and some filesystems (notably Linux) treat leading or
# trailing whitespace as significant. Stripping the value would
# collapse " report.txt" and "report.txt" to the same key and the
# response path would no longer match the client's directoryFiles
# lookup.
if not value.strip():
raise ValueError('file_path must not be blank')
parts = stripped.replace('\\', '/').split('/')
if any(part in ('..',) for part in parts):
parts = value.replace('\\', '/').split('/')
if any(part == '..' for part in parts):
raise ValueError('file_path must not contain traversal segments')
return stripped
return value
@field_validator('file_hash')
@classmethod

View file

@ -589,7 +589,14 @@
}
const { new_files, changed_files, removed_file_ids, unchanged } = comparison;
// Build a path -> collected-file map once so per-file lookups in
// the new/changed loops are O(1) instead of O(n). Also gives us
// one place to detect server-planned paths the client can't
// resolve — which should not happen but we guard defensively so
// the sync summary can't over-report completion.
const filesByPath = new Map(directoryFiles.map((f) => [f.path, f]));
const totalToProcess = new_files.length + changed_files.length;
let processedCount = 0;
@ -647,22 +654,35 @@
let newSucceeded = 0;
let newFailed = 0;
for (const filePath of new_files) {
const fileData = directoryFiles.find((f) => f.path === filePath);
if (fileData) {
const ok = await uploadFileHandler(fileData.file);
if (ok) {
newSucceeded++;
} else {
newFailed++;
}
processedCount++;
toast.info(
$i18n.t('Uploading new: {{current}}/{{total}}', {
current: processedCount,
total: totalToProcess
const fileData = filesByPath.get(filePath);
if (!fileData) {
// Server planned a new-file upload for a path we can't
// map back to a collected file. Count as a failure and
// surface it so the sync summary and the user both see
// that the file wasn't actually uploaded.
newFailed++;
console.error('Sync: could not resolve planned new file path', filePath);
toast.error(
$i18n.t('Failed to upload {{path}}: file not found in scanned directory', {
path: filePath
})
);
processedCount++;
continue;
}
const ok = await uploadFileHandler(fileData.file);
if (ok) {
newSucceeded++;
} else {
newFailed++;
}
processedCount++;
toast.info(
$i18n.t('Uploading new: {{current}}/{{total}}', {
current: processedCount,
total: totalToProcess
})
);
}
// STEP 3: Upload changed files using atomic upload_and_replace
@ -672,38 +692,51 @@
let changedSucceeded = 0;
let changedFailed = 0;
for (const changedFile of changed_files) {
const fileData = directoryFiles.find((f) => f.path === changedFile.file_path);
if (fileData) {
try {
await uploadAndReplaceFile(
localStorage.token,
id,
fileData.file,
changedFile.old_file_id
);
changedSucceeded++;
} catch (replaceErr) {
changedFailed++;
console.error('Replace failed for', changedFile.file_path, replaceErr);
const detail =
typeof replaceErr === 'string'
? replaceErr
: (replaceErr?.detail ?? replaceErr?.message ?? 'unknown error');
toast.error(
$i18n.t('Failed to update {{path}}: {{detail}}', {
path: changedFile.file_path,
detail
})
);
}
const fileData = filesByPath.get(changedFile.file_path);
if (!fileData) {
// Same defensive guard as the new-files loop above.
changedFailed++;
console.error(
'Sync: could not resolve planned replace path',
changedFile.file_path
);
toast.error(
$i18n.t('Failed to update {{path}}: file not found in scanned directory', {
path: changedFile.file_path
})
);
processedCount++;
toast.info(
$i18n.t('Updating: {{current}}/{{total}}', {
current: processedCount,
total: totalToProcess
continue;
}
try {
await uploadAndReplaceFile(
localStorage.token,
id,
fileData.file,
changedFile.old_file_id
);
changedSucceeded++;
} catch (replaceErr) {
changedFailed++;
console.error('Replace failed for', changedFile.file_path, replaceErr);
const detail =
typeof replaceErr === 'string'
? replaceErr
: (replaceErr?.detail ?? replaceErr?.message ?? 'unknown error');
toast.error(
$i18n.t('Failed to update {{path}}: {{detail}}', {
path: changedFile.file_path,
detail
})
);
}
processedCount++;
toast.info(
$i18n.t('Updating: {{current}}/{{total}}', {
current: processedCount,
total: totalToProcess
})
);
}
// Show summary. Use succeeded counts — not the planned counts —