From 914b3544065894e604728dcb0a5087fb8929359c Mon Sep 17 00:00:00 2001 From: Yitz Willroth Date: Thu, 3 Jul 2025 21:13:16 -0400 Subject: [PATCH 1/3] feat: binary file size validation support --- src/Illuminate/Validation/Rules/File.php | 192 ++++-- tests/Validation/ValidationFileRuleTest.php | 675 ++++++++++++++++++++ 2 files changed, 826 insertions(+), 41 deletions(-) diff --git a/src/Illuminate/Validation/Rules/File.php b/src/Illuminate/Validation/Rules/File.php index b7589853e9d2..a6fa026e964e 100644 --- a/src/Illuminate/Validation/Rules/File.php +++ b/src/Illuminate/Validation/Rules/File.php @@ -17,6 +17,16 @@ class File implements Rule, DataAwareRule, ValidatorAwareRule { use Conditionable, Macroable; + /** + * Binary units flag used for size validation. + */ + public const BINARY = 'binary'; + + /** + * International units flag used for size validation. + */ + public const INTERNATIONAL = 'international'; + /** * The MIME types that the given file should match. This array may also contain file extensions. * @@ -45,6 +55,11 @@ class File implements Rule, DataAwareRule, ValidatorAwareRule */ protected $maximumFileSize = null; + /** + * The units used for size validation. + */ + protected string $units = self::INTERNATIONAL; + /** * An array of custom rules that will be merged into the validation rules. * @@ -150,15 +165,32 @@ public function extensions($extensions) return $this; } + /** + * Set the units for size validation to binary. + */ + public function binary(): static + { + $this->units = self::BINARY; + return $this; + } + + /** + * Set the units for size validation to international. + */ + public function international(): static + { + $this->units = self::INTERNATIONAL; + return $this; + } + + + /** * Indicate that the uploaded file should be exactly a certain size in kilobytes. - * - * @param string|int $size - * @return $this */ - public function size($size) + public function size(string|int $size, ?string $units = null): static { - $this->minimumFileSize = $this->toKilobytes($size); + $this->minimumFileSize = $this->toKilobytes($size, $this->units($units)); $this->maximumFileSize = $this->minimumFileSize; return $this; @@ -166,68 +198,142 @@ public function size($size) /** * Indicate that the uploaded file should be between a minimum and maximum size in kilobytes. - * - * @param string|int $minSize - * @param string|int $maxSize - * @return $this */ - public function between($minSize, $maxSize) + public function between(string|int $minSize, string|int $maxSize, ?string $units = null): static { - $this->minimumFileSize = $this->toKilobytes($minSize); - $this->maximumFileSize = $this->toKilobytes($maxSize); + $this->minimumFileSize = $this->toKilobytes($minSize, $this->units($units)); + $this->maximumFileSize = $this->toKilobytes($maxSize, $this->units($units)); return $this; } /** * Indicate that the uploaded file should be no less than the given number of kilobytes. - * - * @param string|int $size - * @return $this */ - public function min($size) + public function min(string|int $size, ?string $units = null): static { - $this->minimumFileSize = $this->toKilobytes($size); + $this->minimumFileSize = $this->toKilobytes($size, $this->units($units)); return $this; } /** * Indicate that the uploaded file should be no more than the given number of kilobytes. - * - * @param string|int $size - * @return $this */ - public function max($size) + public function max(string|int $size, ?string $units = null): static { - $this->maximumFileSize = $this->toKilobytes($size); + $this->maximumFileSize = $this->toKilobytes($size, $this->units($units)); return $this; } + /** + * Resolve the units to use for size calculations. + */ + protected function units(?string $units = null): string + { + return $units ?? $this->units; + } + /** * Convert a potentially human-friendly file size to kilobytes. - * - * @param string|int $size - * @return mixed */ - protected function toKilobytes($size) + protected function toKilobytes(string|int $size, string $units): float|int { if (! is_string($size)) { return $size; } - $size = strtolower(trim($size)); + if (($value = $this->parseSize($size)) === false || $value < 0) { + throw new InvalidArgumentException('Invalid numeric value in file size.'); + } + + return $units === self::BINARY + ? $this->toBinaryKilobytes($size, $value) + : $this->toInternationalKilobytes($size, $value); + } + + /** + * Parse the numeric portion from a file size string. + */ + protected function parseSize($size): false|float + { + return filter_var( + is_numeric($size) + ? $size + : Str::before(trim($size), Str::match('/[a-zA-Z]/', trim($size))), + FILTER_VALIDATE_FLOAT, FILTER_FLAG_ALLOW_THOUSAND + ); + } + + /** + * Convert a human-friendly file size to kilobytes using the International System. + */ + protected function toInternationalKilobytes(string $size, float $value): float|int + { + return round( + $this->protectValueFromOverflow( + $this->prepareValueForPrecision($value), + ! is_numeric($size) + ? match (substr(strtolower(trim($size)), -2)) { + 'kb' => 1, + 'mb' => 1_000, + 'gb' => 1_000_000, + 'tb' => 1_000_000_000, + default => throw new InvalidArgumentException( + 'Invalid file size suffix. Valid suffixes are: KB, MB, GB, TB (case insensitive).' + ), + } : 1 + ) + ); + } + + /** + * Convert a human-friendly file size to kilobytes using the Binary System. + */ + protected function toBinaryKilobytes(string $size, float $value): float|int + { + return round( + $this->protectValueFromOverflow( + $this->prepareValueForPrecision($value), + ! is_numeric($size) + ? match (substr(strtolower(trim($size)), -2)) { + 'kb' => 1, + 'mb' => 1_024, + 'gb' => 1_048_576, + 'tb' => 1_073_741_824, + default => throw new InvalidArgumentException( + 'Invalid file size suffix. Valid suffixes are: KB, MB, GB, TB (case insensitive).' + ), + } : 1 + ) + ); + } - $value = floatval($size); + /** + * Converts whole numbers to integers for exact arithmetic while keeping + * fractional numbers as floats; also provides overflow protection by + * falling back to float arithmetic for values too large for integer range. + */ + protected function prepareValueForPrecision(float $value): float|int + { + return $value > PHP_INT_MAX + || $value < PHP_INT_MIN + || ((float) (int) $value) !== $value + ? $value + : (int) $value; + } - return round(match (true) { - Str::endsWith($size, 'kb') => $value * 1, - Str::endsWith($size, 'mb') => $value * 1_000, - Str::endsWith($size, 'gb') => $value * 1_000_000, - Str::endsWith($size, 'tb') => $value * 1_000_000_000, - default => throw new InvalidArgumentException('Invalid file size suffix.'), - }); + /** + * Protect calculations from integer overflow by switching to float arithmetic when necessary. + */ + protected function protectValueFromOverflow(float|int $value, int $multiplier): float|int + { + return $value > PHP_INT_MAX / $multiplier + || $value < PHP_INT_MIN / $multiplier + || is_float($value) + ? (float) $value * $multiplier + : (int) $value * $multiplier; } /** @@ -283,14 +389,18 @@ protected function buildValidationRules() $rules[] = 'extensions:'.implode(',', array_map(strtolower(...), $this->allowedExtensions)); } - $rules[] = match (true) { - is_null($this->minimumFileSize) && is_null($this->maximumFileSize) => null, - is_null($this->maximumFileSize) => "min:{$this->minimumFileSize}", - is_null($this->minimumFileSize) => "max:{$this->maximumFileSize}", - $this->minimumFileSize !== $this->maximumFileSize => "between:{$this->minimumFileSize},{$this->maximumFileSize}", - default => "size:{$this->minimumFileSize}", + $rule = match (true) { + $this->minimumFileSize === null && $this->maximumFileSize === null => null, + $this->maximumFileSize === null => "min:{$this->minimumFileSize}", + $this->minimumFileSize === null => "max:{$this->maximumFileSize}", + $this->minimumFileSize === $this->maximumFileSize => "size:{$this->minimumFileSize}", + default => "between:{$this->minimumFileSize},{$this->maximumFileSize}", }; + if ($rule) { + $rules[] = $rule; + } + return array_merge(array_filter($rules), $this->customRules); } diff --git a/tests/Validation/ValidationFileRuleTest.php b/tests/Validation/ValidationFileRuleTest.php index 0d95bb2a8e66..e78fd3905f88 100644 --- a/tests/Validation/ValidationFileRuleTest.php +++ b/tests/Validation/ValidationFileRuleTest.php @@ -12,6 +12,7 @@ use Illuminate\Validation\Rules\File; use Illuminate\Validation\ValidationServiceProvider; use Illuminate\Validation\Validator; +use InvalidArgumentException; use PHPUnit\Framework\TestCase; class ValidationFileRuleTest extends TestCase @@ -245,6 +246,40 @@ public function testSize() ); } + public function testSizeWithBinaryUnits(): void + { + $this->passes( + File::default()->size('1MB', File::BINARY), + UploadedFile::fake()->create('foo.txt', 1024) + ); + + $this->fails( + File::default()->size('1MB', File::BINARY), + [ + UploadedFile::fake()->create('foo.txt', 1023), + UploadedFile::fake()->create('foo.txt', 1025), + ], + ['validation.size.file'] + ); + } + + public function testSizeWithInternationalUnits(): void + { + $this->passes( + File::default()->size('1MB', File::INTERNATIONAL), + UploadedFile::fake()->create('foo.txt', 1000) + ); + + $this->fails( + File::default()->size('1MB', File::INTERNATIONAL), + [ + UploadedFile::fake()->create('foo.txt', 999), + UploadedFile::fake()->create('foo.txt', 1001), + ], + ['validation.size.file'] + ); + } + public function testBetween() { $this->fails( @@ -267,6 +302,48 @@ public function testBetween() ); } + public function testBetweenWithBinaryUnits(): void + { + $this->passes( + File::default()->between('1MB', '2MB', File::BINARY), + [ + UploadedFile::fake()->create('foo.txt', 1024), + UploadedFile::fake()->create('foo.txt', 1500), + UploadedFile::fake()->create('foo.txt', 2048), + ] + ); + + $this->fails( + File::default()->between('1MB', '2MB', File::BINARY), + [ + UploadedFile::fake()->create('foo.txt', 1023), + UploadedFile::fake()->create('foo.txt', 2049), + ], + ['validation.between.file'] + ); + } + + public function testBetweenWithInternationalUnits(): void + { + $this->passes( + File::default()->between('1MB', '2MB', File::INTERNATIONAL), + [ + UploadedFile::fake()->create('foo.txt', 1000), + UploadedFile::fake()->create('foo.txt', 1500), + UploadedFile::fake()->create('foo.txt', 2000), + ] + ); + + $this->fails( + File::default()->between('1MB', '2MB', File::INTERNATIONAL), + [ + UploadedFile::fake()->create('foo.txt', 999), + UploadedFile::fake()->create('foo.txt', 2001), + ], + ['validation.between.file'] + ); + } + public function testMin() { $this->fails( @@ -303,6 +380,42 @@ public function testMinWithHumanReadableSize() ); } + public function testMinWithBinaryUnits(): void + { + $this->passes( + File::default()->min('1MB', File::BINARY), + [ + UploadedFile::fake()->create('foo.txt', 1024), + UploadedFile::fake()->create('foo.txt', 1025), + UploadedFile::fake()->create('foo.txt', 2048), + ] + ); + + $this->fails( + File::default()->min('1MB', File::BINARY), + UploadedFile::fake()->create('foo.txt', 1023), + ['validation.min.file'] + ); + } + + public function testMinWithInternationalUnits(): void + { + $this->passes( + File::default()->min('1MB', File::INTERNATIONAL), + [ + UploadedFile::fake()->create('foo.txt', 1000), + UploadedFile::fake()->create('foo.txt', 1001), + UploadedFile::fake()->create('foo.txt', 2000), + ] + ); + + $this->fails( + File::default()->min('1MB', File::INTERNATIONAL), + UploadedFile::fake()->create('foo.txt', 999), + ['validation.min.file'] + ); + } + public function testMax() { $this->fails( @@ -357,6 +470,42 @@ public function testMaxWithHumanReadableSizeAndMultipleValue() ); } + public function testMaxWithBinaryUnits(): void + { + $this->passes( + File::default()->max('1MB', File::BINARY), + [ + UploadedFile::fake()->create('foo.txt', 1024), + UploadedFile::fake()->create('foo.txt', 1023), + UploadedFile::fake()->create('foo.txt', 512), + ] + ); + + $this->fails( + File::default()->max('1MB', File::BINARY), + UploadedFile::fake()->create('foo.txt', 1025), + ['validation.max.file'] + ); + } + + public function testMaxWithInternationalUnits(): void + { + $this->passes( + File::default()->max('1MB', File::INTERNATIONAL), + [ + UploadedFile::fake()->create('foo.txt', 1000), + UploadedFile::fake()->create('foo.txt', 999), + UploadedFile::fake()->create('foo.txt', 500), + ] + ); + + $this->fails( + File::default()->max('1MB', File::INTERNATIONAL), + UploadedFile::fake()->create('foo.txt', 1001), + ['validation.max.file'] + ); + } + public function testMacro() { File::macro('toDocument', function () { @@ -439,6 +588,530 @@ public function testFileSizeConversionWithDifferentUnits() File::image()->size('10xyz'); } + public function testFileConstants(): void + { + $this->assertEquals('binary', File::BINARY); + $this->assertEquals('international', File::INTERNATIONAL); + } + + + public function testGlobalBinaryPrecedence(): void + { + $file1010 = UploadedFile::fake()->create('test.txt', 1010); + + $rule = File::default()->binary(); + + $this->passes( + $rule->max('1MB'), + $file1010 + ); + + $this->fails( + $rule->min('1MB'), + $file1010, + ['validation.size.file'] // when min=max, validation.size.file instead of validation.max.file + ); + } + + public function testGlobalInternationalPrecedence(): void + { + $file1010 = UploadedFile::fake()->create('test.txt', 1010); + + $rule = File::default()->international(); + + $this->passes( + $rule->min('1MB'), + $file1010 + ); + + $this->fails( + $rule->max('1MB'), + $file1010, + ['validation.size.file'] // when min=max, validation.size.file instead of validation.max.file + ); + } + + public function testExplicitParameterOverridesGlobalSetting(): void + { + $file1010 = UploadedFile::fake()->create('test.txt', 1010); + + $this->fails( + File::default()->binary()->max('1MB', File::INTERNATIONAL), + $file1010, + ['validation.max.file'] + ); + + $this->passes( + File::default()->international()->max('1MB', File::BINARY), + $file1010 + ); + } + + public function testDefaultsToInternationalWhenNoGlobalSetting(): void + { + $file1010 = UploadedFile::fake()->create('test.txt', 1010); + + $this->fails( + File::default()->max('1MB'), + $file1010, + ['validation.max.file'] + ); + } + + public function testBinaryVsInternationalDifference(): void + { + $file1010 = UploadedFile::fake()->create('boundary.txt', 1010); + + $this->passes( + File::default()->max('1MB', File::BINARY), + $file1010 + ); + + $this->fails( + File::default()->max('1MB', File::INTERNATIONAL), + $file1010, + ['validation.max.file']); + } + + public function testNumericSizesWorkWithoutUnits(): void + { + $file1000 = UploadedFile::fake()->create('numeric.txt', 1000); + + $this->passes(File::default()->max(1000), $file1000); + $this->passes(File::default()->binary()->max(1000), $file1000); + $this->passes(File::default()->international()->max(1000), $file1000); + } + + public function testDecimalSizes(): void + { + $file512 = UploadedFile::fake()->create('half.txt', 512); + $file500 = UploadedFile::fake()->create('half.txt', 500); + + $this->passes( + File::default()->size('0.5MB', File::BINARY), + $file512 + ); + + $this->fails(File::default()->size('0.5MB', File::BINARY), + $file500, + ['validation.size.file'] + ); + + $this->passes( + File::default()->size('0.5MB', File::INTERNATIONAL), + $file500 + ); + + $this->fails( + File::default()->size('0.5MB', File::INTERNATIONAL), + $file512, + ['validation.size.file'] + ); + } + + public function testLargerUnits(): void + { + $file1048576 = UploadedFile::fake()->create('big.txt', 1048576); + $file1000000 = UploadedFile::fake()->create('big.txt', 1000000); + + $this->passes( + File::default()->size('1GB', File::BINARY), + $file1048576 + ); + + $this->passes( + File::default()->size('1GB', File::INTERNATIONAL), + $file1000000 + ); + } + + public function testBinaryIntegerPrecisionForLargeFileSizes(): void + { + $file999999 = UploadedFile::fake()->create('large999999.txt', 999999); + + $this->passes( + File::default()->binary()->max('1000000KB'), + $file999999 + ); + + $this->fails( + File::default()->binary()->max('999998KB'), + $file999999, + ['validation.max.file'] + ); + } + + public function testInternationalIntegerPrecisionForLargeFileSizes(): void + { + $file999999 = UploadedFile::fake()->create('large999999.txt', 999999); + + $this->passes( + File::default()->international()->max('1000000KB'), + $file999999 + ); + + $this->fails( + File::default()->international()->max('999998KB'), + $file999999, + ['validation.max.file'] + ); + } + + public function testFloatPrecisionForFractionalSizes(): void + { + $file512 = UploadedFile::fake()->create('fractional512.txt', 512); + $file500 = UploadedFile::fake()->create('fractional500.txt', 500); + + $this->passes( + File::default()->binary()->size('0.5MB'), + $file512 + ); + + $this->passes( + File::default()->international()->size('0.5MB'), + $file500 + ); + + $this->fails( + File::default()->binary()->size('0.5MB'), + $file500, + ['validation.size.file'] + ); + + $this->fails( + File::default()->international()->size('0.5MB'), + $file512, + ['validation.size.file'] + ); + } + + public function testBinaryVsInternationalCalculationAccuracy(): void + { + $file1010 = UploadedFile::fake()->create('boundary1010.txt', 1010); + + $this->passes( + File::default()->binary()->max('1MB'), + $file1010 + ); + + $this->fails( + File::default()->international()->max('1MB'), + $file1010, + ['validation.max.file'] + ); + + $file900 = UploadedFile::fake()->create('small900.txt', 900); + + $this->passes( + File::default()->binary()->max('1MB'), + $file900 + ); + + $this->passes( + File::default()->international()->max('1MB'), + $file900 + ); + } + + public function testBinaryLargeFileSizePrecision(): void + { + $file500000 = UploadedFile::fake()->create('huge500000.txt', 500000); + + $this->passes( + File::default()->binary()->between('400MB', '600MB'), + $file500000 + ); + + $this->passes( + File::default()->binary()->max('1GB'), + $file500000 + ); + + $this->fails( + File::default()->binary()->max('488MB'), + $file500000, + ['validation.max.file'] + ); + } + + public function testInternationalLargeFileSizePrecision(): void + { + $file500000 = UploadedFile::fake()->create('huge500000.txt', 500000); + + $this->passes( + File::default()->international()->between('400MB', '600MB'), + $file500000 + ); + + $this->passes( + File::default()->international()->max('1GB'), + $file500000 + ); + + $this->fails( + File::default()->international()->max('499MB'), + $file500000, + ['validation.max.file'] + ); + } + + public function testOverflowProtectionForLargeIntegerValues(): void + { + $fileLarge = UploadedFile::fake()->create('overflow.txt', 2000000000); + + $this->passes( + File::default()->binary()->max('2000000MB'), + $fileLarge + ); + + $this->passes( + File::default()->international()->max('2000000MB'), + $fileLarge + ); + } + + public function testOverflowProtectionWithFractionalValues(): void + { + $file1536 = UploadedFile::fake()->create('fractional.txt', 1536); + + $this->passes( + File::default()->binary()->size('1.5MB'), + $file1536 + ); + + $file1500 = UploadedFile::fake()->create('fractional.txt', 1500); + + $this->passes( + File::default()->international()->size('1.5MB'), + $file1500 + ); + } + + public function testCaseInsensitiveSuffixes(): void + { + $file1024 = UploadedFile::fake()->create('case.txt', 1024); + + $this->passes(File::default()->binary()->size('1MB'), $file1024); + $this->passes(File::default()->binary()->size('1mb'), $file1024); + $this->passes(File::default()->binary()->size('1Mb'), $file1024); + $this->passes(File::default()->binary()->size('1mB'), $file1024); + } + + public function testInvalidSizeSuffixThrowsException(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid file size suffix.'); + + File::default()->max('5xyz'); + } + + public function testZeroAndVerySmallFileSizes(): void + { + $fileZero = UploadedFile::fake()->create('zero.txt', 0); + $fileOne = UploadedFile::fake()->create('tiny.txt', 1); + + $this->passes( + File::default()->min('0KB'), + $fileZero + ); + + $this->passes( + File::default()->size('1KB'), + $fileOne + ); + + $this->passes( + File::default()->binary()->max('0.001MB'), + $fileOne + ); + } + + public function testWhitespaceHandlingInFileSizes(): void + { + $file2048 = UploadedFile::fake()->create('whitespace.txt', 2048); + $file2000 = UploadedFile::fake()->create('whitespace.txt', 2000); + + $this->passes( + File::default()->binary()->size(' 2MB '), + $file2048 + ); + + $this->passes( + File::default()->international()->size(' 2MB '), + $file2000 + ); + + $this->passes( + File::default()->binary()->size('2 MB'), + $file2048 + ); + } + + public function testCommaSeparatedNumberParsing(): void + { + $file1024 = UploadedFile::fake()->create('comma.txt', 1024); + $file10240 = UploadedFile::fake()->create('large.txt', 10240); + + $this->passes( + File::default()->binary()->size('1,024KB'), + $file1024 + ); + + $this->passes( + File::default()->binary()->size('10,240KB'), + $file10240 + ); + + $this->passes( + File::default()->international()->size('1,024KB'), + $file1024 + ); + } + + public function testNegativeFileSizeThrowsException(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid numeric value in file size.'); + + File::default()->max('-5MB'); + } + + public function testBinaryFluentChaining(): void + { + $file1024 = UploadedFile::fake()->create('chain.txt', 1024); + + $rule = File::default()->binary() + ->types(['txt']) + ->min('1MB') + ->max('2MB'); + + $this->passes($rule, $file1024); + } + + public function testInternationalFluentChaining(): void + { + $file1000 = UploadedFile::fake()->create('chain.txt', 1000); + + $rule = File::default()->international() + ->types(['txt']) + ->min('1MB') + ->max('2MB'); + + $this->passes($rule, $file1000); + } + + public function testMixedUnitsInSameRule(): void + { + $file1500 = UploadedFile::fake()->create('mixed.txt', 1500); + + $rule = File::default()->binary() + ->min('1MB') + ->max('2MB', File::INTERNATIONAL); + + $this->passes($rule, $file1500); + } + + public function testComplexUnitsScenarioWithMultipleConstraints(): void + { + $file1500 = UploadedFile::fake()->create('complex.txt', 1500); + + $rule = File::default()->binary() + ->types(['txt']) + ->min('1MB', File::INTERNATIONAL) + ->max('1.4MB', File::BINARY); + + $this->fails($rule, $file1500, ['validation.between.file']); + } + + public function testUnitsMethodsReturnNewInstances(): void + { + $binary1 = File::default()->binary(); + $binary2 = File::default()->binary(); + $international1 = File::default()->international(); + + $this->assertNotSame($binary1, $binary2); + $this->assertNotSame($binary1, $international1); + } + + public function testUnitsBackwardsCompatibility(): void + { + $file1000 = UploadedFile::fake()->create('compat.txt', 1000); + + $this->passes( + File::types(['txt'])->max('1MB'), + $file1000 + ); + + $this->passes( + File::default()->between(500, 1500), + $file1000 + ); + + $this->passes( + File::default()->size(1000), + $file1000 + ); + } + + public function testBinaryFluentMethod(): void + { + $file1024 = UploadedFile::fake()->create('binary.txt', 1024); + + // Create instance then call binary() method + $rule = File::default() + ->types(['txt']) + ->binary() + ->size('1MB'); + + $this->passes($rule, $file1024); + } + + public function testInternationalFluentMethod(): void + { + $file1000 = UploadedFile::fake()->create('international.txt', 1000); + + // Create instance then call international() method + $rule = File::default() + ->types(['txt']) + ->international() + ->size('1MB'); + + $this->passes($rule, $file1000); + } + + public function testUnitsMethodChaining(): void + { + $file1024 = UploadedFile::fake()->create('chaining.txt', 1024); + + // Test switching between binary and international on same instance + $rule = File::default() + ->types(['txt']) + ->international() // Start with international + ->binary() // Switch to binary + ->size('1MB'); // 1MB binary = 1024KB + + $this->passes($rule, $file1024); + } + + public function testInstanceMethodsReturnSameObject(): void + { + $originalRule = File::default()->types(['txt']); + + // Instance methods should return the same object + $binaryRule = $originalRule->binary(); + $internationalRule = $originalRule->international(); + + $this->assertSame($originalRule, $binaryRule); + $this->assertSame($originalRule, $internationalRule); + + // Creating new instances should return different objects + $newBinary = File::default()->binary(); + $newInternational = File::default()->international(); + + $this->assertNotSame($originalRule, $newBinary); + $this->assertNotSame($originalRule, $newInternational); + } + protected function setUp(): void { $container = Container::getInstance(); @@ -461,5 +1134,7 @@ protected function tearDown(): void Facade::clearResolvedInstances(); Facade::setFacadeApplication(null); + + File::$defaultCallback = null; } } From a032838128ca27b89540e1d052d87f32ee0ae20b Mon Sep 17 00:00:00 2001 From: Yitz Willroth Date: Mon, 7 Jul 2025 12:21:11 -0400 Subject: [PATCH 2/3] refactor to resolve units from suffix --- src/Illuminate/Validation/Rules/File.php | 137 ++--- tests/Validation/ValidationFileRuleTest.php | 543 +++++++------------- 2 files changed, 256 insertions(+), 424 deletions(-) diff --git a/src/Illuminate/Validation/Rules/File.php b/src/Illuminate/Validation/Rules/File.php index a6fa026e964e..c524e32a58ce 100644 --- a/src/Illuminate/Validation/Rules/File.php +++ b/src/Illuminate/Validation/Rules/File.php @@ -13,19 +13,19 @@ use Illuminate\Support\Traits\Macroable; use InvalidArgumentException; -class File implements Rule, DataAwareRule, ValidatorAwareRule +class File implements DataAwareRule, Rule, ValidatorAwareRule { use Conditionable, Macroable; /** * Binary units flag used for size validation. */ - public const BINARY = 'binary'; + protected const BINARY = 'binary'; /** * International units flag used for size validation. */ - public const INTERNATIONAL = 'international'; + protected const INTERNATIONAL = 'international'; /** * The MIME types that the given file should match. This array may also contain file extensions. @@ -42,14 +42,14 @@ class File implements Rule, DataAwareRule, ValidatorAwareRule protected $allowedExtensions = []; /** - * The minimum size in kilobytes that the file can be. + * The minimum file size that the file can be. * * @var null|int */ protected $minimumFileSize = null; /** - * The maximum size in kilobytes that the file can be. + * The maximum file size that the file can be. * * @var null|int */ @@ -127,7 +127,7 @@ public static function default() ? call_user_func(static::$defaultCallback) : static::$defaultCallback; - return $file instanceof Rule ? $file : new self(); + return $file instanceof Rule ? $file : new self; } /** @@ -149,7 +149,7 @@ public static function image($allowSvg = false) */ public static function types($mimetypes) { - return tap(new static(), fn ($file) => $file->allowedMimetypes = (array) $mimetypes); + return tap(new static, fn ($file) => $file->allowedMimetypes = (array) $mimetypes); } /** @@ -166,79 +166,73 @@ public function extensions($extensions) } /** - * Set the units for size validation to binary. + * Set the units for size validation to binary (1024-based). */ public function binary(): static { $this->units = self::BINARY; + return $this; } /** - * Set the units for size validation to international. + * Set the units for size validation to international (1000-based). */ public function international(): static { $this->units = self::INTERNATIONAL; + return $this; } - - /** - * Indicate that the uploaded file should be exactly a certain size in kilobytes. + * Indicate that the uploaded file should be exactly a certain size. */ - public function size(string|int $size, ?string $units = null): static + public function size(string|int $size): static { - $this->minimumFileSize = $this->toKilobytes($size, $this->units($units)); + $this->minimumFileSize = $this->toKilobytes($size); $this->maximumFileSize = $this->minimumFileSize; return $this; } /** - * Indicate that the uploaded file should be between a minimum and maximum size in kilobytes. + * Indicate that the uploaded file should be between a minimum and maximum size. */ - public function between(string|int $minSize, string|int $maxSize, ?string $units = null): static + public function between(string|int $minSize, string|int $maxSize): static { - $this->minimumFileSize = $this->toKilobytes($minSize, $this->units($units)); - $this->maximumFileSize = $this->toKilobytes($maxSize, $this->units($units)); + $this->minimumFileSize = $this->toKilobytes($minSize); + $this->maximumFileSize = $this->toKilobytes($maxSize); return $this; } /** - * Indicate that the uploaded file should be no less than the given number of kilobytes. + * Indicate that the uploaded file should be no less than the given size. */ - public function min(string|int $size, ?string $units = null): static + public function min(string|int $size): static { - $this->minimumFileSize = $this->toKilobytes($size, $this->units($units)); + $this->minimumFileSize = $this->toKilobytes($size); return $this; } /** - * Indicate that the uploaded file should be no more than the given number of kilobytes. + * Indicate that the uploaded file should be no more than the given size. */ - public function max(string|int $size, ?string $units = null): static + public function max(string|int $size): static { - $this->maximumFileSize = $this->toKilobytes($size, $this->units($units)); + $this->maximumFileSize = $this->toKilobytes($size); return $this; } - /** - * Resolve the units to use for size calculations. - */ - protected function units(?string $units = null): string - { - return $units ?? $this->units; - } - /** * Convert a potentially human-friendly file size to kilobytes. + * + * Supports suffix detection with precedence over instance settings. */ - protected function toKilobytes(string|int $size, string $units): float|int + protected function toKilobytes(string|int $size): float|int { if (! is_string($size)) { return $size; @@ -246,9 +240,9 @@ protected function toKilobytes(string|int $size, string $units): float|int if (($value = $this->parseSize($size)) === false || $value < 0) { throw new InvalidArgumentException('Invalid numeric value in file size.'); - } + } - return $units === self::BINARY + return $this->detectUnits($size) === self::BINARY ? $this->toBinaryKilobytes($size, $value) : $this->toInternationalKilobytes($size, $value); } @@ -266,6 +260,19 @@ protected function parseSize($size): false|float ); } + /** + * Detect the suffix and determine appropriate units from a file size string. + */ + protected function detectUnits(string $size): string + { + return match (true) { + is_numeric($size) => $this->units, + in_array(strtolower(substr(trim($size), -3)), ['kib', 'mib', 'gib', 'tib']) => self::BINARY, + in_array(strtolower(substr(trim($size), -2)), ['kb', 'mb', 'gb', 'tb']) => self::INTERNATIONAL, + default => $this->units, + }; + } + /** * Convert a human-friendly file size to kilobytes using the International System. */ @@ -275,17 +282,24 @@ protected function toInternationalKilobytes(string $size, float $value): float|i $this->protectValueFromOverflow( $this->prepareValueForPrecision($value), ! is_numeric($size) - ? match (substr(strtolower(trim($size)), -2)) { - 'kb' => 1, - 'mb' => 1_000, - 'gb' => 1_000_000, - 'tb' => 1_000_000_000, - default => throw new InvalidArgumentException( - 'Invalid file size suffix. Valid suffixes are: KB, MB, GB, TB (case insensitive).' - ), - } : 1 - ) - ); + ? $this->getInternationalMultiplier(strtolower(trim($size))) + : 1 + ) + ); + } + + /** + * Get the international multiplier for a given size string. + */ + protected function getInternationalMultiplier(string $size): int + { + return match (substr($size, -2)) { + 'kb' => 1, + 'mb' => 1_000, + 'gb' => 1_000_000, + 'tb' => 1_000_000_000, + default => throw new InvalidArgumentException('Invalid file size suffix.'), + }; } /** @@ -297,17 +311,24 @@ protected function toBinaryKilobytes(string $size, float $value): float|int $this->protectValueFromOverflow( $this->prepareValueForPrecision($value), ! is_numeric($size) - ? match (substr(strtolower(trim($size)), -2)) { - 'kb' => 1, - 'mb' => 1_024, - 'gb' => 1_048_576, - 'tb' => 1_073_741_824, - default => throw new InvalidArgumentException( - 'Invalid file size suffix. Valid suffixes are: KB, MB, GB, TB (case insensitive).' - ), - } : 1 - ) - ); + ? $this->getBinaryMultiplier(strtolower(trim($size))) + : 1 + ) + ); + } + + /** + * Get the binary multiplier for a given size string. + */ + protected function getBinaryMultiplier(string $size): int + { + return match (substr($size, -3)) { + 'kib' => 1, + 'mib' => 1_024, + 'gib' => 1_048_576, + 'tib' => 1_073_741_824, + default => throw new InvalidArgumentException('Invalid file size suffix.'), + }; } /** @@ -329,7 +350,7 @@ protected function prepareValueForPrecision(float $value): float|int */ protected function protectValueFromOverflow(float|int $value, int $multiplier): float|int { - return $value > PHP_INT_MAX / $multiplier + return $value > PHP_INT_MAX / $multiplier || $value < PHP_INT_MIN / $multiplier || is_float($value) ? (float) $value * $multiplier diff --git a/tests/Validation/ValidationFileRuleTest.php b/tests/Validation/ValidationFileRuleTest.php index e78fd3905f88..6c26421c445b 100644 --- a/tests/Validation/ValidationFileRuleTest.php +++ b/tests/Validation/ValidationFileRuleTest.php @@ -17,7 +17,7 @@ class ValidationFileRuleTest extends TestCase { - public function testBasic() + public function test_basic() { $this->fails( File::default(), @@ -63,7 +63,7 @@ protected function passes($rule, $values) $this->assertValidationRules($rule, $values, true, []); } - public function testSingleMimetype() + public function test_single_mimetype() { $this->fails( File::types('text/plain'), @@ -77,7 +77,7 @@ public function testSingleMimetype() ); } - public function testMultipleMimeTypes() + public function test_multiple_mime_types() { $this->fails( File::types(['text/plain', 'image/jpeg']), @@ -91,7 +91,7 @@ public function testMultipleMimeTypes() ); } - public function testSingleMime() + public function test_single_mime() { $this->fails( File::types('txt'), @@ -105,7 +105,7 @@ public function testSingleMime() ); } - public function testMultipleMimes() + public function test_multiple_mimes() { $this->fails( File::types(['png', 'jpg', 'jpeg', 'svg']), @@ -122,7 +122,7 @@ public function testMultipleMimes() ); } - public function testMixOfMimetypesAndMimes() + public function test_mix_of_mimetypes_and_mimes() { $this->fails( File::types(['png', 'image/png']), @@ -136,7 +136,7 @@ public function testMixOfMimetypesAndMimes() ); } - public function testSingleExtension() + public function test_single_extension() { $this->fails( File::default()->extensions('png'), @@ -162,7 +162,7 @@ public function testSingleExtension() ); } - public function testMultipleExtensions() + public function test_multiple_extensions() { $this->fails( File::default()->extensions(['png', 'jpeg', 'jpg']), @@ -182,7 +182,7 @@ public function testMultipleExtensions() ); } - public function testImage() + public function test_image() { $this->fails( File::image(), @@ -196,7 +196,7 @@ public function testImage() ); } - public function testImageFailsOnSvgByDefault() + public function test_image_fails_on_svg_by_default() { $maliciousSvgFileWithXSS = UploadedFile::fake()->createWithContent( name: 'foo.svg', @@ -229,7 +229,7 @@ public function testImageFailsOnSvgByDefault() ); } - public function testSize() + public function test_size() { $this->fails( File::default()->size(1024), @@ -246,41 +246,7 @@ public function testSize() ); } - public function testSizeWithBinaryUnits(): void - { - $this->passes( - File::default()->size('1MB', File::BINARY), - UploadedFile::fake()->create('foo.txt', 1024) - ); - - $this->fails( - File::default()->size('1MB', File::BINARY), - [ - UploadedFile::fake()->create('foo.txt', 1023), - UploadedFile::fake()->create('foo.txt', 1025), - ], - ['validation.size.file'] - ); - } - - public function testSizeWithInternationalUnits(): void - { - $this->passes( - File::default()->size('1MB', File::INTERNATIONAL), - UploadedFile::fake()->create('foo.txt', 1000) - ); - - $this->fails( - File::default()->size('1MB', File::INTERNATIONAL), - [ - UploadedFile::fake()->create('foo.txt', 999), - UploadedFile::fake()->create('foo.txt', 1001), - ], - ['validation.size.file'] - ); - } - - public function testBetween() + public function test_between() { $this->fails( File::default()->between(1024, 2048), @@ -302,49 +268,7 @@ public function testBetween() ); } - public function testBetweenWithBinaryUnits(): void - { - $this->passes( - File::default()->between('1MB', '2MB', File::BINARY), - [ - UploadedFile::fake()->create('foo.txt', 1024), - UploadedFile::fake()->create('foo.txt', 1500), - UploadedFile::fake()->create('foo.txt', 2048), - ] - ); - - $this->fails( - File::default()->between('1MB', '2MB', File::BINARY), - [ - UploadedFile::fake()->create('foo.txt', 1023), - UploadedFile::fake()->create('foo.txt', 2049), - ], - ['validation.between.file'] - ); - } - - public function testBetweenWithInternationalUnits(): void - { - $this->passes( - File::default()->between('1MB', '2MB', File::INTERNATIONAL), - [ - UploadedFile::fake()->create('foo.txt', 1000), - UploadedFile::fake()->create('foo.txt', 1500), - UploadedFile::fake()->create('foo.txt', 2000), - ] - ); - - $this->fails( - File::default()->between('1MB', '2MB', File::INTERNATIONAL), - [ - UploadedFile::fake()->create('foo.txt', 999), - UploadedFile::fake()->create('foo.txt', 2001), - ], - ['validation.between.file'] - ); - } - - public function testMin() + public function test_min() { $this->fails( File::default()->min(1024), @@ -362,7 +286,7 @@ public function testMin() ); } - public function testMinWithHumanReadableSize() + public function test_min_with_human_readable_size() { $this->fails( File::default()->min('1024kb'), @@ -380,43 +304,7 @@ public function testMinWithHumanReadableSize() ); } - public function testMinWithBinaryUnits(): void - { - $this->passes( - File::default()->min('1MB', File::BINARY), - [ - UploadedFile::fake()->create('foo.txt', 1024), - UploadedFile::fake()->create('foo.txt', 1025), - UploadedFile::fake()->create('foo.txt', 2048), - ] - ); - - $this->fails( - File::default()->min('1MB', File::BINARY), - UploadedFile::fake()->create('foo.txt', 1023), - ['validation.min.file'] - ); - } - - public function testMinWithInternationalUnits(): void - { - $this->passes( - File::default()->min('1MB', File::INTERNATIONAL), - [ - UploadedFile::fake()->create('foo.txt', 1000), - UploadedFile::fake()->create('foo.txt', 1001), - UploadedFile::fake()->create('foo.txt', 2000), - ] - ); - - $this->fails( - File::default()->min('1MB', File::INTERNATIONAL), - UploadedFile::fake()->create('foo.txt', 999), - ['validation.min.file'] - ); - } - - public function testMax() + public function test_max() { $this->fails( File::default()->max(1024), @@ -434,7 +322,7 @@ public function testMax() ); } - public function testMaxWithHumanReadableSize() + public function test_max_with_human_readable_size() { $this->fails( File::default()->max('1024kb'), @@ -452,7 +340,7 @@ public function testMaxWithHumanReadableSize() ); } - public function testMaxWithHumanReadableSizeAndMultipleValue() + public function test_max_with_human_readable_size_and_multiple_value() { $this->fails( File::default()->max('1mb'), @@ -470,43 +358,7 @@ public function testMaxWithHumanReadableSizeAndMultipleValue() ); } - public function testMaxWithBinaryUnits(): void - { - $this->passes( - File::default()->max('1MB', File::BINARY), - [ - UploadedFile::fake()->create('foo.txt', 1024), - UploadedFile::fake()->create('foo.txt', 1023), - UploadedFile::fake()->create('foo.txt', 512), - ] - ); - - $this->fails( - File::default()->max('1MB', File::BINARY), - UploadedFile::fake()->create('foo.txt', 1025), - ['validation.max.file'] - ); - } - - public function testMaxWithInternationalUnits(): void - { - $this->passes( - File::default()->max('1MB', File::INTERNATIONAL), - [ - UploadedFile::fake()->create('foo.txt', 1000), - UploadedFile::fake()->create('foo.txt', 999), - UploadedFile::fake()->create('foo.txt', 500), - ] - ); - - $this->fails( - File::default()->max('1MB', File::INTERNATIONAL), - UploadedFile::fake()->create('foo.txt', 1001), - ['validation.max.file'] - ); - } - - public function testMacro() + public function test_macro() { File::macro('toDocument', function () { return static::default()->rules('mimes:txt,csv'); @@ -527,7 +379,7 @@ public function testMacro() ); } - public function testItUsesTheCorrectValidationMessageForFile(): void + public function test_it_uses_the_correct_validation_message_for_file(): void { file_put_contents($path = __DIR__.'/test.json', 'this-is-a-test'); @@ -542,7 +394,7 @@ public function testItUsesTheCorrectValidationMessageForFile(): void unlink($path); } - public function testItCanSetDefaultUsing() + public function test_it_can_set_default_using() { $this->assertInstanceOf(File::class, File::default()); @@ -567,7 +419,7 @@ public function testItCanSetDefaultUsing() ); } - public function testFileSizeConversionWithDifferentUnits() + public function test_file_size_conversion_with_different_units() { $this->passes( File::image()->size('5MB'), @@ -588,32 +440,25 @@ public function testFileSizeConversionWithDifferentUnits() File::image()->size('10xyz'); } - public function testFileConstants(): void - { - $this->assertEquals('binary', File::BINARY); - $this->assertEquals('international', File::INTERNATIONAL); - } - - - public function testGlobalBinaryPrecedence(): void + public function test_global_binary_precedence(): void { $file1010 = UploadedFile::fake()->create('test.txt', 1010); $rule = File::default()->binary(); $this->passes( - $rule->max('1MB'), + $rule->max(1024), $file1010 ); $this->fails( - $rule->min('1MB'), + $rule->max(1000), $file1010, - ['validation.size.file'] // when min=max, validation.size.file instead of validation.max.file + ['validation.max.file'] ); } - public function testGlobalInternationalPrecedence(): void + public function test_global_international_precedence(): void { $file1010 = UploadedFile::fake()->create('test.txt', 1010); @@ -627,27 +472,11 @@ public function testGlobalInternationalPrecedence(): void $this->fails( $rule->max('1MB'), $file1010, - ['validation.size.file'] // when min=max, validation.size.file instead of validation.max.file - ); - } - - public function testExplicitParameterOverridesGlobalSetting(): void - { - $file1010 = UploadedFile::fake()->create('test.txt', 1010); - - $this->fails( - File::default()->binary()->max('1MB', File::INTERNATIONAL), - $file1010, - ['validation.max.file'] - ); - - $this->passes( - File::default()->international()->max('1MB', File::BINARY), - $file1010 + ['validation.size.file'] ); } - public function testDefaultsToInternationalWhenNoGlobalSetting(): void + public function test_defaults_to_international_when_no_global_setting(): void { $file1010 = UploadedFile::fake()->create('test.txt', 1010); @@ -658,22 +487,7 @@ public function testDefaultsToInternationalWhenNoGlobalSetting(): void ); } - public function testBinaryVsInternationalDifference(): void - { - $file1010 = UploadedFile::fake()->create('boundary.txt', 1010); - - $this->passes( - File::default()->max('1MB', File::BINARY), - $file1010 - ); - - $this->fails( - File::default()->max('1MB', File::INTERNATIONAL), - $file1010, - ['validation.max.file']); - } - - public function testNumericSizesWorkWithoutUnits(): void + public function test_numeric_sizes_work_without_units(): void { $file1000 = UploadedFile::fake()->create('numeric.txt', 1000); @@ -682,58 +496,15 @@ public function testNumericSizesWorkWithoutUnits(): void $this->passes(File::default()->international()->max(1000), $file1000); } - public function testDecimalSizes(): void - { - $file512 = UploadedFile::fake()->create('half.txt', 512); - $file500 = UploadedFile::fake()->create('half.txt', 500); - - $this->passes( - File::default()->size('0.5MB', File::BINARY), - $file512 - ); - - $this->fails(File::default()->size('0.5MB', File::BINARY), - $file500, - ['validation.size.file'] - ); - - $this->passes( - File::default()->size('0.5MB', File::INTERNATIONAL), - $file500 - ); - - $this->fails( - File::default()->size('0.5MB', File::INTERNATIONAL), - $file512, - ['validation.size.file'] - ); - } - - public function testLargerUnits(): void - { - $file1048576 = UploadedFile::fake()->create('big.txt', 1048576); - $file1000000 = UploadedFile::fake()->create('big.txt', 1000000); - - $this->passes( - File::default()->size('1GB', File::BINARY), - $file1048576 - ); - - $this->passes( - File::default()->size('1GB', File::INTERNATIONAL), - $file1000000 - ); - } - - public function testBinaryIntegerPrecisionForLargeFileSizes(): void + public function test_binary_integer_precision_for_large_file_sizes(): void { $file999999 = UploadedFile::fake()->create('large999999.txt', 999999); - + $this->passes( File::default()->binary()->max('1000000KB'), $file999999 ); - + $this->fails( File::default()->binary()->max('999998KB'), $file999999, @@ -741,15 +512,15 @@ public function testBinaryIntegerPrecisionForLargeFileSizes(): void ); } - public function testInternationalIntegerPrecisionForLargeFileSizes(): void + public function test_international_integer_precision_for_large_file_sizes(): void { $file999999 = UploadedFile::fake()->create('large999999.txt', 999999); - + $this->passes( File::default()->international()->max('1000000KB'), $file999999 ); - + $this->fails( File::default()->international()->max('999998KB'), $file999999, @@ -757,76 +528,76 @@ public function testInternationalIntegerPrecisionForLargeFileSizes(): void ); } - public function testFloatPrecisionForFractionalSizes(): void + public function test_float_precision_for_fractional_sizes(): void { $file512 = UploadedFile::fake()->create('fractional512.txt', 512); $file500 = UploadedFile::fake()->create('fractional500.txt', 500); - + $this->passes( - File::default()->binary()->size('0.5MB'), + File::default()->size('0.5MiB'), $file512 ); - + $this->passes( - File::default()->international()->size('0.5MB'), + File::default()->size('0.5MB'), $file500 ); - + $this->fails( - File::default()->binary()->size('0.5MB'), + File::default()->size('0.5MiB'), $file500, ['validation.size.file'] ); - + $this->fails( - File::default()->international()->size('0.5MB'), + File::default()->size('0.5MB'), $file512, ['validation.size.file'] ); } - public function testBinaryVsInternationalCalculationAccuracy(): void + public function test_binary_vs_international_calculation_accuracy(): void { $file1010 = UploadedFile::fake()->create('boundary1010.txt', 1010); - + $this->passes( - File::default()->binary()->max('1MB'), + File::default()->max('1MiB'), $file1010 ); - + $this->fails( - File::default()->international()->max('1MB'), + File::default()->max('1MB'), $file1010, ['validation.max.file'] ); - + $file900 = UploadedFile::fake()->create('small900.txt', 900); - + $this->passes( - File::default()->binary()->max('1MB'), + File::default()->max('1MiB'), $file900 ); - + $this->passes( - File::default()->international()->max('1MB'), + File::default()->max('1MB'), $file900 ); } - public function testBinaryLargeFileSizePrecision(): void + public function test_binary_large_file_size_precision(): void { $file500000 = UploadedFile::fake()->create('huge500000.txt', 500000); - + $this->passes( File::default()->binary()->between('400MB', '600MB'), $file500000 ); - + $this->passes( File::default()->binary()->max('1GB'), $file500000 ); - + $this->fails( File::default()->binary()->max('488MB'), $file500000, @@ -834,20 +605,20 @@ public function testBinaryLargeFileSizePrecision(): void ); } - public function testInternationalLargeFileSizePrecision(): void + public function test_international_large_file_size_precision(): void { $file500000 = UploadedFile::fake()->create('huge500000.txt', 500000); - + $this->passes( File::default()->international()->between('400MB', '600MB'), $file500000 ); - + $this->passes( File::default()->international()->max('1GB'), $file500000 ); - + $this->fails( File::default()->international()->max('499MB'), $file500000, @@ -855,49 +626,49 @@ public function testInternationalLargeFileSizePrecision(): void ); } - public function testOverflowProtectionForLargeIntegerValues(): void + public function test_overflow_protection_for_large_integer_values(): void { $fileLarge = UploadedFile::fake()->create('overflow.txt', 2000000000); - + $this->passes( File::default()->binary()->max('2000000MB'), $fileLarge ); - + $this->passes( File::default()->international()->max('2000000MB'), $fileLarge ); } - public function testOverflowProtectionWithFractionalValues(): void + public function test_overflow_protection_with_fractional_values(): void { $file1536 = UploadedFile::fake()->create('fractional.txt', 1536); - + $this->passes( - File::default()->binary()->size('1.5MB'), + File::default()->size('1.5MiB'), $file1536 ); - + $file1500 = UploadedFile::fake()->create('fractional.txt', 1500); - + $this->passes( - File::default()->international()->size('1.5MB'), + File::default()->size('1.5MB'), $file1500 ); } - public function testCaseInsensitiveSuffixes(): void + public function test_case_insensitive_suffixes(): void { $file1024 = UploadedFile::fake()->create('case.txt', 1024); - $this->passes(File::default()->binary()->size('1MB'), $file1024); - $this->passes(File::default()->binary()->size('1mb'), $file1024); - $this->passes(File::default()->binary()->size('1Mb'), $file1024); - $this->passes(File::default()->binary()->size('1mB'), $file1024); + $this->passes(File::default()->size('1MiB'), $file1024); + $this->passes(File::default()->size('1mib'), $file1024); + $this->passes(File::default()->size('1Mib'), $file1024); + $this->passes(File::default()->size('1MIB'), $file1024); } - public function testInvalidSizeSuffixThrowsException(): void + public function test_invalid_size_suffix_throws_exception(): void { $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Invalid file size suffix.'); @@ -905,70 +676,70 @@ public function testInvalidSizeSuffixThrowsException(): void File::default()->max('5xyz'); } - public function testZeroAndVerySmallFileSizes(): void + public function test_zero_and_very_small_file_sizes(): void { $fileZero = UploadedFile::fake()->create('zero.txt', 0); $fileOne = UploadedFile::fake()->create('tiny.txt', 1); - + $this->passes( File::default()->min('0KB'), $fileZero ); - + $this->passes( File::default()->size('1KB'), $fileOne ); - + $this->passes( File::default()->binary()->max('0.001MB'), $fileOne ); } - public function testWhitespaceHandlingInFileSizes(): void + public function test_whitespace_handling_in_file_sizes(): void { $file2048 = UploadedFile::fake()->create('whitespace.txt', 2048); $file2000 = UploadedFile::fake()->create('whitespace.txt', 2000); - + $this->passes( - File::default()->binary()->size(' 2MB '), + File::default()->size(' 2MiB '), $file2048 ); - + $this->passes( - File::default()->international()->size(' 2MB '), + File::default()->size(' 2MB '), $file2000 ); - + $this->passes( - File::default()->binary()->size('2 MB'), + File::default()->size('2 MiB'), $file2048 ); } - public function testCommaSeparatedNumberParsing(): void + public function test_comma_separated_number_parsing(): void { $file1024 = UploadedFile::fake()->create('comma.txt', 1024); $file10240 = UploadedFile::fake()->create('large.txt', 10240); - + $this->passes( File::default()->binary()->size('1,024KB'), $file1024 ); - + $this->passes( File::default()->binary()->size('10,240KB'), $file10240 ); - + $this->passes( File::default()->international()->size('1,024KB'), $file1024 ); } - public function testNegativeFileSizeThrowsException(): void + public function test_negative_file_size_throws_exception(): void { $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Invalid numeric value in file size.'); @@ -976,7 +747,7 @@ public function testNegativeFileSizeThrowsException(): void File::default()->max('-5MB'); } - public function testBinaryFluentChaining(): void + public function test_binary_fluent_chaining(): void { $file1024 = UploadedFile::fake()->create('chain.txt', 1024); @@ -988,7 +759,7 @@ public function testBinaryFluentChaining(): void $this->passes($rule, $file1024); } - public function testInternationalFluentChaining(): void + public function test_international_fluent_chaining(): void { $file1000 = UploadedFile::fake()->create('chain.txt', 1000); @@ -1000,30 +771,7 @@ public function testInternationalFluentChaining(): void $this->passes($rule, $file1000); } - public function testMixedUnitsInSameRule(): void - { - $file1500 = UploadedFile::fake()->create('mixed.txt', 1500); - - $rule = File::default()->binary() - ->min('1MB') - ->max('2MB', File::INTERNATIONAL); - - $this->passes($rule, $file1500); - } - - public function testComplexUnitsScenarioWithMultipleConstraints(): void - { - $file1500 = UploadedFile::fake()->create('complex.txt', 1500); - - $rule = File::default()->binary() - ->types(['txt']) - ->min('1MB', File::INTERNATIONAL) - ->max('1.4MB', File::BINARY); - - $this->fails($rule, $file1500, ['validation.between.file']); - } - - public function testUnitsMethodsReturnNewInstances(): void + public function test_units_methods_return_new_instances(): void { $binary1 = File::default()->binary(); $binary2 = File::default()->binary(); @@ -1033,7 +781,7 @@ public function testUnitsMethodsReturnNewInstances(): void $this->assertNotSame($binary1, $international1); } - public function testUnitsBackwardsCompatibility(): void + public function test_units_backwards_compatibility(): void { $file1000 = UploadedFile::fake()->create('compat.txt', 1000); @@ -1053,24 +801,22 @@ public function testUnitsBackwardsCompatibility(): void ); } - public function testBinaryFluentMethod(): void + public function test_binary_fluent_method(): void { $file1024 = UploadedFile::fake()->create('binary.txt', 1024); - // Create instance then call binary() method $rule = File::default() ->types(['txt']) ->binary() - ->size('1MB'); + ->size('1MiB'); $this->passes($rule, $file1024); } - public function testInternationalFluentMethod(): void + public function test_international_fluent_method(): void { $file1000 = UploadedFile::fake()->create('international.txt', 1000); - // Create instance then call international() method $rule = File::default() ->types(['txt']) ->international() @@ -1079,39 +825,104 @@ public function testInternationalFluentMethod(): void $this->passes($rule, $file1000); } - public function testUnitsMethodChaining(): void + public function test_units_method_chaining(): void { $file1024 = UploadedFile::fake()->create('chaining.txt', 1024); - // Test switching between binary and international on same instance $rule = File::default() ->types(['txt']) - ->international() // Start with international - ->binary() // Switch to binary - ->size('1MB'); // 1MB binary = 1024KB + ->international() + ->binary() + ->size('1MiB'); $this->passes($rule, $file1024); } - public function testInstanceMethodsReturnSameObject(): void + public function test_instance_methods_return_same_object(): void { $originalRule = File::default()->types(['txt']); - - // Instance methods should return the same object + $binaryRule = $originalRule->binary(); $internationalRule = $originalRule->international(); - + $this->assertSame($originalRule, $binaryRule); $this->assertSame($originalRule, $internationalRule); - - // Creating new instances should return different objects + $newBinary = File::default()->binary(); $newInternational = File::default()->international(); - + $this->assertNotSame($originalRule, $newBinary); $this->assertNotSame($originalRule, $newInternational); } + public function test_suffix_precedence_over_instance_methods(): void + { + $file1000 = UploadedFile::fake()->create('test.txt', 1000); + $file1030 = UploadedFile::fake()->create('test.txt', 1030); + + $this->passes( + File::default()->binary()->max('1MB'), + $file1000 + ); + + $this->fails( + File::default()->international()->max('1MiB'), + $file1030, + ['validation.max.file'] + ); + } + + public function test_naked_values_fallback_to_instance_methods(): void + { + $file1000 = UploadedFile::fake()->create('numeric.txt', 1000); + + $this->passes( + File::default()->binary()->max(1024), + $file1000 + ); + + $this->passes( + File::default()->international()->max(1000), + $file1000 + ); + + $this->fails( + File::default()->international()->max(999), + $file1000, + ['validation.max.file'] + ); + } + + public function test_comprehensive_binary_suffixes(): void + { + $file1 = UploadedFile::fake()->create('1kb.txt', 1); + $file1024 = UploadedFile::fake()->create('1mb.txt', 1024); + $file1048576 = UploadedFile::fake()->create('1gb.txt', 1048576); + + $this->passes(File::default()->size('1KiB'), $file1); + $this->passes(File::default()->size('1MiB'), $file1024); + $this->passes(File::default()->size('1GiB'), $file1048576); + } + + public function test_mixed_unit_constraints(): void + { + $file1500 = UploadedFile::fake()->create('mixed.txt', 1500); + + $rule = File::default() + ->min('1MB') + ->max('2MiB'); + + $this->passes($rule, $file1500); + + $file500 = UploadedFile::fake()->create('small.txt', 500); + + $this->fails( + $rule, + $file500, + ['validation.between.file'] + ); + } + protected function setUp(): void { $container = Container::getInstance(); From a5783c4960d193a738112080e35c43e2ba62213a Mon Sep 17 00:00:00 2001 From: Yitz Willroth Date: Wed, 9 Jul 2025 19:58:42 -0400 Subject: [PATCH 3/3] updates based upon code review feedback --- src/Illuminate/Validation/Rules/File.php | 60 ++++++++++++--------- tests/Validation/ValidationFileRuleTest.php | 4 +- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/Illuminate/Validation/Rules/File.php b/src/Illuminate/Validation/Rules/File.php index c524e32a58ce..0fbe1d81b278 100644 --- a/src/Illuminate/Validation/Rules/File.php +++ b/src/Illuminate/Validation/Rules/File.php @@ -261,31 +261,49 @@ protected function parseSize($size): false|float } /** - * Detect the suffix and determine appropriate units from a file size string. + * Detect the suffix and determine appropriate units from a file size string + * + * @throws InvalidArgumentException */ protected function detectUnits(string $size): string { return match (true) { is_numeric($size) => $this->units, - in_array(strtolower(substr(trim($size), -3)), ['kib', 'mib', 'gib', 'tib']) => self::BINARY, - in_array(strtolower(substr(trim($size), -2)), ['kb', 'mb', 'gb', 'tb']) => self::INTERNATIONAL, - default => $this->units, + $this->isBinarySizeString($size) => self::BINARY, + $this->isInternationalSizeString($size) => self::INTERNATIONAL, + default => throw new InvalidArgumentException( + "Invalid file size; units must be one of [kib, mib, gib, tib, kb, mb, gb, tb]. Given: {$size}." + ), }; } + protected function isBinarySizeString(string $size): bool + { + return in_array( + strtolower(substr(trim($size), -3)), + ['kib', 'mib', 'gib', 'tib'], + true + ); + } + + protected function isInternationalSizeString(string $size): bool + { + return in_array( + strtolower(substr(trim($size), -2)), + ['kb', 'mb', 'gb', 'tb'], + true + ); + } + /** * Convert a human-friendly file size to kilobytes using the International System. */ protected function toInternationalKilobytes(string $size, float $value): float|int { - return round( - $this->protectValueFromOverflow( - $this->prepareValueForPrecision($value), - ! is_numeric($size) - ? $this->getInternationalMultiplier(strtolower(trim($size))) - : 1 - ) - ); + return round($this->protectValueFromOverflow( + $this->prepareValueForPrecision($value), + ! is_numeric($size) ? $this->getInternationalMultiplier($size) : 1 + )); } /** @@ -293,12 +311,11 @@ protected function toInternationalKilobytes(string $size, float $value): float|i */ protected function getInternationalMultiplier(string $size): int { - return match (substr($size, -2)) { + return match (strtolower(substr(trim($size), -2))) { 'kb' => 1, 'mb' => 1_000, 'gb' => 1_000_000, 'tb' => 1_000_000_000, - default => throw new InvalidArgumentException('Invalid file size suffix.'), }; } @@ -307,14 +324,10 @@ protected function getInternationalMultiplier(string $size): int */ protected function toBinaryKilobytes(string $size, float $value): float|int { - return round( - $this->protectValueFromOverflow( - $this->prepareValueForPrecision($value), - ! is_numeric($size) - ? $this->getBinaryMultiplier(strtolower(trim($size))) - : 1 - ) - ); + return round($this->protectValueFromOverflow( + $this->prepareValueForPrecision($value), + ! is_numeric($size) ? $this->getBinaryMultiplier($size) : 1 + )); } /** @@ -322,12 +335,11 @@ protected function toBinaryKilobytes(string $size, float $value): float|int */ protected function getBinaryMultiplier(string $size): int { - return match (substr($size, -3)) { + return match (strtolower(substr(trim($size), -3))) { 'kib' => 1, 'mib' => 1_024, 'gib' => 1_048_576, 'tib' => 1_073_741_824, - default => throw new InvalidArgumentException('Invalid file size suffix.'), }; } diff --git a/tests/Validation/ValidationFileRuleTest.php b/tests/Validation/ValidationFileRuleTest.php index 6c26421c445b..21908b091ca4 100644 --- a/tests/Validation/ValidationFileRuleTest.php +++ b/tests/Validation/ValidationFileRuleTest.php @@ -671,7 +671,9 @@ public function test_case_insensitive_suffixes(): void public function test_invalid_size_suffix_throws_exception(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Invalid file size suffix.'); + $this->expectExceptionMessage( + 'Invalid file size; units must be one of [kib, mib, gib, tib, kb, mb, gb, tb]. Given: 5xyz.' + ); File::default()->max('5xyz'); }