Correct support of SI/IEC units for filesize

apps/provisioning_api/tests/Controller/UsersControllerTest.php

@@ -2142,7 +2142,7 @@ public function testEditUserAdminUserSelfEditChangeValidQuota(): void {
 		$targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
 		$targetUser->expects($this->once())
 			->method('setQuota')
-			->with('2.9 MB');
+			->with('2.9 MiB');
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -2230,7 +2230,7 @@ public function testEditUserAdminUserEditChangeValidQuota(): void {
 		$targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
 		$targetUser->expects($this->once())
 			->method('setQuota')
-			->with('2.9 MB');
+			->with('2.9 MiB');
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
@@ -2483,7 +2483,7 @@ public function testEditUserSubadminUserAccessible(): void {
 		$targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
 		$targetUser->expects($this->once())
 			->method('setQuota')
-			->with('2.9 MB');
+			->with('2.9 MiB');
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')

build/integration/dav_features/webdav-related.feature

@@ -196,7 +196,7 @@ Feature: webdav-related
 		Given using old dav path
 		And As an "admin"
 		And user "user0" exists
-		When user "user0" has a quota of "10 MB"
+		When user "user0" has a quota of "10 MiB"
 		Then as "user0" gets properties of folder "/" with
 		  |{DAV:}quota-available-bytes|
 		And the single response should contain a property "{DAV:}quota-available-bytes" with value "10485421"
@@ -207,7 +207,7 @@ Feature: webdav-related
 		And user "user0" exists
 		And user "user1" exists
 		And user "user0" has unlimited quota
-		And user "user1" has a quota of "10 MB"
+		And user "user1" has a quota of "10 MiB"
 		And As an "user1"
 		And user "user1" created a folder "/testquota"
 		And as "user1" creating a share with
@@ -243,7 +243,7 @@ Feature: webdav-related
 		Given using old dav path
 		And As an "admin"
 		And user "user0" exists
-		And user "user0" has a quota of "1 KB"
+		And user "user0" has a quota of "1 KiB"
 		And user "user0" adds a file of 93 bytes to "/prueba.txt"
 		When as "user0" gets properties of folder "/" with
 		  |{DAV:}quota-available-bytes|
@@ -254,7 +254,7 @@ Feature: webdav-related
 		And As an "admin"
 		And user "user0" exists
 		And user "user1" exists
-		And user "user1" has a quota of "1 KB"
+		And user "user1" has a quota of "1 KiB"
 		And user "user0" adds a file of 93 bytes to "/user0.txt"
 		And file "user0.txt" of user "user0" is shared with user "user1"
 		And user "user1" accepts last share

cypress/e2e/settings/users_modify.cy.ts

@@ -113,7 +113,8 @@ describe('Settings: Change user properties', function() {
 		cy.runOccCommand(`user:info --output=json '${user.userId}'`).then(($result) => {
 			expect($result.exitCode).to.equal(0)
 			const info = JSON.parse($result.stdout)
-			expect(info?.quota).to.equal('5 GB')
+			// TODO Frontend actually send 5 GiB (not 5 GB) and it's correctly reported by backend
+			expect(info?.quota).to.equal('5 GiB')
 		})
 	})
 

lib/private/User/User.php

@@ -597,8 +597,13 @@ public function setQuota($quota): void {
 				throw new InvalidArgumentException('Failed to set quota to invalid value ' . $quota);
 			}
 			$quota = Util::humanFileSize($bytesQuota);
+			$bytesOldQuota = Util::computerFileSize((string)$oldQuota);
+			$changed = $bytesOldQuota !== $bytesQuota;
+		} else {
+			$changed = $quota != $oldQuota;
 		}
-		if ($quota !== $oldQuota) {
+
+		if ($changed === true) {
 			$this->config->setUserValue($this->uid, 'files', 'quota', $quota);
 			$this->triggerChange('quota', $quota, $oldQuota);
 		}

lib/public/Util.php

@@ -321,33 +321,24 @@ public static function numericToNumber(string|float|int $number): int|float {
 	 * @param int|float $bytes file size in bytes
 	 * @return string a human readable file size
 	 * @since 4.0.0
+	 * @since 34.0.0 Return correct unit (IEC prefixes instead of SI prefixes)
+	 * @since 34.0.0 Handle negative size
 	 */
 	public static function humanFileSize(int|float $bytes): string {
-		if ($bytes < 0) {
-			return '?';
-		}
-		if ($bytes < 1024) {
-			return "$bytes B";
-		}
-		$bytes = round($bytes / 1024, 0);
-		if ($bytes < 1024) {
-			return "$bytes KB";
-		}
-		$bytes = round($bytes / 1024, 1);
-		if ($bytes < 1024) {
-			return "$bytes MB";
-		}
-		$bytes = round($bytes / 1024, 1);
-		if ($bytes < 1024) {
-			return "$bytes GB";
+		if ($bytes == 0) {
+			return '0 B';
 		}
-		$bytes = round($bytes / 1024, 1);
-		if ($bytes < 1024) {
-			return "$bytes TB";
+		$sign = '';
+		if ($bytes < 0) {
+			$sign = '−';
+			$bytes = abs($bytes);
 		}
 
-		$bytes = round($bytes / 1024, 1);
-		return "$bytes PB";
+		$base = max(log($bytes, 1024), 0);
+		$suffixes = ['B', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB'];
+		$precision = 1;
+
+		return $sign . round(pow(1024, $base - floor($base)), $precision) . ' ' . $suffixes[(int)floor($base)];
 	}
 
 	/**
@@ -357,31 +348,28 @@ public static function humanFileSize(int|float $bytes): string {
 	 * @param string $str file size in a fancy format
 	 * @return false|int|float a file size in bytes
 	 * @since 4.0.0
+	 * @since 34.0.0 Supports IEC and SI units instead of assuming IEC units
 	 */
 	public static function computerFileSize(string $str): false|int|float {
-		$str = strtolower($str);
 		if (is_numeric($str)) {
 			return Util::numericToNumber($str);
 		}
 
-		$bytes_array = [
-			'b' => 1,
-			'k' => 1024,
-			'kb' => 1024,
-			'mb' => 1024 * 1024,
-			'm' => 1024 * 1024,
-			'gb' => 1024 * 1024 * 1024,
-			'g' => 1024 * 1024 * 1024,
-			'tb' => 1024 * 1024 * 1024 * 1024,
-			't' => 1024 * 1024 * 1024 * 1024,
-			'pb' => 1024 * 1024 * 1024 * 1024 * 1024,
-			'p' => 1024 * 1024 * 1024 * 1024 * 1024,
+		$str = strtolower($str);
+		$supportedUnits = [
+			'' => 0,
+			'k' => 1,
+			'm' => 2,
+			'g' => 3,
+			't' => 4,
+			'p' => 5,
 		];
 
 		$bytes = (float)$str;
 
-		if (preg_match('#([kmgtp]?b?)$#si', $str, $matches) && isset($bytes_array[$matches[1]])) {
-			$bytes *= $bytes_array[$matches[1]];
+		if (preg_match('#([kmgtp]?)(i)?b$#si', $str, $matches) && isset($supportedUnits[$matches[1]])) {
+			$base = ($matches[2] ?? '') === 'i' ? 1024 : 1000;
+			$bytes *= $base ** $supportedUnits[$matches[1]];
 		} else {
 			return false;
 		}

tests/Core/Command/Log/FileTest.php

@@ -68,8 +68,10 @@ public static function changeRotateSizeProvider(): array {
 		return [
 			['42', 42],
 			['0', 0],
-			['1 kB', 1024],
-			['5MB', 5 * 1024 * 1024],
+			['1 kiB', 1024],
+			['1 kB', 1000],
+			['5MiB', 5 * 1024 * 1024],
+			['5MB', 5 * 1000 * 1000],
 		];
 	}
 
@@ -99,7 +101,7 @@ public function testGetConfiguration(): void {
 		$calls = [
 			['Log backend file: disabled'],
 			['Log file: /var/log/nextcloud.log'],
-			['Rotate at: 5 MB'],
+			['Rotate at: 5 MiB'],
 		];
 		$this->consoleOutput->expects($this->exactly(3))
 			->method('writeln')

tests/lib/User/UserTest.php

@@ -634,7 +634,7 @@ public function testSetEMailAddressNoChange(): void {
 		$user->setSystemEMailAddress('foo@bar.com');
 	}
 
-	public function testSetQuota(): void {
+	public function testSetQuotaSI(): void {
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->method('getBackendName')->willReturn('foo');
 
@@ -644,7 +644,7 @@ public function testSetQuota(): void {
 		$hook = function (IUser $user, string $feature, string $value) use ($test, &$hooksCalled): void {
 			$hooksCalled++;
 			$test->assertEquals('quota', $feature);
-			$test->assertEquals('23 TB', $value);
+			$test->assertEquals('20.9 TiB', $value);
 		};
 
 		$emitter = new PublicEmitter();
@@ -657,13 +657,43 @@ public function testSetQuota(): void {
 				'foo',
 				'files',
 				'quota',
-				'23 TB'
+				'20.9 TiB'
 			);
 
 		$user = new User('foo', $backend, $this->dispatcher, $emitter, $config);
 		$user->setQuota('23 TB');
 	}
 
+	public function testSetQuotaIEC(): void {
+		$backend = $this->createMock(\Test\Util\User\Dummy::class);
+		$backend->method('getBackendName')->willReturn('foo');
+
+		$test = $this;
+		$hooksCalled = 0;
+
+		$hook = function (IUser $user, string $feature, string $value) use ($test, &$hooksCalled): void {
+			$hooksCalled++;
+			$test->assertEquals('quota', $feature);
+			$test->assertEquals('23 TiB', $value);
+		};
+
+		$emitter = new PublicEmitter();
+		$emitter->listen('\OC\User', 'changeUser', $hook);
+
+		$config = $this->createMock(IConfig::class);
+		$config->expects($this->once())
+			->method('setUserValue')
+			->with(
+				'foo',
+				'files',
+				'quota',
+				'23 TiB'
+			);
+
+		$user = new User('foo', $backend, $this->dispatcher, $emitter, $config);
+		$user->setQuota('23 TiB');
+	}
+
 	public function testGetDefaultUnlimitedQuota(): void {
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->method('getBackendName')->willReturn('foo');
@@ -721,7 +751,7 @@ public function testGetDefaultUnlimitedQuotaForbidden(): void {
 			->willReturnMap($appValueMap);
 
 		$this->assertEquals('1 GB', $user->getQuota());
-		$this->assertEquals(1024 * 1024 * 1024, $user->getQuotaBytes());
+		$this->assertEquals(1000 ** 3, $user->getQuotaBytes());
 	}
 
 	public function testSetQuotaAddressNoChange(): void {

tests/lib/UtilTest.php

@@ -351,13 +351,16 @@ public function testHumanFileSize($expected, $input): void {
 
 	public static function humanFileSizeProvider(): array {
 		return [
+			['−2 KiB', -2000],
 			['0 B', 0],
-			['1 KB', 1024],
-			['9.5 MB', 10000000],
-			['1.3 GB', 1395864371],
-			['465.7 GB', 500000000000],
-			['454.7 TB', 500000000000000],
-			['444.1 PB', 500000000000000000],
+			['1 B', 0.123],
+			['1 KiB', 1024],
+			['9.5 MiB', 10000000],
+			['1.3 GiB', 1395864371],
+			['−1.3 GiB', -1395864371],
+			['465.7 GiB', 500000000000],
+			['454.7 TiB', 500000000000000],
+			['444.1 PiB', 500000000000000000],
 		];
 	}
 
@@ -369,11 +372,14 @@ public function testComputerFileSize($expected, $input): void {
 
 	public static function providesComputerFileSize(): array {
 		return [
-			[0.0, '0 B'],
-			[1024.0, '1 KB'],
-			[1395864371.0, '1.3 GB'],
-			[9961472.0, '9.5 MB'],
-			[500041567437.0, '465.7 GB'],
+			[0., '0 B'],
+			[5., '5 B'],
+			[1024., '1 KiB'],
+			[1000., '1 KB'],
+			[1395864371., '1.3 GiB'],
+			[9961472., '9.5 MiB'],
+			[500041567437., '465.7 GiB'],
+			[465785000000., '465.785 GB'],
 			[false, '12 GB etfrhzui']
 		];
 	}