Skip to content

Commit f0a938f

Browse files
sureemosureemo
authored
Fix issue - Edit/Copy/Delete row doesn't work when using GROUP BY (#17758)
* Fix issue - Edit/Copy/Delete row doesn't work when using GROUP BY Root cause: GROUP_FLAG and NUM_FLAG have the same flags value 32768 Reference to https://www.php.net/manual/en/mysqli-result.fetch-fields.php Fixes #17756 Signed-off-by: Mo Sureerat <sureemo@gmail.com>
1 parent bbf02a1 commit f0a938f

4 files changed

Lines changed: 164 additions & 10 deletions

File tree

‎libraries/classes/FieldMetadata.php‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,13 +227,22 @@ final class FieldMetadata
227227

228228
public function __construct(int $fieldType, int $fieldFlags, object $field)
229229
{
230+
$this->mappedType = $this->getTypeMap()[$fieldType] ?? null;
231+
230232
$this->isMultipleKey = (bool) ($fieldFlags & MYSQLI_MULTIPLE_KEY_FLAG);
231233
$this->isPrimaryKey = (bool) ($fieldFlags & MYSQLI_PRI_KEY_FLAG);
232234
$this->isUniqueKey = (bool) ($fieldFlags & MYSQLI_UNIQUE_KEY_FLAG);
233235
$this->isNotNull = (bool) ($fieldFlags & MYSQLI_NOT_NULL_FLAG);
234236
$this->isUnsigned = (bool) ($fieldFlags & MYSQLI_UNSIGNED_FLAG);
235237
$this->isZerofill = (bool) ($fieldFlags & MYSQLI_ZEROFILL_FLAG);
236-
$this->isNumeric = (bool) ($fieldFlags & MYSQLI_NUM_FLAG);
238+
239+
// as flags 32768 can be NUM_FLAG or GROUP_FLAG
240+
// reference: https://www.php.net/manual/en/mysqli-result.fetch-fields.php
241+
// so check field type instead of flags
242+
// but if no or unknown field type then check flags
243+
$this->isNumeric = $this->isType(self::TYPE_INT)
244+
|| ($fieldType <= 0 && ($fieldFlags & MYSQLI_NUM_FLAG));
245+
237246
$this->isBlob = (bool) ($fieldFlags & MYSQLI_BLOB_FLAG);
238247
$this->isEnum = (bool) ($fieldFlags & MYSQLI_ENUM_FLAG);
239248
$this->isSet = (bool) ($fieldFlags & MYSQLI_SET_FLAG);
@@ -244,8 +253,6 @@ public function __construct(int $fieldType, int $fieldFlags, object $field)
244253
MYSQLI_AUTO_INCREMENT_FLAG => 'auto_increment',
245254
*/
246255

247-
$this->mappedType = $this->getTypeMap()[$fieldType] ?? null;
248-
249256
$this->isMappedTypeBit = $this->isType(self::TYPE_BIT);
250257
$this->isMappedTypeGeometry = $this->isType(self::TYPE_GEOMETRY);
251258
$this->isMappedTypeTimestamp = $this->isType(self::TYPE_TIMESTAMP);

‎libraries/classes/Util.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -875,11 +875,7 @@ private static function getConditionValue(
875875
$isBinaryString = $meta->isType(FieldMetadata::TYPE_STRING) && $meta->isBinary();
876876
// 63 is the binary charset, see: https://dev.mysql.com/doc/internals/en/charsets.html
877877
$isBlobAndIsBinaryCharset = $meta->isType(FieldMetadata::TYPE_BLOB) && $meta->charsetnr === 63;
878-
// timestamp is numeric on some MySQL 4.1
879-
// for real we use CONCAT above and it should compare to string
880-
// See commit: 049fc7fef7548c2ba603196937c6dcaf9ff9bf00
881-
// See bug: https://sourceforge.net/p/phpmyadmin/bugs/3064/
882-
if ($meta->isNumeric && ! $meta->isMappedTypeTimestamp && $meta->isNotType(FieldMetadata::TYPE_REAL)) {
878+
if ($meta->isNumeric) {
883879
$conditionValue = '= ' . $row;
884880
} elseif ($isBlobAndIsBinaryCharset || (! empty($row) && $isBinaryString)) {
885881
// hexify only if this is a true not empty BLOB or a BINARY
@@ -911,7 +907,7 @@ private static function getConditionValue(
911907
. self::printableBitValue((int) $row, (int) $meta->length) . "'";
912908
} else {
913909
$conditionValue = '= \''
914-
. $dbi->escapeString($row) . '\'';
910+
. $dbi->escapeString((string) $row) . '\'';
915911
}
916912

917913
return [$conditionValue, $condition];

‎test/classes/FieldMetadataTest.php‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use const MYSQLI_BLOB_FLAG;
1111
use const MYSQLI_NUM_FLAG;
1212
use const MYSQLI_TYPE_FLOAT;
13+
use const MYSQLI_TYPE_INT24;
1314
use const MYSQLI_TYPE_STRING;
1415

1516
/**
@@ -88,6 +89,23 @@ public function testIsBinary(): void
8889
}
8990

9091
public function testIsNumeric(): void
92+
{
93+
$fm = new FieldMetadata(MYSQLI_TYPE_INT24, MYSQLI_NUM_FLAG, (object) []);
94+
$this->assertSame('int', $fm->getMappedType());
95+
$this->assertFalse($fm->isBinary());
96+
$this->assertFalse($fm->isEnum());
97+
$this->assertFalse($fm->isUniqueKey());
98+
$this->assertFalse($fm->isUnsigned());
99+
$this->assertFalse($fm->isZerofill());
100+
$this->assertFalse($fm->isSet());
101+
$this->assertFalse($fm->isNotNull());
102+
$this->assertFalse($fm->isPrimaryKey());
103+
$this->assertFalse($fm->isMultipleKey());
104+
$this->assertTrue($fm->isNumeric());
105+
$this->assertFalse($fm->isBlob());
106+
}
107+
108+
public function testIsNumericForUnknownFieldType(): void
91109
{
92110
$fm = new FieldMetadata(-1, MYSQLI_NUM_FLAG, (object) []);
93111
$this->assertSame('', $fm->getMappedType());
@@ -134,7 +152,7 @@ public function testIsNumericFloat(): void
134152
$this->assertFalse($fm->isNotNull());
135153
$this->assertFalse($fm->isPrimaryKey());
136154
$this->assertFalse($fm->isMultipleKey());
137-
$this->assertTrue($fm->isNumeric());
155+
$this->assertFalse($fm->isNumeric());
138156
$this->assertFalse($fm->isBlob());
139157
}
140158
}

‎test/classes/UtilTest.php‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@
4040
use const MYSQLI_TYPE_STRING;
4141
use const MYSQLI_UNIQUE_KEY_FLAG;
4242

43+
const FIELD_TYPE_INTEGER = 1;
44+
const FIELD_TYPE_VARCHAR = 253;
45+
const FIELD_TYPE_UNKNOWN = -1;
46+
4347
/**
4448
* @covers \PhpMyAdmin\Util
4549
*/
@@ -259,6 +263,135 @@ public function testGetUniqueConditionWithUniqueKey(): void
259263
$this->assertEquals(['`table`.`id` = \'unique\'', true, ['`table`.`id`' => '= \'unique\'']], $actual);
260264
}
261265

266+
/**
267+
* Test for Util::getUniqueCondition
268+
* note: GROUP_FLAG = MYSQLI_NUM_FLAG = 32769
269+
*
270+
* @param FieldMetadata[] $meta Meta Information for Field
271+
* @param array $row Current Ddata Row
272+
* @param array $expected Expected Result
273+
* @psalm-param array<int, mixed> $row
274+
* @psalm-param array{string, bool, array<string, string>} $expected
275+
*
276+
* @dataProvider providerGetUniqueConditionForGroupFlag
277+
*/
278+
public function testGetUniqueConditionForGroupFlag(array $meta, array $row, array $expected): void
279+
{
280+
$fieldsCount = count($meta);
281+
$actual = Util::getUniqueCondition($fieldsCount, $meta, $row);
282+
283+
$this->assertEquals($expected, $actual);
284+
}
285+
286+
/**
287+
* Provider for testGetUniqueConditionForGroupFlag
288+
*
289+
* @return array<string, array{FieldMetadata[], array<int, mixed>, array{string, bool, array<string, string>}}>
290+
*/
291+
public function providerGetUniqueConditionForGroupFlag(): array
292+
{
293+
return [
294+
'field type is integer, value is number - not escape string' => [
295+
[
296+
new FieldMetadata(FIELD_TYPE_INTEGER, MYSQLI_NUM_FLAG, (object) [
297+
'name' => 'col',
298+
'table' => 'table',
299+
'orgtable' => 'table',
300+
]),
301+
],
302+
[123],
303+
[
304+
'`table`.`col` = 123',
305+
false,
306+
['`table`.`col`' => '= 123'],
307+
],
308+
],
309+
'field type is unknown, value is string - not escape string' => [
310+
[
311+
new FieldMetadata(FIELD_TYPE_UNKNOWN, MYSQLI_NUM_FLAG, (object) [
312+
'name' => 'col',
313+
'table' => 'table',
314+
'orgtable' => 'table',
315+
]),
316+
],
317+
['test'],
318+
[
319+
'`table`.`col` = test',
320+
false,
321+
['`table`.`col`' => '= test'],
322+
],
323+
],
324+
'field type is varchar, value is string - escape string' => [
325+
[
326+
new FieldMetadata(FIELD_TYPE_VARCHAR, MYSQLI_NUM_FLAG, (object) [
327+
'name' => 'col',
328+
'table' => 'table',
329+
'orgtable' => 'table',
330+
]),
331+
],
332+
['test'],
333+
[
334+
"`table`.`col` = 'test'",
335+
false,
336+
['`table`.`col`' => "= 'test'"],
337+
],
338+
],
339+
'field type is varchar, value is string with double quote - escape string' => [
340+
[
341+
new FieldMetadata(FIELD_TYPE_VARCHAR, MYSQLI_NUM_FLAG, (object) [
342+
'name' => 'col',
343+
'table' => 'table',
344+
'orgtable' => 'table',
345+
]),
346+
],
347+
['"test"'],
348+
[
349+
"`table`.`col` = '\\\"test\\\"'",
350+
false,
351+
['`table`.`col`' => "= '\\\"test\\\"'"],
352+
],
353+
],
354+
'field type is varchar, value is string with single quote - escape string' => [
355+
[
356+
new FieldMetadata(FIELD_TYPE_VARCHAR, MYSQLI_NUM_FLAG, (object) [
357+
'name' => 'col',
358+
'table' => 'table',
359+
'orgtable' => 'table',
360+
]),
361+
],
362+
["'test'"],
363+
[
364+
"`table`.`col` = '\'test\''",
365+
false,
366+
['`table`.`col`' => "= '\'test\''"],
367+
],
368+
],
369+
'group by multiple columns and field type is mixed' => [
370+
[
371+
new FieldMetadata(FIELD_TYPE_VARCHAR, MYSQLI_NUM_FLAG, (object) [
372+
'name' => 'col',
373+
'table' => 'table',
374+
'orgtable' => 'table',
375+
]),
376+
new FieldMetadata(FIELD_TYPE_INTEGER, MYSQLI_NUM_FLAG, (object) [
377+
'name' => 'status_id',
378+
'table' => 'table',
379+
'orgtable' => 'table',
380+
]),
381+
],
382+
['test', 2],
383+
[
384+
"`table`.`col` = 'test' AND `table`.`status_id` = 2",
385+
false,
386+
[
387+
'`table`.`col`' => "= 'test'",
388+
'`table`.`status_id`' => '= 2',
389+
],
390+
],
391+
],
392+
];
393+
}
394+
262395
/**
263396
* Test for Page Selector
264397
*/

0 commit comments

Comments
 (0)