A name a snapshot cannot hold gets a notification of its own

Reported apart from unreadable files, with example paths and the advice to rename them, instead of advice about permissions. Start of the 1.1.0 plan in PLAN.md.
2026-11-06 23:38:40 +01:00
parent f81c5e13e9
commit 6540a9314a
8 changed files with 63 additions and 5 deletions
@@ -11,6 +11,13 @@ Each heading starts with that `<version>`, once: the app store shows a release t
 whose heading starts with its `x.y.z` and ignores anything after it, so `## [1.0.0-rc.6]` would be
 shown under 1.0.0. The git tag of a release candidate goes after the date.

+## [Unreleased]
+
+### Changed
+- A file or folder that is left out because of its name now gets a notification of its own, in the
+  log and in `occ backup:run`. It says the item is missing from the backup, gives example paths and
+  asks to rename it; before, it was reported as unreadable, with advice about permissions.
+
 ## [1.0.3] - 2026-11-06

 ### Fixed
@@ -472,9 +472,12 @@ in README "Known limitations":
  tijd terug. Hoort zo, vooral bij oudere verzamelingen: de maptijd wordt gebruikt om in te schatten
  wanneer een map is aangemaakt. De backup moet dan de tijd van alle mappen vastleggen en de restore
  die aan het eind terugzetten; werkt pas vanaf de eerste backup met die versie. Op de roadmap onder 1.1.0.
+  **1.0.3 uitgebracht (2026-11-06)**, ook op nc01 geïnstalleerd.
 - **Branches (besluit 2026-11-06):** werk voor een versie op een branch met die naam (`1.0.3`);
  `polish` alleen voor kleine cosmetische zaken, per geval bekijken of en wanneer die gemerged
  worden. SMB staat uit op sub5 en sub6; sub6 is het testaccount, eerst aanzetten voor een SMB-test.
+  Na 1.0.3 gaat het werk naar `1.1.0`. Een spoedfix die niet kan wachten gaat op `1.0.4` vanaf `main`,
+  krijgt een eigen merge en release; daarna `main` in `1.1.0` mergen, zodat daar met de fix getest wordt.
 - **1.0.1** (uitgebracht 2026-11-01): alleen de kleine punten (besluit 2026-10-30): 4, 6–13. Punten 1, 2 en 5 gaan naar
  1.0.2 (README-roadmap aangepast); punt 3 is een test, ingepland voor de week van 2 november,
  niet aangekondigd en los van de release. Voor 1.0.2 ook: nieuwe screenshots (het labelveld bij
@@ -569,7 +572,13 @@ in README "Known limitations":
 - **Polish:** `target:add` toonde bij het eerste indexbestand al "about 642 min left" (S3,
  4.568 bestanden, daarna binnen een minuut klaar): een schatting pas na ~5% of na meer dan
  een paar bestanden.
- **1.1.0:**
+- **1.1.0** (geen harde datum: een week was een voorzet, langer mag; de releasefrequentie mag omlaag,
+  besluit 2026-11-06):
+  **Betere melding voor namen die een snapshot niet kan bevatten** (backslash, stuurteken, geen UTF-8):
+  een eigen melding in plaats van "unreadable" met het advies over rechten. Zegt dat deze bestanden of
+  mappen in de backup ontbreken, geeft voorbeelden van de paden en vraagt ze te hernoemen; de volgende
+  backup neemt ze mee. Alleen als Nextcloud-melding: de app mailt niet zelf, of meldingen gemaild worden
+  is de keuze van de beheerder (besluit 2026-11-06).
  1. `backup:key:import` zonder kit: de sleutel-slots van de locatie (`config`) lezen, met de kit
     als reserve als `config` ontbreekt of niet opent. Vergt een andere volgorde van
     `target:add` en sleutelimport (het gevoeligste pad), daarom niet in rc1. Daarna de
@@ -79,6 +79,9 @@ HELP);
 				if (($f['unreadable'] ?? 0) > 0) {
 					$output->writeln(sprintf('<comment>  warning: %d unreadable files or folders are missing from this backup (see the server log)</comment>', $f['unreadable']));
 				}
+				if (($f['badNames'] ?? 0) > 0) {
+					$output->writeln(sprintf('<comment>  warning: %d files or folders are missing from this backup because their name is not supported; rename them and the next backup includes them (see the server log)</comment>', $f['badNames']));
+				}
 				if (($f['changing'] ?? 0) > 0) {
 					$output->writeln(sprintf('<comment>  warning: %d files kept changing and were saved as read (see the server log)</comment>', $f['changing']));
 				}
@@ -64,7 +64,7 @@ final class BackupRun {
 					// such path must not make a backup fail, or the whole snapshot unreadable later.
 					// The cursor stays where it is: the state is JSON, which cannot hold such a name. A
 					// next step meets it again, and it is reported once.
-					self::recordUnreadable($state, $stats, mb_scrub($logical, 'UTF-8'));
+					self::recordBadName($state, $stats, $logical);
 					continue;
 				}
 				if (is_dir($path)) {
@@ -250,10 +250,26 @@ final class BackupRun {
 		$stats['unreadable'] = ($stats['unreadable'] ?? 0) + 1;
 	}

-	/** @return array{unreadable: list<string>, changing: list<string>} examples for the report (first 50 each) */
+	/**
+	 * A file or directory left out because of its name. Remembered like an unreadable one, in a form
+	 * that fits the JSON state and is safe to show: control characters and invalid bytes replaced.
+	 */
+	private static function recordBadName(array &$state, array &$stats, string $logical): void {
+		$shown = (string)preg_replace('/[\x00-\x1f\x7f]/', '?', mb_scrub($logical, 'UTF-8'));
+		if (isset($state['badNames'][$shown])) {
+			return;
+		}
+		if (count($state['badNames'] ?? []) < 1000) {
+			$state['badNames'][$shown] = true;
+		}
+		$stats['badNames'] = ($stats['badNames'] ?? 0) + 1;
+	}
+
+	/** @return array{unreadable: list<string>, badNames: list<string>, changing: list<string>} examples for the report (first 50 each) */
 	public static function warnings(array $state): array {
 		return [
 			'unreadable' => array_slice(array_map('strval', array_keys($state['unreadable'] ?? [])), 0, 50),
+			'badNames' => array_slice(array_map('strval', array_keys($state['badNames'] ?? [])), 0, 50),
 			'changing' => array_slice(array_map('strval', array_values($state['changing'] ?? [])), 0, 50),
 		];
 	}
@@ -46,6 +46,10 @@ final class Notifier implements INotifier {
 				$notification->setParsedSubject($l->t('Backup to %1$s finished with warnings: %2$s unreadable, %3$s changed while being read', [$p['target'], $p['unreadable'], $p['changing']]));
 				$notification->setParsedMessage($l->t('Unreadable files and folders are missing from this backup (check their permissions). Files that kept changing were saved as they were while being read. Examples: %s', [$p['examples'] ?? '']));
 				break;
+			case 'run_bad_names':
+				$notification->setParsedSubject($l->n('Backup to %s finished, but %n item is missing because of its name', 'Backup to %s finished, but %n items are missing because of their names', (int)$p['count'], [$p['target']]));
+				$notification->setParsedMessage($l->t('Names with a backslash or a control character, or that are not valid UTF-8, are not supported, so these files and folders are not in this backup. Rename them; the next backup includes them. Examples: %s', [$p['examples'] ?? '']));
+				break;
 			case 'quota_warning':
 				$notification->setParsedSubject($l->t('Backup location %1$s is at %2$s%% of its limit (%3$s of %4$s)', [$p['target'], $p['percent'], $p['used'], $p['limit']]));
 				$notification->setParsedMessage($l->t('Backups stop when the limit is reached. Raise the limit, or let the retention policy remove older backups.'));
@@ -61,6 +61,16 @@ final class AlertService {
 			'examples' => mb_strimwidth(implode(', ', array_merge($examples['unreadable'], $examples['changing'])), 0, 300, '…')]);
 	}

+	/**
+	 * A backup that finished but left out files or folders because of their name.
+	 *
+	 * @param list<string> $examples
+	 */
+	public function runBadNames(Run $run, string $targetName, int $count, array $examples): void {
+		$this->notifyAdmins('run_bad_names', 'run-' . $run->getId(), ['target' => $targetName, 'count' => (string)$count,
+			'examples' => mb_strimwidth(implode(', ', $examples), 0, 300, '…')]);
+	}
+
 	/**
 	 * Remember how much is stored on a location (for the settings page) and warn administrators
 	 * once a day when it is at 80% of its limit or more.
@@ -253,8 +253,13 @@ final class BackupService {
 					try {
 						$this->alerts->storedBytes($target, $repo->storedBytes());
 						$stats = $state['files']['stats'];
+						$warnings = BackupRun::warnings($state['files']);
+						if (($stats['badNames'] ?? 0) > 0) {
+							$this->logger->warning(sprintf('NG Backup run %d to %s: %d left out because their name is not supported (rename them): %s',
+								$run->getId(), $target->getName(), $stats['badNames'], implode(', ', $warnings['badNames'])));
+							$this->alerts->runBadNames($run, $target->getName(), (int)$stats['badNames'], $warnings['badNames']);
+						}
 						if (($stats['unreadable'] ?? 0) > 0 || ($stats['changing'] ?? 0) > 0) {
-							$warnings = BackupRun::warnings($state['files']);
 							$this->logger->warning(sprintf('NG Backup run %d to %s: %d unreadable (missing from the snapshot): %s; %d changed while being read: %s',
 								$run->getId(), $target->getName(), $stats['unreadable'] ?? 0, implode(', ', $warnings['unreadable']),
 								$stats['changing'] ?? 0, implode(', ', $warnings['changing'])));
@@ -82,7 +82,11 @@ class EmptyDirsTest extends TestCase {
 			$state = json_decode(json_encode(BackupRun::step($this->repo(), $state, 0.0), JSON_THROW_ON_ERROR), true, 512, JSON_THROW_ON_ERROR);
 		}
 		$this->assertSame('commit', $state['phase']);
-		$this->assertSame(4, $state['stats']['unreadable'], 'each reported once, over several steps');
+		$this->assertSame(4, $state['stats']['badNames'], 'each reported once, over several steps');
+		$this->assertArrayNotHasKey('unreadable', $state['stats'], 'not a problem with permissions');
+		$shown = BackupRun::warnings($state)['badNames'];
+		$this->assertContains('data/alice/files/line?break', $shown, 'no control character in a message');
+		$this->assertSame($shown, array_filter($shown, fn (string $name): bool => mb_check_encoding($name, 'UTF-8')));
 		$this->repo()->recordSnapshot($state['snapshot']);

 		$this->assertSame(['data/alice/files/Hollow/Nested', 'data/skipped/inner'], array_column($this->repo()->emptyDirs($state['snapshot']), 0));