From 7ad5236eda66cf75c1547a9effbe1c24f120c135 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Sat, 1 Aug 2026 20:11:09 +0200 Subject: [PATCH] fix(disk): parse the mount point as the rest of the line `df -P` prints one record per line with the mount point last, so take everything after the capacity column as the mount point. Filesystems mounted under a path containing a space, a dot or a non-ASCII character are reported again. Used and Capacity are matched as \S+ so that filesystems printing "-" there are reported too. Blocks and Available still require digits; they are the only values read. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Frank Karlitschek --- lib/OperatingSystems/FreeBSD.php | 6 ++- lib/OperatingSystems/Linux.php | 6 ++- tests/data/freebsd_df_tp_mount_points | 5 ++ tests/data/linux_df_tp_mount_points | 6 +++ tests/lib/FreeBSDTest.php | 44 ++++++++++++++++ tests/lib/LinuxTest.php | 72 +++++++++++++++++++++++++++ 6 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 tests/data/freebsd_df_tp_mount_points create mode 100644 tests/data/linux_df_tp_mount_points diff --git a/lib/OperatingSystems/FreeBSD.php b/lib/OperatingSystems/FreeBSD.php index ca72bb1d8..624e37583 100644 --- a/lib/OperatingSystems/FreeBSD.php +++ b/lib/OperatingSystems/FreeBSD.php @@ -187,7 +187,11 @@ public function getDiskInfo(): array { } $matches = []; - $pattern = '/^(?[\S]+)\s*(?[\S]+)\s*(?\d+)\s*(?\d+)\s*(?\d+)\s*(?\d+%)\s*(?[\w\/-]+)$/m'; + // `df -P` prints one record per line with the mount point last, so the mount + // point is everything after the capacity column. Used and Capacity are loose + // because nothing reads them and `df` prints "-" for filesystems without + // usage numbers. + $pattern = '/^(?\S+)[ \t]+(?\S+)[ \t]+(?\d+)[ \t]+(?\S+)[ \t]+(?\d+)[ \t]+(?\S+)[ \t]+(?\S.*?)[ \t\r]*$/m'; $result = preg_match_all($pattern, $disks, $matches); if ($result === 0 || $result === false) { diff --git a/lib/OperatingSystems/Linux.php b/lib/OperatingSystems/Linux.php index d463ceed0..16c4293ad 100644 --- a/lib/OperatingSystems/Linux.php +++ b/lib/OperatingSystems/Linux.php @@ -206,7 +206,11 @@ public function getDiskInfo(): array { } $matches = []; - $pattern = '/^(?[\S]+)\s*(?[\S]+)\s*(?\d+)\s*(?\d+)\s*(?\d+)\s*(?\d+%)\s*(?[\w\/\-\.]+)$/m'; + // `df -P` prints one record per line with the mount point last, so the mount + // point is everything after the capacity column. Used and Capacity are loose + // because nothing reads them and `df` prints "-" for filesystems without + // usage numbers. + $pattern = '/^(?\S+)[ \t]+(?\S+)[ \t]+(?\d+)[ \t]+(?\S+)[ \t]+(?\d+)[ \t]+(?\S+)[ \t]+(?\S.*?)[ \t\r]*$/m'; $result = preg_match_all($pattern, $disks, $matches); if ($result === 0 || $result === false) { diff --git a/tests/data/freebsd_df_tp_mount_points b/tests/data/freebsd_df_tp_mount_points new file mode 100644 index 000000000..eae1fe039 --- /dev/null +++ b/tests/data/freebsd_df_tp_mount_points @@ -0,0 +1,5 @@ +Filesystem Type 1024-blocks Used Available Capacity Mounted on +/dev/gpt/rootfs ufs 56422560 6205468 47321220 12% / +zroot/data zfs 10000000 2000000 8000000 20% /mnt/My Backup +zroot/media zfs 20000000 4000000 16000000 20% /mnt/media.old +storage:/vol nfs 40000000 - 32000000 - /mnt/nfs diff --git a/tests/data/linux_df_tp_mount_points b/tests/data/linux_df_tp_mount_points new file mode 100644 index 000000000..9cf2d9ce4 --- /dev/null +++ b/tests/data/linux_df_tp_mount_points @@ -0,0 +1,6 @@ +Filesystem Type 1024-blocks Used Available Capacity Mounted on +/dev/sda1 ext4 56422560 6205468 47321220 12% / +/dev/sdb1 ext4 10000000 2000000 8000000 20% /mnt/My Backup +/dev/sdc1 ext4 20000000 4000000 16000000 20% /mnt/Fotos-Übung +/dev/sdd1 btrfs 30000000 6000000 24000000 20% /mnt/data (old) +storage:/vol nfs4 40000000 - 32000000 - /mnt/nfs diff --git a/tests/lib/FreeBSDTest.php b/tests/lib/FreeBSDTest.php index 431c799e7..b89bdfe75 100644 --- a/tests/lib/FreeBSDTest.php +++ b/tests/lib/FreeBSDTest.php @@ -10,6 +10,7 @@ namespace OCA\ServerInfo\Tests; use OCA\ServerInfo\OperatingSystems\FreeBSD; +use OCA\ServerInfo\Resources\Disk; use OCA\ServerInfo\Resources\Memory; use OCA\ServerInfo\Resources\NetInterface; use PHPUnit\Framework\MockObject\MockObject; @@ -134,6 +135,49 @@ public function testGetNetworkInterfaces(): void { $this->assertEquals($expected, $actual); } + public function testGetDiskInfoUnusualMountPoints(): void { + $this->os->method('executeCommand') + ->with('df -TPk') + ->willReturn(file_get_contents(__DIR__ . '/../data/freebsd_df_tp_mount_points')); + + // getDiskInfo() had no coverage here at all, and this pattern was even + // narrower than the Linux one: it allowed no dots, so a mount point like + // /mnt/media.old removed that filesystem from the report entirely. + $disk1 = new Disk(); + $disk1->setDevice('/dev/gpt/rootfs'); + $disk1->setFs('ufs'); + $disk1->setUsed(8889); + $disk1->setAvailable(46212); + $disk1->setPercent('16.13%'); + $disk1->setMount('/'); + + $disk2 = new Disk(); + $disk2->setDevice('zroot/data'); + $disk2->setFs('zfs'); + $disk2->setUsed(1954); + $disk2->setAvailable(7812); + $disk2->setPercent('20%'); + $disk2->setMount('/mnt/My Backup'); + + $disk3 = new Disk(); + $disk3->setDevice('zroot/media'); + $disk3->setFs('zfs'); + $disk3->setUsed(3907); + $disk3->setAvailable(15625); + $disk3->setPercent('20%'); + $disk3->setMount('/mnt/media.old'); + + $disk4 = new Disk(); + $disk4->setDevice('storage:/vol'); + $disk4->setFs('nfs'); + $disk4->setUsed(7813); + $disk4->setAvailable(31250); + $disk4->setPercent('20%'); + $disk4->setMount('/mnt/nfs'); + + $this->assertEquals([$disk1, $disk2, $disk3, $disk4], $this->os->getDiskInfo()); + } + public function testGetNetworkInfo(): void { $this->os->method('executeCommand') ->with('netstat -rn') diff --git a/tests/lib/LinuxTest.php b/tests/lib/LinuxTest.php index f840b8cd5..ae5f0b35a 100644 --- a/tests/lib/LinuxTest.php +++ b/tests/lib/LinuxTest.php @@ -251,6 +251,78 @@ public function testGetDiskInfoInvalidCommandOutput(): void { $this->assertEquals([], $this->os->getDiskInfo()); } + public function testGetDiskInfoSkipsMalformedLine(): void { + $this->os->method('executeCommand') + ->with('df -TPk') + ->willReturn("a b 1 c 2\n/dev/sda1 ext4 10000000 2000000 8000000 20% /mnt/data\n"); + + $disk = new Disk(); + $disk->setDevice('/dev/sda1'); + $disk->setFs('ext4'); + $disk->setUsed(1954); + $disk->setAvailable(7812); + $disk->setPercent('20%'); + $disk->setMount('/mnt/data'); + + $this->assertEquals([$disk], $this->os->getDiskInfo()); + } + + public function testGetDiskInfoUnusualMountPoints(): void { + $this->os->method('executeCommand') + ->with('df -TPk') + ->willReturn(file_get_contents(__DIR__ . '/../data/linux_df_tp_mount_points')); + + // Only the first of these five used to be reported. The other four were + // dropped by the mount point pattern, which silently removed the whole + // filesystem from the monitoring page rather than reporting it partially. + $disk1 = new Disk(); + $disk1->setDevice('/dev/sda1'); + $disk1->setFs('ext4'); + $disk1->setUsed(8889); + $disk1->setAvailable(46212); + $disk1->setPercent('16.13%'); + $disk1->setMount('/'); + + // A space in the mount point + $disk2 = new Disk(); + $disk2->setDevice('/dev/sdb1'); + $disk2->setFs('ext4'); + $disk2->setUsed(1954); + $disk2->setAvailable(7812); + $disk2->setPercent('20%'); + $disk2->setMount('/mnt/My Backup'); + + // Non-ASCII, which \w does not cover without the unicode flag + $disk3 = new Disk(); + $disk3->setDevice('/dev/sdc1'); + $disk3->setFs('ext4'); + $disk3->setUsed(3907); + $disk3->setAvailable(15625); + $disk3->setPercent('20%'); + $disk3->setMount('/mnt/Fotos-Übung'); + + // Parentheses + $disk4 = new Disk(); + $disk4->setDevice('/dev/sdd1'); + $disk4->setFs('btrfs'); + $disk4->setUsed(5860); + $disk4->setAvailable(23437); + $disk4->setPercent('20%'); + $disk4->setMount('/mnt/data (old)'); + + // `df` prints "-" for used and capacity when a filesystem does not report + // them; both are unread here, so the row is still usable + $disk5 = new Disk(); + $disk5->setDevice('storage:/vol'); + $disk5->setFs('nfs4'); + $disk5->setUsed(7813); + $disk5->setAvailable(31250); + $disk5->setPercent('20%'); + $disk5->setMount('/mnt/nfs'); + + $this->assertEquals([$disk1, $disk2, $disk3, $disk4, $disk5], $this->os->getDiskInfo()); + } + public function testSupported(): void { $this->assertTrue($this->os->supported()); }