Skip to content

Commit b4c83f1

Browse files
WyriHaximusWyriHaximus
authored
Merge pull request #211 from clue-labs/scalar-params
Validate query parameters and reject non-scalars
2 parents 87fb39e + 860f0ce commit b4c83f1

File tree

9 files changed

+216
-192
lines changed

9 files changed

+216
-192
lines changed

‎README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ given event loop instance.
202202

203203
#### query()
204204

205-
The `query(string $query, array $params = []): PromiseInterface<MysqlResult>` method can be used to
205+
The `query(string $query, list<string|int|float|bool|null> $params = []): PromiseInterface<MysqlResult>` method can be used to
206206
perform an async query.
207207

208208
This method returns a promise that will resolve with a `MysqlResult` on
@@ -258,7 +258,7 @@ suited for exposing multiple possible results.
258258

259259
#### queryStream()
260260

261-
The `queryStream(string $sql, array $params = []): ReadableStreamInterface` method can be used to
261+
The `queryStream(string $sql, list<string|int|float|bool|null> $params = []): ReadableStreamInterface` method can be used to
262262
perform an async query and stream the rows of the result set.
263263

264264
This method returns a readable stream that will emit each row of the

‎src/Io/Connection.php

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -90,16 +90,8 @@ public function isBusy()
9090
return $this->parser->isBusy() || !$this->executor->isIdle();
9191
}
9292

93-
/**
94-
* {@inheritdoc}
95-
*/
96-
public function query($sql, array $params = [])
93+
public function query(Query $query)
9794
{
98-
$query = new Query($sql);
99-
if ($params) {
100-
$query->bindParamsFromArray($params);
101-
}
102-
10395
$command = new QueryCommand();
10496
$command->setQuery($query);
10597
try {
@@ -146,13 +138,8 @@ public function query($sql, array $params = [])
146138
return $deferred->promise();
147139
}
148140

149-
public function queryStream($sql, $params = [])
141+
public function queryStream(Query $query)
150142
{
151-
$query = new Query($sql);
152-
if ($params) {
153-
$query->bindParamsFromArray($params);
154-
}
155-
156143
$command = new QueryCommand();
157144
$command->setQuery($query);
158145
$this->_doCommand($command);

‎src/Io/Query.php

Lines changed: 12 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -41,46 +41,22 @@ class Query
4141
//"_" => "\\_",
4242
];
4343

44-
public function __construct($sql)
45-
{
46-
$this->sql = $this->builtSql = $sql;
47-
}
48-
49-
/**
50-
* Binding params for the query, multiple arguments support.
51-
*
52-
* @param mixed $param
53-
* @return self
54-
*/
55-
public function bindParams()
56-
{
57-
$this->builtSql = null;
58-
$this->params = func_get_args();
59-
60-
return $this;
61-
}
62-
63-
public function bindParamsFromArray(array $params)
64-
{
65-
$this->builtSql = null;
66-
$this->params = $params;
67-
68-
return $this;
69-
}
70-
7144
/**
72-
* Binding params for the query, multiple arguments support.
73-
*
74-
* @param mixed $param
75-
* @return self
76-
* @deprecated
45+
* @param string $sql
46+
* @param list<string|int|float|bool|null> $params
47+
* @throws \InvalidArgumentException if given $params are invalid
7748
*/
78-
public function params()
49+
public function __construct($sql, array $params = [])
7950
{
80-
$this->params = func_get_args();
81-
$this->builtSql = null;
51+
foreach ($params as $param) {
52+
if (!\is_scalar($param) && $param !== null) {
53+
throw new \InvalidArgumentException('Query param must be of type string|int|float|bool|null, ' . (\is_object($param) ? \get_class($param) : \gettype($param)) . ' given');
54+
}
55+
}
8256

83-
return $this;
57+
$this->sql = $sql;
58+
$this->builtSql = $params ? null : $sql;
59+
$this->params = $params;
8460
}
8561

8662
public function escape($str)
@@ -105,19 +81,9 @@ protected function resolveValueForSql($value)
10581
case 'string':
10682
$value = "'" . $this->escape($value) . "'";
10783
break;
108-
case 'array':
109-
$nvalue = [];
110-
foreach ($value as $v) {
111-
$nvalue[] = $this->resolveValueForSql($v);
112-
}
113-
$value = implode(',', $nvalue);
114-
break;
11584
case 'NULL':
11685
$value = 'NULL';
11786
break;
118-
default:
119-
throw new \InvalidArgumentException(sprintf('Not supported value type of %s.', $type));
120-
break;
12187
}
12288

12389
return $value;

‎src/MysqlClient.php

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use React\Mysql\Commands\AuthenticateCommand;
88
use React\Mysql\Io\Connection;
99
use React\Mysql\Io\Factory;
10+
use React\Mysql\Io\Query;
1011
use React\Promise\Deferred;
1112
use React\Promise\Promise;
1213
use React\Promise\PromiseInterface;
@@ -177,19 +178,22 @@ public function __construct(
177178
* could allow for possible SQL injection attacks and this API is not
178179
* suited for exposing multiple possible results.
179180
*
180-
* @param string $sql SQL statement
181-
* @param array $params Parameters which should be bound to query
181+
* @param string $sql SQL statement
182+
* @param list<string|int|float|bool|null> $params Parameters which should be bound to query
182183
* @return PromiseInterface<MysqlResult>
183184
* Resolves with a `MysqlResult` on success or rejects with an `Exception` on error.
185+
* @throws \InvalidArgumentException if given $params are invalid
184186
*/
185187
public function query($sql, array $params = [])
186188
{
189+
$query = new Query($sql, $params);
190+
187191
if ($this->closed || $this->quitting) {
188192
return \React\Promise\reject(new Exception('Connection closed'));
189193
}
190194

191-
return $this->getConnection()->then(function (Connection $connection) use ($sql, $params) {
192-
return $connection->query($sql, $params)->then(function (MysqlResult $result) use ($connection) {
195+
return $this->getConnection()->then(function (Connection $connection) use ($query) {
196+
return $connection->query($query)->then(function (MysqlResult $result) use ($connection) {
193197
$this->handleConnectionReady($connection);
194198
return $result;
195199
}, function (\Exception $e) use ($connection) {
@@ -254,19 +258,23 @@ public function query($sql, array $params = [])
254258
* could allow for possible SQL injection attacks and this API is not
255259
* suited for exposing multiple possible results.
256260
*
257-
* @param string $sql SQL statement
258-
* @param array $params Parameters which should be bound to query
261+
* @param string $sql SQL statement
262+
* @param list<string|int|float|bool|null> $params Parameters which should be bound to query
259263
* @return ReadableStreamInterface
264+
* @throws \InvalidArgumentException if given $params are invalid
265+
* @throws Exception if connection is already closed/closing
260266
*/
261-
public function queryStream($sql, $params = [])
267+
public function queryStream($sql, array $params = [])
262268
{
269+
$query = new Query($sql, $params);
270+
263271
if ($this->closed || $this->quitting) {
264272
throw new Exception('Connection closed');
265273
}
266274

267275
return \React\Promise\Stream\unwrapReadable(
268-
$this->getConnection()->then(function (Connection $connection) use ($sql, $params) {
269-
$stream = $connection->queryStream($sql, $params);
276+
$this->getConnection()->then(function (Connection $connection) use ($query) {
277+
$stream = $connection->queryStream($query);
270278

271279
$stream->on('end', function () use ($connection) {
272280
$this->handleConnectionReady($connection);

‎tests/Io/ConnectionTest.php

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace React\Tests\Mysql\Io;
44

55
use React\Mysql\Io\Connection;
6+
use React\Mysql\Io\Query;
67
use React\Tests\Mysql\BaseTestCase;
78

89
class ConnectionTest extends BaseTestCase
@@ -22,7 +23,7 @@ public function testIsBusyReturnsTrueWhenParserIsBusy()
2223

2324
$connection = new Connection($stream, $executor, $parser, $loop, null);
2425

25-
$connection->query('SELECT 1');
26+
$connection->query(new Query('SELECT 1'));
2627

2728
$this->assertTrue($connection->isBusy());
2829
}
@@ -57,7 +58,7 @@ public function testQueryWillEnqueueOneCommand()
5758
$loop->expects($this->never())->method('addTimer');
5859

5960
$conn = new Connection($stream, $executor, $parser, $loop, null);
60-
$conn->query('SELECT 1');
61+
$conn->query(new Query('SELECT 1'));
6162
}
6263

6364
public function testQueryWillReturnResolvedPromiseAndStartIdleTimerWhenQueryCommandEmitsSuccess()
@@ -81,7 +82,7 @@ public function testQueryWillReturnResolvedPromiseAndStartIdleTimerWhenQueryComm
8182

8283
$this->assertNull($currentCommand);
8384

84-
$promise = $connection->query('SELECT 1');
85+
$promise = $connection->query(new Query('SELECT 1'));
8586

8687
$promise->then($this->expectCallableOnceWith($this->isInstanceOf('React\Mysql\MysqlResult')));
8788

@@ -110,7 +111,7 @@ public function testQueryWillReturnResolvedPromiseAndStartIdleTimerWhenQueryComm
110111

111112
$this->assertNull($currentCommand);
112113

113-
$promise = $connection->query('SELECT 1');
114+
$promise = $connection->query(new Query('SELECT 1'));
114115

115116
$promise->then($this->expectCallableOnceWith($this->isInstanceOf('React\Mysql\MysqlResult')));
116117

@@ -139,7 +140,7 @@ public function testQueryWillReturnResolvedPromiseAndStartIdleTimerWhenIdlePerio
139140

140141
$this->assertNull($currentCommand);
141142

142-
$promise = $connection->query('SELECT 1');
143+
$promise = $connection->query(new Query('SELECT 1'));
143144

144145
$promise->then($this->expectCallableOnceWith($this->isInstanceOf('React\Mysql\MysqlResult')));
145146

@@ -166,7 +167,7 @@ public function testQueryWillReturnResolvedPromiseAndNotStartIdleTimerWhenIdlePe
166167

167168
$this->assertNull($currentCommand);
168169

169-
$promise = $connection->query('SELECT 1');
170+
$promise = $connection->query(new Query('SELECT 1'));
170171

171172
$promise->then($this->expectCallableOnceWith($this->isInstanceOf('React\Mysql\MysqlResult')));
172173

@@ -195,7 +196,7 @@ public function testQueryWillReturnRejectedPromiseAndStartIdleTimerWhenQueryComm
195196

196197
$this->assertNull($currentCommand);
197198

198-
$promise = $connection->query('SELECT 1');
199+
$promise = $connection->query(new Query('SELECT 1'));
199200

200201
$promise->then(null, $this->expectCallableOnce());
201202

@@ -230,7 +231,7 @@ public function testQueryFollowedByIdleTimerWillQuitUnderlyingConnectionAndEmitC
230231

231232
$this->assertNull($currentCommand);
232233

233-
$connection->query('SELECT 1');
234+
$connection->query(new Query('SELECT 1'));
234235

235236
$this->assertNotNull($currentCommand);
236237
$currentCommand->emit('success');
@@ -269,7 +270,7 @@ public function testQueryFollowedByIdleTimerWillQuitUnderlyingConnectionAndEmitC
269270

270271
$this->assertNull($currentCommand);
271272

272-
$connection->query('SELECT 1');
273+
$connection->query(new Query('SELECT 1'));
273274

274275
$this->assertNotNull($currentCommand);
275276
$currentCommand->emit('success');
@@ -300,8 +301,8 @@ public function testQueryTwiceWillEnqueueSecondQueryWithoutStartingIdleTimerWhen
300301

301302
$this->assertNull($currentCommand);
302303

303-
$connection->query('SELECT 1');
304-
$connection->query('SELECT 2');
304+
$connection->query(new Query('SELECT 1'));
305+
$connection->query(new Query('SELECT 2'));
305306

306307
$this->assertNotNull($currentCommand);
307308
$currentCommand->emit('success');
@@ -328,12 +329,12 @@ public function testQueryTwiceAfterIdleTimerWasStartedWillCancelIdleTimerAndEnqu
328329

329330
$this->assertNull($currentCommand);
330331

331-
$connection->query('SELECT 1');
332+
$connection->query(new Query('SELECT 1'));
332333

333334
$this->assertNotNull($currentCommand);
334335
$currentCommand->emit('success');
335336

336-
$connection->query('SELECT 2');
337+
$connection->query(new Query('SELECT 2'));
337338
}
338339

339340
public function testQueryStreamWillEnqueueOneCommand()
@@ -350,7 +351,7 @@ public function testQueryStreamWillEnqueueOneCommand()
350351
$loop->expects($this->never())->method('addTimer');
351352

352353
$conn = new Connection($stream, $executor, $parser, $loop, null);
353-
$conn->queryStream('SELECT 1');
354+
$conn->queryStream(new Query('SELECT 1'));
354355
}
355356

356357
public function testQueryStreamWillReturnStreamThatWillEmitEndEventAndStartIdleTimerWhenQueryCommandEmitsSuccess()
@@ -374,7 +375,7 @@ public function testQueryStreamWillReturnStreamThatWillEmitEndEventAndStartIdleT
374375

375376
$this->assertNull($currentCommand);
376377

377-
$stream = $connection->queryStream('SELECT 1');
378+
$stream = $connection->queryStream(new Query('SELECT 1'));
378379

379380
$stream->on('end', $this->expectCallableOnce());
380381
$stream->on('close', $this->expectCallableOnce());
@@ -404,7 +405,7 @@ public function testQueryStreamWillReturnStreamThatWillEmitErrorEventAndStartIdl
404405

405406
$this->assertNull($currentCommand);
406407

407-
$stream = $connection->queryStream('SELECT 1');
408+
$stream = $connection->queryStream(new Query('SELECT 1'));
408409

409410
$stream->on('error', $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));
410411
$stream->on('close', $this->expectCallableOnce());
@@ -641,7 +642,7 @@ public function testQueryAfterQuitRejectsImmediately()
641642

642643
$conn = new Connection($stream, $executor, $parser, $loop, null);
643644
$conn->quit();
644-
$promise = $conn->query('SELECT 1');
645+
$promise = $conn->query(new Query('SELECT 1'));
645646

646647
$promise->then(null, $this->expectCallableOnceWith(
647648
$this->logicalAnd(
@@ -668,7 +669,7 @@ public function testQueryAfterCloseRejectsImmediately()
668669

669670
$conn = new Connection($stream, $executor, $parser, $loop, null);
670671
$conn->close();
671-
$promise = $conn->query('SELECT 1');
672+
$promise = $conn->query(new Query('SELECT 1'));
672673

673674
$promise->then(null, $this->expectCallableOnceWith(
674675
$this->logicalAnd(
@@ -697,7 +698,7 @@ public function testQueryStreamAfterQuitThrows()
697698
$conn->quit();
698699

699700
try {
700-
$conn->queryStream('SELECT 1');
701+
$conn->queryStream(new Query('SELECT 1'));
701702
} catch (\RuntimeException $e) {
702703
$this->assertEquals('Connection closing (ENOTCONN)', $e->getMessage());
703704
$this->assertEquals(defined('SOCKET_ENOTCONN') ? SOCKET_ENOTCONN : 107, $e->getCode());

0 commit comments

Comments
 (0)