Skip to content
This repository was archived by the owner on Jan 31, 2020. It is now read-only.

Commit 48c80ce

Browse files
weierophinneyweierophinney
committed
Merge branch 'hotfix/174'
Close #174 Fixes #167
2 parents ce7ab72 + 8729b71 commit 48c80ce

File tree

3 files changed

+228
-30
lines changed

3 files changed

+228
-30
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ All notable changes to this project will be documented in this file, in reverse
2222
`Zend\Validator\File\MimeType` "closes" the open FileInfo handle for the file
2323
being validated, using `unset()` instead of `finfo_close()`; this resolves a
2424
segfault that occurs on older PHP versions.
25+
- [#174](https://github.com/zendframework/zend-validator/pull/174) fixes how
26+
`Zend\Validator\Between` handles two situations: (1) when a non-numeric value
27+
is validated against numeric min/max values, and (2) when a numeric value is
28+
validated against non-numeric min/max values. Previously, these incorrectly
29+
validated as true; now they are marked invalid.
2530

2631
## 2.9.1 - 2017-05-17
2732

‎src/Between.php‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,15 @@ class Between extends AbstractValidator
1616
{
1717
const NOT_BETWEEN = 'notBetween';
1818
const NOT_BETWEEN_STRICT = 'notBetweenStrict';
19+
const VALUE_NOT_NUMERIC = 'valueNotNumeric';
20+
const VALUE_NOT_STRING = 'valueNotString';
21+
22+
/**
23+
* Retain if min and max are numeric values. Allow to not compare string and numeric types
24+
*
25+
* @var boolean
26+
*/
27+
private $numeric;
1928

2029
/**
2130
* Validation failure message template definitions
@@ -24,7 +33,10 @@ class Between extends AbstractValidator
2433
*/
2534
protected $messageTemplates = [
2635
self::NOT_BETWEEN => "The input is not between '%min%' and '%max%', inclusively",
27-
self::NOT_BETWEEN_STRICT => "The input is not strictly between '%min%' and '%max%'"
36+
self::NOT_BETWEEN_STRICT => "The input is not strictly between '%min%' and '%max%'",
37+
self::VALUE_NOT_NUMERIC => "The min ('%min%') and max ('%max%') values are numeric, but the input is not",
38+
self::VALUE_NOT_STRING => "The min ('%min%') and max ('%max%') values are non-numeric strings, "
39+
. "but the input is not a string",
2840
];
2941

3042
/**
@@ -81,7 +93,17 @@ public function __construct($options = null)
8193
if (count($options) !== 2
8294
&& (! array_key_exists('min', $options) || ! array_key_exists('max', $options))
8395
) {
84-
throw new Exception\InvalidArgumentException("Missing option. 'min' and 'max' have to be given");
96+
throw new Exception\InvalidArgumentException("Missing option: 'min' and 'max' have to be given");
97+
}
98+
99+
if (is_numeric($options['min']) && is_numeric($options['max'])) {
100+
$this->numeric = true;
101+
} elseif (is_string($options['min']) && is_string($options['max'])) {
102+
$this->numeric = false;
103+
} else {
104+
throw new Exception\InvalidArgumentException(
105+
"Invalid options: 'min' and 'max' should be of the same scalar type"
106+
);
85107
}
86108

87109
parent::__construct($options);
@@ -164,6 +186,15 @@ public function isValid($value)
164186
{
165187
$this->setValue($value);
166188

189+
if ($this->numeric && ! is_numeric($value)) {
190+
$this->error(self::VALUE_NOT_NUMERIC);
191+
return false;
192+
}
193+
if (! $this->numeric && ! is_string($value)) {
194+
$this->error(self::VALUE_NOT_STRING);
195+
return false;
196+
}
197+
167198
if ($this->getInclusive()) {
168199
if ($this->getMin() > $value || $value > $this->getMax()) {
169200
$this->error(self::NOT_BETWEEN);

‎test/BetweenTest.php‎

Lines changed: 190 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,38 +18,178 @@
1818
*/
1919
class BetweenTest extends TestCase
2020
{
21+
public function providerBasic()
22+
{
23+
return [
24+
'inclusive-int-valid-floor' => [
25+
'min' => 1,
26+
'max' => 100,
27+
'inclusive' => true,
28+
'expected' => true,
29+
'value' => 1,
30+
],
31+
'inclusive-int-valid-between' => [
32+
'min' => 1,
33+
'max' => 100,
34+
'inclusive' => true,
35+
'expected' => true,
36+
'value' => 10,
37+
],
38+
'inclusive-int-valid-ceiling' => [
39+
'min' => 1,
40+
'max' => 100,
41+
'inclusive' => true,
42+
'expected' => true,
43+
'value' => 100,
44+
],
45+
'inclusive-int-invaild-below' => [
46+
'min' => 1,
47+
'max' => 100,
48+
'inclusive' => true,
49+
'expected' => false,
50+
'value' => 0,
51+
],
52+
'inclusive-int-invalid-below-fractional' => [
53+
'min' => 1,
54+
'max' => 100,
55+
'inclusive' => true,
56+
'expected' => false,
57+
'value' => 0.99,
58+
],
59+
'inclusive-int-invalid-above-fractional' => [
60+
'min' => 1,
61+
'max' => 100,
62+
'inclusive' => true,
63+
'expected' => false,
64+
'value' => 100.01,
65+
],
66+
'inclusive-int-invalid-above' => [
67+
'min' => 1,
68+
'max' => 100,
69+
'inclusive' => true,
70+
'expected' => false,
71+
'value' => 101,
72+
],
73+
'exclusive-int-invalid-below' => [
74+
'min' => 1,
75+
'max' => 100,
76+
'inclusive' => false,
77+
'expected' => false,
78+
'value' => 0,
79+
],
80+
'exclusive-int-invalid-floor' => [
81+
'min' => 1,
82+
'max' => 100,
83+
'inclusive' => false,
84+
'expected' => false,
85+
'value' => 1,
86+
],
87+
'exclusive-int-invalid-ceiling' => [
88+
'min' => 1,
89+
'max' => 100,
90+
'inclusive' => false,
91+
'expected' => false,
92+
'value' => 100,
93+
],
94+
'exclusive-int-invalid-above' => [
95+
'min' => 1,
96+
'max' => 100,
97+
'inclusive' => false,
98+
'expected' => false,
99+
'value' => 101,
100+
],
101+
'inclusive-string-valid-floor' => [
102+
'min' => 'a',
103+
'max' => 'z',
104+
'inclusive' => true,
105+
'expected' => true,
106+
'value' => 'a',
107+
],
108+
'inclusive-string-valid-between' => [
109+
'min' => 'a',
110+
'max' => 'z',
111+
'inclusive' => true,
112+
'expected' => true,
113+
'value' => 'm',
114+
],
115+
'inclusive-string-valid-ceiling' => [
116+
'min' => 'a',
117+
'max' => 'z',
118+
'inclusive' => true,
119+
'expected' => true,
120+
'value' => 'z',
121+
],
122+
'exclusive-string-invalid-out-of-range' => [
123+
'min' => 'a',
124+
'max' => 'z',
125+
'inclusive' => false,
126+
'expected' => false,
127+
'value' => '!',
128+
],
129+
'exclusive-string-invalid-floor' => [
130+
'min' => 'a',
131+
'max' => 'z',
132+
'inclusive' => false,
133+
'expected' => false,
134+
'value' => 'a',
135+
],
136+
'exclusive-string-invalid-ceiling' => [
137+
'min' => 'a',
138+
'max' => 'z',
139+
'inclusive' => false,
140+
'expected' => false,
141+
'value' => 'z',
142+
],
143+
'inclusive-int-invalid-string' => [
144+
'min' => 0,
145+
'max' => 99999999,
146+
'inclusive' => true,
147+
'expected' => false,
148+
'value' => 'asdasd',
149+
],
150+
'inclusive-int-invalid-char' => [
151+
'min' => 0,
152+
'max' => 99999999,
153+
'inclusive' => true,
154+
'expected' => false,
155+
'value' => 'q',
156+
],
157+
'inclusive-string-invalid-zero' => [
158+
'min' => 'a',
159+
'max' => 'zzzzz',
160+
'inclusive' => true,
161+
'expected' => false,
162+
'value' => 0,
163+
],
164+
'inclusive-string-invalid-non-zero' => [
165+
'min' => 'a',
166+
'max' => 'zzzzz',
167+
'inclusive' => true,
168+
'expected' => false,
169+
'value' => 10,
170+
],
171+
];
172+
}
21173
/**
22174
* Ensures that the validator follows expected behavior
23175
*
176+
* @dataProvider providerBasic
177+
* @param int|float|string $min
178+
* @param int|float|string $max
179+
* @param bool $inclusive
180+
* @param bool $expected
181+
* @param mixed $value
24182
* @return void
25183
*/
26-
public function testBasic()
184+
public function testBasic($min, $max, $inclusive, $expected, $value)
27185
{
28-
/**
29-
* The elements of each array are, in order:
30-
* - minimum
31-
* - maximum
32-
* - inclusive
33-
* - expected validation result
34-
* - array of test input values
35-
*/
36-
$valuesExpected = [
37-
[1, 100, true, true, [1, 10, 100]],
38-
[1, 100, true, false, [0, 0.99, 100.01, 101]],
39-
[1, 100, false, false, [0, 1, 100, 101]],
40-
['a', 'z', true, true, ['a', 'b', 'y', 'z']],
41-
['a', 'z', false, false, ['!', 'a', 'z']]
42-
];
43-
foreach ($valuesExpected as $element) {
44-
$validator = new Between(['min' => $element[0], 'max' => $element[1], 'inclusive' => $element[2]]);
45-
foreach ($element[4] as $input) {
46-
$this->assertEquals(
47-
$element[3],
48-
$validator->isValid($input),
49-
'Failed values: ' . $input . ":" . implode("\n", $validator->getMessages())
50-
);
51-
}
52-
}
186+
$validator = new Between(['min' => $min, 'max' => $max, 'inclusive' => $inclusive]);
187+
188+
$this->assertSame(
189+
$expected,
190+
$validator->isValid($value),
191+
'Failed value: ' . $value . ':' . implode("\n", $validator->getMessages())
192+
);
53193
}
54194

55195
/**
@@ -117,7 +257,7 @@ public function testEqualsMessageVariables()
117257
public function testMissingMinOrMax(array $args)
118258
{
119259
$this->expectException(InvalidArgumentException::class);
120-
$this->expectExceptionMessage("Missing option. 'min' and 'max' have to be given");
260+
$this->expectExceptionMessage("Missing option: 'min' and 'max' have to be given");
121261

122262
new Between($args);
123263
}
@@ -140,12 +280,34 @@ public function testConstructorCanAcceptInclusiveParameter()
140280
$this->assertFalse($validator->getInclusive());
141281
}
142282

143-
public function testConstructWithTravesableOptions()
283+
public function testConstructWithTraversableOptions()
144284
{
145285
$options = new \ArrayObject(['min' => 1, 'max' => 10, 'inclusive' => false]);
146286
$validator = new Between($options);
147287

148288
$this->assertTrue($validator->isValid(5));
149289
$this->assertFalse($validator->isValid(10));
150290
}
291+
292+
public function testStringValidatedAgainstNumericMinAndMaxIsInvalidAndReturnsAFailureMessage()
293+
{
294+
$validator = new Between(['min' => 1, 'max' => 10]);
295+
$this->assertFalse($validator->isValid('a'));
296+
$messages = $validator->getMessages();
297+
$this->assertContains(
298+
'The min (\'1\') and max (\'10\') values are numeric, but the input is not',
299+
$messages
300+
);
301+
}
302+
303+
public function testNumericValidatedAgainstStringMinAndMaxIsInvalidAndReturnsAFailureMessage()
304+
{
305+
$validator = new Between(['min' => 'a', 'max' => 'z']);
306+
$this->assertFalse($validator->isValid(10));
307+
$messages = $validator->getMessages();
308+
$this->assertContains(
309+
'The min (\'a\') and max (\'z\') values are non-numeric strings, but the input is not a string',
310+
$messages
311+
);
312+
}
151313
}

0 commit comments

Comments
 (0)