Skip to content

Commit cafdc5b

Browse files
webmozartwebmozart
committed
[Validator] Improved to-string conversion of the file size/size limit
1 parent 2147a7e commit cafdc5b

File tree

2 files changed

+119
-72
lines changed

2 files changed

+119
-72
lines changed

‎Constraints/FileValidator.php

Lines changed: 55 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@
2525
*/
2626
class FileValidator extends ConstraintValidator
2727
{
28+
const KB_BYTES = 1000;
29+
30+
const MB_BYTES = 1000000;
31+
32+
private static $suffices = array(
33+
1 => 'bytes',
34+
self::KB_BYTES => 'kB',
35+
self::MB_BYTES => 'MB',
36+
);
37+
2838
/**
2939
* {@inheritdoc}
3040
*/
@@ -43,21 +53,21 @@ public function validate($value, Constraint $constraint)
4353
case UPLOAD_ERR_INI_SIZE:
4454
if ($constraint->maxSize) {
4555
if (ctype_digit((string) $constraint->maxSize)) {
46-
$maxSize = (int) $constraint->maxSize;
56+
$limitInBytes = (int) $constraint->maxSize;
4757
} elseif (preg_match('/^\d++k$/', $constraint->maxSize)) {
48-
$maxSize = $constraint->maxSize * 1000;
58+
$limitInBytes = $constraint->maxSize * self::KB_BYTES;
4959
} elseif (preg_match('/^\d++M$/', $constraint->maxSize)) {
50-
$maxSize = $constraint->maxSize * 1000 * 1000;
60+
$limitInBytes = $constraint->maxSize * self::MB_BYTES;
5161
} else {
5262
throw new ConstraintDefinitionException(sprintf('"%s" is not a valid maximum size', $constraint->maxSize));
5363
}
54-
$maxSize = min(UploadedFile::getMaxFilesize(), $maxSize);
64+
$limitInBytes = min(UploadedFile::getMaxFilesize(), $limitInBytes);
5565
} else {
56-
$maxSize = UploadedFile::getMaxFilesize();
66+
$limitInBytes = UploadedFile::getMaxFilesize();
5767
}
5868

5969
$this->context->addViolation($constraint->uploadIniSizeErrorMessage, array(
60-
'{{ limit }}' => $maxSize,
70+
'{{ limit }}' => $limitInBytes,
6171
'{{ suffix }}' => 'bytes',
6272
));
6373

@@ -112,27 +122,45 @@ public function validate($value, Constraint $constraint)
112122
}
113123

114124
if ($constraint->maxSize) {
115-
if (ctype_digit((string) $constraint->maxSize)) {
116-
$size = filesize($path);
117-
$limit = (int) $constraint->maxSize;
118-
$suffix = 'bytes';
119-
} elseif (preg_match('/^\d++k$/', $constraint->maxSize)) {
120-
$size = round(filesize($path) / 1000, 2);
121-
$limit = (int) $constraint->maxSize;
122-
$suffix = 'KB';
125+
$sizeInBytes = filesize($path);
126+
$limitInBytes = (int) $constraint->maxSize;
127+
128+
if (preg_match('/^\d++k$/', $constraint->maxSize)) {
129+
$limitInBytes *= self::KB_BYTES;
123130
} elseif (preg_match('/^\d++M$/', $constraint->maxSize)) {
124-
$size = round(filesize($path) / (1000 * 1000), 2);
125-
$limit = (int) $constraint->maxSize;
126-
$suffix = 'MB';
127-
} else {
131+
$limitInBytes *= self::MB_BYTES;
132+
} elseif (!ctype_digit((string) $constraint->maxSize)) {
128133
throw new ConstraintDefinitionException(sprintf('"%s" is not a valid maximum size', $constraint->maxSize));
129134
}
130135

131-
if ($size > $limit) {
136+
if ($sizeInBytes > $limitInBytes) {
137+
// Convert the limit to the smallest possible number
138+
// (i.e. try "MB", then "kB", then "bytes")
139+
$coef = self::MB_BYTES;
140+
$limitAsString = (string) ($limitInBytes / $coef);
141+
142+
// Restrict the limit to 2 decimals (without rounding! we
143+
// need the precise value)
144+
while (self::moreDecimalsThan($limitAsString, 2)) {
145+
$coef /= 1000;
146+
$limitAsString = (string) ($limitInBytes / $coef);
147+
}
148+
149+
// Convert size to the same measure, but round to 2 decimals
150+
$sizeAsString = (string) round($sizeInBytes / $coef, 2);
151+
152+
// If the size and limit produce the same string output
153+
// (due to rounding), reduce the coefficient
154+
while ($sizeAsString === $limitAsString) {
155+
$coef /= 1000;
156+
$limitAsString = (string) ($limitInBytes / $coef);
157+
$sizeAsString = (string) round($sizeInBytes / $coef, 2);
158+
}
159+
132160
$this->context->addViolation($constraint->maxSizeMessage, array(
133-
'{{ size }}' => $size,
134-
'{{ limit }}' => $limit,
135-
'{{ suffix }}' => $suffix,
161+
'{{ size }}' => $sizeAsString,
162+
'{{ limit }}' => $limitAsString,
163+
'{{ suffix }}' => static::$suffices[$coef],
136164
'{{ file }}' => $path,
137165
));
138166

@@ -172,4 +200,9 @@ public function validate($value, Constraint $constraint)
172200
}
173201
}
174202
}
203+
204+
private static function moreDecimalsThan($double, $numberOfDecimals)
205+
{
206+
return strlen((string) $double) > strlen(round($double, $numberOfDecimals));
207+
}
175208
}

‎Tests/Constraints/FileValidatorTest.php

Lines changed: 64 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,13 @@ protected function setUp()
3333

3434
protected function tearDown()
3535
{
36-
fclose($this->file);
36+
if (is_resource($this->file)) {
37+
fclose($this->file);
38+
}
39+
40+
if (file_exists($this->path)) {
41+
unlink($this->path);
42+
}
3743

3844
$this->context = null;
3945
$this->validator = null;
@@ -82,90 +88,98 @@ public function testValidUploadedfile()
8288
$this->validator->validate($file, new File());
8389
}
8490

85-
public function testTooLargeBytes()
91+
public function provideMaxSizeExceededTests()
8692
{
87-
fwrite($this->file, str_repeat('0', 11));
93+
return array(
94+
array(11, 10, '11', '10', 'bytes'),
8895

89-
$constraint = new File(array(
90-
'maxSize' => 10,
91-
'maxSizeMessage' => 'myMessage',
92-
));
96+
array(ceil(1.005*1000), ceil(1.005*1000) - 1, '1005', '1004', 'bytes'),
97+
array(ceil(1.005*1000*1000), ceil(1.005*1000*1000) - 1, '1005000', '1004999', 'bytes'),
9398

94-
$this->context->expects($this->once())
95-
->method('addViolation')
96-
->with('myMessage', array(
97-
'{{ limit }}' => '10',
98-
'{{ size }}' => '11',
99-
'{{ suffix }}' => 'bytes',
100-
'{{ file }}' => $this->path,
101-
));
99+
// round(size) == 1.01kB, limit == 1kB
100+
array(ceil(1.005*1000), 1000, '1.01', '1', 'kB'),
101+
array(ceil(1.005*1000), '1k', '1.01', '1', 'kB'),
102102

103-
$this->validator->validate($this->getFile($this->path), $constraint);
104-
}
103+
// round(size) == 1kB, limit == 1kB -> use bytes
104+
array(ceil(1.004*1000), 1000, '1004', '1000', 'bytes'),
105+
array(ceil(1.004*1000), '1k', '1004', '1000', 'bytes'),
105106

106-
public function testTooLargeKiloBytes()
107-
{
108-
fwrite($this->file, str_repeat('0', 1400));
107+
array(1000 + 1, 1000, '1001', '1000', 'bytes'),
108+
array(1000 + 1, '1k', '1001', '1000', 'bytes'),
109109

110-
$constraint = new File(array(
111-
'maxSize' => '1k',
112-
'maxSizeMessage' => 'myMessage',
113-
));
110+
// round(size) == 1.01MB, limit == 1MB
111+
array(ceil(1.005*1000*1000), 1000*1000, '1.01', '1', 'MB'),
112+
array(ceil(1.005*1000*1000), '1000k', '1.01', '1', 'MB'),
113+
array(ceil(1.005*1000*1000), '1M', '1.01', '1', 'MB'),
114114

115-
$this->context->expects($this->once())
116-
->method('addViolation')
117-
->with('myMessage', array(
118-
'{{ limit }}' => '1',
119-
'{{ size }}' => '1.37',
120-
'{{ suffix }}' => 'KiB',
121-
'{{ file }}' => $this->path,
122-
));
115+
// round(size) == 1MB, limit == 1MB -> use kB
116+
array(ceil(1.004*1000*1000), 1000*1000, '1004', '1000', 'kB'),
117+
array(ceil(1.004*1000*1000), '1000k', '1004', '1000', 'kB'),
118+
array(ceil(1.004*1000*1000), '1M', '1004', '1000', 'kB'),
123119

124-
$this->validator->validate($this->getFile($this->path), $constraint);
120+
array(1000*1000 + 1, 1000*1000, '1000001', '1000000', 'bytes'),
121+
array(1000*1000 + 1, '1000k', '1000001', '1000000', 'bytes'),
122+
array(1000*1000 + 1, '1M', '1000001', '1000000', 'bytes'),
123+
);
125124
}
126125

127-
public function testTooLargeMegaBytes()
126+
/**
127+
* @dataProvider provideMaxSizeExceededTests
128+
*/
129+
public function testMaxSizeExceeded($bytesWritten, $limit, $sizeAsString, $limitAsString, $suffix)
128130
{
129-
fwrite($this->file, str_repeat('0', 1400000));
131+
fseek($this->file, $bytesWritten-1, SEEK_SET);
132+
fwrite($this->file, '0');
133+
fclose($this->file);
130134

131135
$constraint = new File(array(
132-
'maxSize' => '1M',
136+
'maxSize' => $limit,
133137
'maxSizeMessage' => 'myMessage',
134138
));
135139

136140
$this->context->expects($this->once())
137141
->method('addViolation')
138142
->with('myMessage', array(
139-
'{{ limit }}' => '1',
140-
'{{ size }}' => '1.34',
141-
'{{ suffix }}' => 'MiB',
143+
'{{ limit }}' => $limitAsString,
144+
'{{ size }}' => $sizeAsString,
145+
'{{ suffix }}' => $suffix,
142146
'{{ file }}' => $this->path,
143147
));
144148

145149
$this->validator->validate($this->getFile($this->path), $constraint);
146150
}
147151

148-
public function testMaxSizeKiloBytes()
152+
public function provideMaxSizeNotExceededTests()
149153
{
150-
fwrite($this->file, str_repeat('0', 1010));
154+
return array(
155+
array(10, 10),
156+
array(9, 10),
151157

152-
$constraint = new File(array(
153-
'maxSize' => '1k',
154-
));
158+
array(1000, '1k'),
159+
array(1000 - 1, '1k'),
155160

156-
$this->context->expects($this->never())->method('addViolation');
157-
$this->validator->validate($this->getFile($this->path), $constraint);
161+
array(1000*1000, '1M'),
162+
array(1000*1000 - 1, '1M'),
163+
);
158164
}
159165

160-
public function testMaxSizeMegaBytes()
166+
/**
167+
* @dataProvider provideMaxSizeNotExceededTests
168+
*/
169+
public function testMaxSizeNotExceeded($bytesWritten, $limit)
161170
{
162-
fwrite($this->file, str_repeat('0', (1024 * 1022)));
171+
fseek($this->file, $bytesWritten-1, SEEK_SET);
172+
fwrite($this->file, '0');
173+
fclose($this->file);
163174

164175
$constraint = new File(array(
165-
'maxSize' => '1M',
176+
'maxSize' => $limit,
177+
'maxSizeMessage' => 'myMessage',
166178
));
167179

168-
$this->context->expects($this->never())->method('addViolation');
180+
$this->context->expects($this->never())
181+
->method('addViolation');
182+
169183
$this->validator->validate($this->getFile($this->path), $constraint);
170184
}
171185

0 commit comments

Comments
 (0)