Skip to content

Commit

Permalink
Cast arrays into scalar values safely.
Browse files Browse the repository at this point in the history 
strval and intval throw errors when they receive arrays. Convert
string/bool values into ''/false. Integers and floats cast into 1 as
that's what PHP would normally do.

Refs #6083
  • Loading branch information
@markstory
markstory committed Mar 16, 2015
1 parent 30f9ec5 commit 7468434
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 71 deletions.
3 changes: 3 additions & 0 deletions src/Database/Type.php
View file
Expand Up @@ -186,6 +186,9 @@ protected function _basicTypeCast($value)
if ($value === null) {
return null;
}
if (is_array($value)) {
$value = '';
}

if (!empty(self::$_basicTypes[$this->_name])) {
$typeInfo = self::$_basicTypes[$this->_name];
Expand Down
9 changes: 9 additions & 0 deletions src/Database/Type/FloatType.php
View file
Expand Up @@ -52,6 +52,9 @@ public function toDatabase($value, Driver $driver)
if ($value === null || $value === '') {
return null;
}
if (is_array($value)) {
return 1;
}
return floatval($value);
}

Expand All @@ -68,6 +71,9 @@ public function toPHP($value, Driver $driver)
if ($value === null) {
return null;
}
if (is_array($value)) {
return 1;
}
return floatval($value);
}

Expand Down Expand Up @@ -99,6 +105,9 @@ public function marshal($value)
} elseif (is_string($value) && $this->_useLocaleParser) {
return $this->_parseValue($value);
}
if (is_array($value)) {
return 1;
}

return $value;
}
Expand Down
9 changes: 9 additions & 0 deletions src/Database/Type/IntegerType.php
View file
Expand Up @@ -37,6 +37,9 @@ public function toDatabase($value, Driver $driver)
if ($value === null || $value === '') {
return null;
}
if (is_array($value)) {
return 1;
}
return (int)$value;
}

Expand All @@ -53,6 +56,9 @@ public function toPHP($value, Driver $driver)
if ($value === null) {
return null;
}
if (is_array($value)) {
return 1;
}
return (int)$value;
}

Expand Down Expand Up @@ -85,6 +91,9 @@ public function marshal($value)
if (ctype_digit($value)) {
return (int)$value;
}
if (is_array($value)) {
return 1;
}
return $value;
}
}
9 changes: 9 additions & 0 deletions tests/TestCase/Database/Type/FloatTypeTest.php
View file
Expand Up @@ -69,6 +69,9 @@ public function testToPHP()

$result = $this->type->toPHP('2 bears', $this->driver);
$this->assertSame(2.0, $result);

$result = $this->type->toPHP(['3', '4'], $this->driver);
$this->assertSame(1, $result);
}

/**
Expand All @@ -86,6 +89,9 @@ public function testToDatabase()

$result = $this->type->toDatabase('2.51', $this->driver);
$this->assertSame(2.51, $result);

$result = $this->type->toDatabase(['3', '4'], $this->driver);
$this->assertSame(1, $result);
}

/**
Expand All @@ -106,6 +112,9 @@ public function testMarshal()

$result = $this->type->marshal('3.5 bears', $this->driver);
$this->assertSame('3.5 bears', $result);

$result = $this->type->marshal(['3', '4'], $this->driver);
$this->assertSame(1, $result);
}

/**
Expand Down
9 changes: 9 additions & 0 deletions tests/TestCase/Database/Type/IntegerTypeTest.php
View file
Expand Up @@ -54,6 +54,9 @@ public function testToPHP()

$result = $this->type->toPHP('2 bears', $this->driver);
$this->assertSame(2, $result);

$result = $this->type->toPHP(['3', '4'], $this->driver);
$this->assertSame(1, $result);
}

/**
Expand All @@ -71,6 +74,9 @@ public function testToDatabase()

$result = $this->type->toDatabase('2', $this->driver);
$this->assertSame(2, $result);

$result = $this->type->toDatabase(['3', '4'], $this->driver);
$this->assertSame(1, $result);
}

/**
Expand Down Expand Up @@ -100,6 +106,9 @@ public function testMarshal()

$result = $this->type->marshal('2 monkeys', $this->driver);
$this->assertSame('2 monkeys', $result);

$result = $this->type->marshal(['3', '4'], $this->driver);
$this->assertSame(1, $result);
}

/**
Expand Down
79 changes: 8 additions & 71 deletions tests/TestCase/Database/TypeTest.php
View file
Expand Up @@ -84,8 +84,6 @@ public function testBuildBasicTypes($name)
public function basicTypesProvider()
{
return [
['float'],
['integer'],
['string'],
['text'],
['boolean']
Expand Down Expand Up @@ -157,63 +155,6 @@ public function testClear()
$this->assertNotSame($type, Type::build('float'));
}

/**
* Tests floats from database are converted correctly to PHP
*
* @return void
*/
public function testFloatToPHP()
{
$type = Type::build('float');
$float = '3.14159';
$driver = $this->getMock('\Cake\Database\Driver');
$this->assertEquals(3.14159, $type->toPHP($float, $driver));
$this->assertEquals(3.14159, $type->toPHP(3.14159, $driver));
$this->assertEquals(3.00, $type->toPHP(3, $driver));
}

/**
* Tests floats from PHP are converted correctly to statement value
*
* @return void
*/

public function testFloatToStatement()
{
$type = Type::build('float');
$float = '3.14159';
$driver = $this->getMock('\Cake\Database\Driver');
$this->assertEquals(PDO::PARAM_STR, $type->toStatement($float, $driver));
}

/**
* Tests integers from database are converted correctly to PHP
*
* @return void
*/
public function testIntegerToPHP()
{
$type = Type::build('integer');
$integer = '3';
$driver = $this->getMock('\Cake\Database\Driver');
$this->assertEquals(3, $type->toPHP($integer, $driver));
$this->assertEquals(3, $type->toPHP(3, $driver));
$this->assertEquals(3, $type->toPHP(3.57, $driver));
}

/**
* Tests integers from PHP are converted correctly to statement value
*
* @return void
*/
public function testIntegerToStatement()
{
$type = Type::build('integer');
$integer = '3';
$driver = $this->getMock('\Cake\Database\Driver');
$this->assertEquals(PDO::PARAM_INT, $type->toStatement($integer, $driver));
}

/**
* Tests bigintegers from database are converted correctly to PHP
*
Expand Down Expand Up @@ -259,6 +200,7 @@ public function testStringToPHP()
$this->assertEquals('foo', $type->toPHP($string, $driver));
$this->assertEquals('3', $type->toPHP(3, $driver));
$this->assertEquals('3.14159', $type->toPHP(3.14159, $driver));
$this->assertEquals('', $type->toPHP([3, 'elf'], $driver));
}

/**
Expand Down Expand Up @@ -287,6 +229,7 @@ public function testTextToPHP()
$this->assertEquals('foo', $type->toPHP($string, $driver));
$this->assertEquals('3', $type->toPHP(3, $driver));
$this->assertEquals('3.14159', $type->toPHP(3.14159, $driver));
$this->assertEquals('', $type->toPHP([2, 3], $driver));
}

/**
Expand All @@ -311,22 +254,14 @@ public function testBooleanToDatabase()
{
$type = Type::build('boolean');
$driver = $this->getMock('\Cake\Database\Driver');
$this->assertTrue($type->toDatabase(true, $driver));

$driver = $this->getMock('\Cake\Database\Driver');
$this->assertTrue($type->toDatabase(true, $driver));
$this->assertFalse($type->toDatabase(false, $driver));

$driver = $this->getMock('\Cake\Database\Driver');
$this->assertTrue($type->toDatabase(1, $driver));

$driver = $this->getMock('\Cake\Database\Driver');
$this->assertFalse($type->toDatabase(0, $driver));

$driver = $this->getMock('\Cake\Database\Driver');
$this->assertTrue($type->toDatabase('1', $driver));

$driver = $this->getMock('\Cake\Database\Driver');
$this->assertFalse($type->toDatabase('0', $driver));
$this->assertFalse($type->toDatabase([1, 2], $driver));
}

/**
Expand All @@ -338,9 +273,8 @@ public function testBooleanToStatement()
{
$type = Type::build('boolean');
$driver = $this->getMock('\Cake\Database\Driver');
$this->assertEquals(PDO::PARAM_BOOL, $type->toStatement(true, $driver));

$driver = $this->getMock('\Cake\Database\Driver');
$this->assertEquals(PDO::PARAM_BOOL, $type->toStatement(true, $driver));
$this->assertEquals(PDO::PARAM_BOOL, $type->toStatement(false, $driver));
}

Expand All @@ -365,6 +299,7 @@ public function testBooleanToPHP()
$this->assertFalse($type->toPHP('0', $driver));
$this->assertFalse($type->toPHP('FALSE', $driver));
$this->assertFalse($type->toPHP('false', $driver));
$this->assertFalse($type->toPHP(['2', '3'], $driver));
}

/**
Expand All @@ -385,6 +320,7 @@ public function testBooleanMarshal()
$this->assertFalse($type->marshal(0));
$this->assertFalse($type->marshal(''));
$this->assertFalse($type->marshal('invalid'));
$this->assertFalse($type->marshal(['2', '3']));
}


Expand Down Expand Up @@ -429,6 +365,7 @@ public function testDecimalToPHP()
$this->assertSame(3.14159, $type->toPHP('3.14159', $driver));
$this->assertSame(3.14159, $type->toPHP(3.14159, $driver));
$this->assertSame(3.0, $type->toPHP(3, $driver));
$this->assertSame(1, $type->toPHP(['3', '4'], $driver));
}

/**
Expand Down

10 comments on commit 7468434

@davidyell
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boo! You broke my plugin, again!

@dereuromark
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mine, too :P But already before this due to the blind appliance of the string methods.
At least now we know that we must use a custom Type class.

@robertscherer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this change breaks the JsonType example from the docs, is there a proposed fix for that?

Thanks,

@davidyell
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the docs there are going to be updated, it would be nice to put a mention in of Type::marshal() too.

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidyell @dereuromark What broke?

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robertscherer I can update the jsonType to have a marshal method.

@davidyell
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it was in my beforeSave() method, the entity which was passed in should have contained a file upload array, but instead was blank.

I fixed it like this, davidyell/CakePHP-Proffer@d72b88c

Which is also why I thought it was worth mentioning marshal() in the docs.

@ADmad
Copy link
Member

@ADmad ADmad commented on 7468434 Mar 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markstory If a POSTed field does not have a corresponding database field it will be left untouched right? Edit: Or rather the entity property does not have a corresponding db field.

@robertscherer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markstory I have fixed the issue by adding the marshal method like this to JsonType:

/**
 * Convert request data into an array
 *
 * @param mixed $value Request Data
 * @return mixed
 */
public function marshal($value)
{
    if (is_array($value) || $value === null) {
        return $value;
    }
    return json_decode($value, true);
}

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ADmad Yes, fields not in the schema do not get marshalled by the type classes.

Please sign in to comment.