Skip to content

Commit e44cfab

Browse files
n0nag0nn0nag0n
authored
Merge pull request #22 from flightphp/windows-and-escape-fixes
Windows and escape fixes
2 parents 3b52872 + 3650a3a commit e44cfab

File tree

7 files changed

+139
-58
lines changed

7 files changed

+139
-58
lines changed

‎README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
# FlightPHP Active Record
2-
[![Latest Stable Version](http://poser.pugx.org/flightphp/active-record/v)](https://packagist.org/packages/flightphp/active-record)
2+
[![Latest Stable Version](https://poser.pugx.org/flightphp/active-record/v)](https://packagist.org/packages/flightphp/active-record)
33
[![License](https://poser.pugx.org/flightphp/active-record/license)](https://packagist.org/packages/flightphp/active-record)
4-
[![PHP Version Require](http://poser.pugx.org/flightphp/active-record/require/php)](https://packagist.org/packages/flightphp/active-record)
5-
[![Dependencies](http://poser.pugx.org/flightphp/active-record/dependents)](https://packagist.org/packages/flightphp/active-record)
4+
[![PHP Version Require](https://poser.pugx.org/flightphp/active-record/require/php)](https://packagist.org/packages/flightphp/active-record)
5+
[![Dependencies](https://poser.pugx.org/flightphp/active-record/dependents)](https://packagist.org/packages/flightphp/active-record)
66

77
An active record is mapping a database entity to a PHP object. Spoken plainly, if you have a users table in your database, you can "translate" a row in that table to a `User` class and a `$user` object in your codebase. See [basic example](#basic-example).
88

‎composer.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
"phpunit/phpunit": "^9.0",
2727
"squizlabs/php_codesniffer": "^3.8",
2828
"rregeer/phpunit-coverage-check": "^0.3.1",
29-
"flightphp/runway": "^0.2"
29+
"flightphp/runway": "^0.2.4 || ^1.0"
3030
},
3131
"autoload": {
3232
"psr-4": {"flight\\": "src/"}
@@ -35,6 +35,6 @@
3535
"test": "phpunit",
3636
"test-coverage": "XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-html=coverage --coverage-clover=clover.xml && vendor/bin/coverage-check clover.xml 100",
3737
"beautify": "phpcbf --standard=phpcs.xml",
38-
"phpcs": "phpcs --standard=phpcs.xml"
39-
}
38+
"phpcs": "phpcs -n --standard=phpcs.xml"
39+
}
4040
}

‎src/ActiveRecord.php‎

Lines changed: 66 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
use Exception;
88
use flight\database\DatabaseInterface;
99
use flight\database\DatabaseStatementInterface;
10+
use flight\database\mysqli\MysqliAdapter;
11+
use flight\database\pdo\PdoAdapter;
1012
use JsonSerializable;
1113
use mysqli;
1214
use PDO;
@@ -93,15 +95,20 @@ abstract class ActiveRecord extends Base implements JsonSerializable
9395
/**
9496
* Database connection
9597
*
96-
* @var DatabaseInterface
98+
* @var DatabaseInterface|null
9799
*/
98-
protected DatabaseInterface $databaseConnection;
100+
protected ?DatabaseInterface $databaseConnection;
99101

100102
/**
101103
* @var string The table name in database.
102104
*/
103105
protected string $table;
104106

107+
/**
108+
* @var string The type of database engine
109+
*/
110+
protected string $databaseEngineType;
111+
105112
/**
106113
* @var string The primary key of this ActiveRecord, only supports single primary key.
107114
*/
@@ -162,8 +169,12 @@ public function __construct($databaseConnection = null, ?string $table = '', arr
162169
$this->transformAndPersistConnection($rawConnection);
163170
} elseif ($databaseConnection instanceof DatabaseInterface) {
164171
$this->databaseConnection = $databaseConnection;
172+
} else {
173+
$this->databaseConnection = null;
165174
}
166175

176+
$this->databaseEngineType = $this->getDatabaseEngine();
177+
167178
if ($table) {
168179
$this->table = $table;
169180
}
@@ -193,9 +204,9 @@ public function __call($name, $args)
193204
'operator' => ActiveRecordData::SQL_PARTS[$name],
194205
'target' => implode(', ', $args)
195206
]);
196-
} else if(method_exists($this->databaseConnection, $name) === true) {
197-
return call_user_func_array([ $this->databaseConnection, $name ], $args);
198-
}
207+
} elseif (method_exists($this->databaseConnection, $name) === true) {
208+
return call_user_func_array([ $this->databaseConnection, $name ], $args);
209+
}
199210
return $this;
200211
}
201212

@@ -317,7 +328,7 @@ protected function resetQueryData(): self
317328
{
318329
$this->params = [];
319330
$this->sqlExpressions = [];
320-
$this->join = null;
331+
$this->join = null;
321332
return $this;
322333
}
323334
/**
@@ -391,6 +402,23 @@ public function setDatabaseConnection($databaseConnection): void
391402
}
392403
}
393404

405+
/**
406+
* Returns the type of database engine. Can be one of: mysql, pgsql, sqlite, oci, sqlsrv, odbc, ibm, informix, firebird, 4D, generic.
407+
*
408+
* @return string
409+
*/
410+
public function getDatabaseEngine(): string
411+
{
412+
if ($this->databaseConnection instanceof PdoAdapter || is_subclass_of($this->databaseConnection, PDO::class) === true) {
413+
// returns value of mysql, pgsql, sqlite, oci, sqlsrv, odbc, ibm, informix, firebird, 4D, generic.
414+
return $this->databaseConnection->getConnection()->getAttribute(PDO::ATTR_DRIVER_NAME);
415+
} elseif ($this->databaseConnection instanceof MysqliAdapter) {
416+
return 'mysql';
417+
} else {
418+
return 'generic';
419+
}
420+
}
421+
394422
/**
395423
* function to find one record and assign in to current object.
396424
* @param int|string $id If call this function using this param, will find record by using this id. If not set, just find the first record in database.
@@ -449,9 +477,14 @@ public function insert(): ActiveRecord
449477
}
450478

451479
$value = $this->filterParam($this->dirty);
480+
481+
// escape column names from dirty data
482+
$columnNames = array_keys($this->dirty);
483+
$escapedColumnNames = array_map([$this, 'escapeIdentifier'], $columnNames);
484+
452485
$this->insert = new Expressions([
453-
'operator' => 'INSERT INTO ' . $this->table,
454-
'target' => new WrapExpressions(['target' => array_keys($this->dirty)])
486+
'operator' => 'INSERT INTO ' . $this->escapeIdentifier($this->table),
487+
'target' => new WrapExpressions(['target' => $escapedColumnNames])
455488
]);
456489
$this->values = new Expressions(['operator' => 'VALUES', 'target' => new WrapExpressions(['target' => $value])]);
457490

@@ -622,9 +655,9 @@ protected function &getRelation(string $name)
622655
protected function buildSqlCallback(string $sql_statement, ActiveRecord $object): string
623656
{
624657
if ('select' === $sql_statement && null == $object->$sql_statement) {
625-
$sql_statement = strtoupper($sql_statement) . ' ' . $object->table . '.*';
658+
$sql_statement = strtoupper($sql_statement) . ' ' . $this->escapeIdentifier($object->table) . '.*';
626659
} elseif (('update' === $sql_statement || 'from' === $sql_statement) && null == $object->$sql_statement) {
627-
$sql_statement = strtoupper($sql_statement) . ' ' . $object->table;
660+
$sql_statement = strtoupper($sql_statement) . ' ' . $this->escapeIdentifier($object->table);
628661
} elseif ('delete' === $sql_statement) {
629662
$sql_statement = strtoupper($sql_statement) . ' ';
630663
} else {
@@ -723,7 +756,7 @@ public function addCondition(string $field, string $operator, $value, string $de
723756
// skip adding the `table.` prefix if it's already there or a function is being supplied.
724757
$skip_table_prefix = (strpos($field, '.') !== false || strpos($field, '(') !== false);
725758
$expressions = new Expressions([
726-
'source' => ('where' === $name && $skip_table_prefix === false ? $this->table . '.' : '') . $field,
759+
'source' => ('where' === $name && $skip_table_prefix === false ? $this->escapeIdentifier($this->table) . '.' : '') . $this->escapeIdentifier($field),
727760
'operator' => $operator,
728761
'target' => (
729762
is_array($value)
@@ -816,6 +849,28 @@ protected function processEvent($event, array $data_to_pass = [])
816849
}
817850
}
818851

852+
853+
/**
854+
* Escapes a database identifier (e.g., table or column name) to prevent SQL injection.
855+
*
856+
* @param string $name The database identifier to be escaped.
857+
* @return string The escaped database identifier.
858+
*/
859+
public function escapeIdentifier(string $name)
860+
{
861+
switch ($this->databaseEngineType) {
862+
case 'sqlite':
863+
case 'pgsql':
864+
return '"' . $name . '"';
865+
case 'mysql':
866+
return '`' . $name . '`';
867+
case 'sqlsrv':
868+
return '[' . $name . ']';
869+
default:
870+
return $name;
871+
}
872+
}
873+
819874
/**
820875
* @inheritDoc
821876
*/

‎src/database/pdo/PdoAdapter.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,14 @@ public function lastInsertId()
4242
{
4343
return $this->pdo->lastInsertId();
4444
}
45+
46+
/**
47+
* Returns a PDO connection to the database.
48+
*
49+
* @return PDO The PDO connection instance.
50+
*/
51+
public function getConnection(): PDO
52+
{
53+
return $this->pdo;
54+
}
4555
}

‎tests/ActiveRecordPdoIntegrationTest.php‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ public function testInsert()
5757
$user->password = md5('demo');
5858
$user->insert();
5959
$this->assertGreaterThan(0, $user->id);
60+
$sql = $user->getBuiltSql();
61+
$this->assertStringContainsString('INSERT INTO "user" ("name","password")', $sql);
62+
$this->assertStringContainsString('VALUES (:ph1,:ph2)', $sql);
6063
}
6164

6265
public function testInsertNoChanges()
@@ -81,6 +84,9 @@ public function testEdit()
8184
$this->assertEquals('demo1', $user->name);
8285
$this->assertNotEquals($original_password, $user->password);
8386
$this->assertEquals($original_id, $user->id);
87+
88+
$sql = $user->getBuiltSql();
89+
$this->assertStringContainsString('UPDATE "user" SET "name" = :ph3 , "password" = :ph4 WHERE "user"."id" = :ph5', $sql);
8490
}
8591

8692
public function testUpdateNoChanges()
@@ -183,7 +189,7 @@ public function testJoin()
183189
$this->assertEquals($user->address, $contact->address);
184190
}
185191

186-
public function testJoinIsClearedAfterCalledTwice()
192+
public function testJoinIsClearedAfterCalledTwice()
187193
{
188194
$user = new User(new PDO('sqlite:test.db'));
189195
$user->name = 'demo';
@@ -202,8 +208,8 @@ public function testJoinIsClearedAfterCalledTwice()
202208
$this->assertEquals($user->email, $contact->email);
203209
$this->assertEquals($user->address, $contact->address);
204210

205-
$user->select('*, c.email, c.address')->join('contact as c', 'c.user_id = user.id')->find();
206-
// email and address will stored in user data array.
211+
$user->select('*, c.email, c.address')->join('contact as c', 'c.user_id = user.id')->find();
212+
// email and address will stored in user data array.
207213
$this->assertEquals($user->id, $contact->user_id);
208214
$this->assertEquals($user->email, $contact->email);
209215
$this->assertEquals($user->address, $contact->address);
@@ -228,6 +234,8 @@ public function getDirty()
228234
$contact->insert();
229235

230236
$user->isNotNull('id')->eq('id', 1)->lt('id', 2)->gt('id', 0)->find();
237+
$sql = $user->getBuiltSql();
238+
$this->assertStringContainsString('SELECT "user".* FROM "user" WHERE "user"."id" IS NOT NULL AND "user"."id" = 1 AND "user"."id" < 2 AND "user"."id" > 0', $sql);
231239
$this->assertGreaterThan(0, $user->id);
232240
$this->assertSame([], $user->getDirty());
233241
$user->name = 'testname';
@@ -277,12 +285,15 @@ public function testDelete()
277285
$this->assertEquals($uid, $new_user->eq('id', $uid)->find()->id);
278286
$this->assertTrue($contact->user->delete());
279287
$this->assertTrue($contact->delete());
288+
289+
$sql = $contact->getBuiltSql();
280290
$new_contact = new Contact(new PDO('sqlite:test.db'));
281291
$new_user = new User(new PDO('sqlite:test.db'));
282292
$this->assertInstanceOf(Contact::class, $new_contact->eq('id', $cid)->find());
283293
$this->assertEmpty($new_contact->id);
284294
$this->assertInstanceOf(User::class, $new_user->find($uid));
285295
$this->assertEmpty($new_user->id);
296+
$this->assertStringContainsString('DELETE FROM "contact" WHERE "contact"."id" = :ph4', $sql);
286297
}
287298

288299
public function testDeleteWithConditions()
@@ -654,10 +665,10 @@ public function testTextBasedPrimaryKeyDuplicateKey()
654665
$myTextTable2->save();
655666
}
656667

657-
public function testCallMethodPassingToPdoConnection()
658-
{
659-
$result = $this->ActiveRecord->prepare('SELECT * FROM user');
660-
$this->assertInstanceOf(PdoStatementAdapter::class, $result);
661-
$this->assertNotInstanceOf(ActiveRecord::class, $result);
662-
}
668+
public function testCallMethodPassingToPdoConnection()
669+
{
670+
$result = $this->ActiveRecord->prepare('SELECT * FROM user');
671+
$this->assertInstanceOf(PdoStatementAdapter::class, $result);
672+
$this->assertNotInstanceOf(ActiveRecord::class, $result);
673+
}
663674
}

‎tests/ActiveRecordTest.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class ActiveRecordTest extends \PHPUnit\Framework\TestCase
1313
public function testMagicSet()
1414
{
1515
$pdo_mock = $this->createStub(PDO::class);
16+
$pdo_mock->method('getAttribute')->willReturn('generic');
1617
$record = new class ($pdo_mock) extends ActiveRecord {
1718
public function getDirty()
1819
{
@@ -28,6 +29,7 @@ public function getDirty()
2829
public function testExecutePdoError()
2930
{
3031
$pdo_mock = $this->createStub(PDO::class);
32+
$pdo_mock->method('getAttribute')->willReturn('generic');
3133
$pdo_mock->method('prepare')->willReturn(false);
3234
$pdo_mock->method('errorInfo')->willReturn(['HY000', 1, 'test']);
3335
$record = new class ($pdo_mock) extends ActiveRecord {
@@ -42,6 +44,7 @@ public function testExecuteStatementError()
4244
$statement_mock = $this->createStub(PDOStatement::class);
4345
$pdo_mock = $this->createStub(PDO::class);
4446
$pdo_mock->method('prepare')->willReturn($statement_mock);
47+
$pdo_mock->method('getAttribute')->willReturn('generic');
4548
$statement_mock->method('execute')->willReturn(false);
4649
$statement_mock->method('errorInfo')->willReturn(['HY000', 1, 'test_statement']);
4750
$record = new class ($pdo_mock) extends ActiveRecord {
@@ -54,6 +57,7 @@ public function testExecuteStatementError()
5457
public function testUnsetSqlExpressions()
5558
{
5659
$pdo_mock = $this->createStub(PDO::class);
60+
$pdo_mock->method('getAttribute')->willReturn('generic');
5761
$record = new class ($pdo_mock) extends ActiveRecord {
5862
};
5963
$record->where = '1';
@@ -64,6 +68,7 @@ public function testUnsetSqlExpressions()
6468
public function testCustomData()
6569
{
6670
$pdo_mock = $this->createStub(PDO::class);
71+
$pdo_mock->method('getAttribute')->willReturn('generic');
6772
$record = new class ($pdo_mock) extends ActiveRecord {
6873
};
6974
$record->setCustomData('test', 'something');
@@ -73,6 +78,7 @@ public function testCustomData()
7378
public function testCustomDataUnset()
7479
{
7580
$pdo_mock = $this->createStub(PDO::class);
81+
$pdo_mock->method('getAttribute')->willReturn('generic');
7682
$record = new class ($pdo_mock) extends ActiveRecord {
7783
};
7884
$record->setCustomData('test', 'something');

0 commit comments

Comments
 (0)