Skip to content

Commit db5db26

Browse files
kamil-tekielakamil-tekiela
authored
Refactor Database Search (#17962)
* Refactor Search::getWhereClause() Signed-off-by: Kamil Tekiela <tekiela246@gmail.com> * Refactor Search::getSearchSqls() Signed-off-by: Kamil Tekiela <tekiela246@gmail.com> * Fix HTML escaping issues in Database search Signed-off-by: Kamil Tekiela <tekiela246@gmail.com> * Change links to buttons More semantically correct, does not link to a separate page on middleclick, and looks better. Signed-off-by: Kamil Tekiela <tekiela246@gmail.com> * Refactor Search::getSearchResults() Signed-off-by: Kamil Tekiela <tekiela246@gmail.com> * Update psalm-baseline.xml Signed-off-by: Kamil Tekiela <tekiela246@gmail.com> * Update phpstan-baseline.neon Signed-off-by: Kamil Tekiela <tekiela246@gmail.com> Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
1 parent 1e4af77 commit db5db26

7 files changed

Lines changed: 70 additions & 128 deletions

File tree

‎js/src/database/search.js‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import getImageTag from '../modules/functions/getImageTag.js';
2323
* Unbind all event handlers before tearing down a page
2424
*/
2525
AJAX.registerTeardown('database/search.js', function () {
26-
$('a.browse_results').off('click');
27-
$('a.delete_results').off('click');
26+
$('.browse_results').off('click');
27+
$('.delete_results').off('click');
2828
$('#buttonGo').off('click');
2929
$('#togglesearchresultlink').off('click');
3030
$('#togglequerybox').off('click');
@@ -119,7 +119,7 @@ AJAX.registerOnload('database/search.js', function () {
119119
/*
120120
* Ajax Event handler for retrieving the results from a table
121121
*/
122-
$(document).on('click', 'a.browse_results', function (e) {
122+
$(document).on('click', '.browse_results', function (e) {
123123
e.preventDefault();
124124
/** Hides the results shown by the delete criteria */
125125
var $msg = ajaxShowMessage(window.Messages.strBrowsing, false);
@@ -128,9 +128,9 @@ AJAX.registerOnload('database/search.js', function () {
128128
/** Load the browse results to the page */
129129
$('#table-info').show();
130130
var tableName = $(this).data('table-name');
131-
$('#table-link').attr({ 'href': $(this).attr('href') }).text(tableName);
131+
$('#table-link').attr({ 'href': $(this).data('href') }).text(tableName);
132132

133-
var url = $(this).attr('href') + '#searchresults';
133+
var url = $(this).data('href') + '#searchresults';
134134
var browseSql = $(this).data('browse-sql');
135135
var params = {
136136
'ajax_request': true,
@@ -159,7 +159,7 @@ AJAX.registerOnload('database/search.js', function () {
159159
/*
160160
* Ajax Event handler for deleting the results from a table
161161
*/
162-
$(document).on('click', 'a.delete_results', function (e) {
162+
$(document).on('click', '.delete_results', function (e) {
163163
e.preventDefault();
164164
/** Hides the results shown by the browse criteria */
165165
$('#table-info').hide();
@@ -179,7 +179,7 @@ AJAX.registerOnload('database/search.js', function () {
179179
'is_js_confirmed': true,
180180
'sql_query': $(this).data('delete-sql')
181181
};
182-
var url = $(this).attr('href');
182+
var url = $(this).data('href');
183183

184184
$.post(url, params, function (data) {
185185
if (typeof data === 'undefined' || ! data.success) {

‎libraries/classes/Database/Search.php‎

Lines changed: 27 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,10 @@
1414
use function __;
1515
use function array_intersect;
1616
use function array_key_exists;
17-
use function count;
1817
use function explode;
19-
use function htmlspecialchars;
2018
use function implode;
21-
use function intval;
2219
use function is_array;
2320
use function is_string;
24-
use function strlen;
2521

2622
/**
2723
* Class to handle database search
@@ -73,7 +69,7 @@ class Search
7369
/**
7470
* Criteria Tables to search in
7571
*
76-
* @var array
72+
* @var string[]
7773
*/
7874
private $criteriaTables;
7975

@@ -145,9 +141,9 @@ private function setSearchParams(): void
145141
}
146142

147143
if (empty($_POST['criteriaColumnName']) || ! is_string($_POST['criteriaColumnName'])) {
148-
unset($this->criteriaColumnName);
144+
$this->criteriaColumnName = '';
149145
} else {
150-
$this->criteriaColumnName = $this->dbi->escapeString($_POST['criteriaColumnName']);
146+
$this->criteriaColumnName = $_POST['criteriaColumnName'];
151147
}
152148
}
153149

@@ -156,35 +152,25 @@ private function setSearchParams(): void
156152
*
157153
* @param string $table The table name
158154
*
159-
* @return array 3 SQL queries (for count, display and delete results)
155+
* @return string[] 3 SQL queries (for count, display and delete results)
160156
*
161157
* @todo can we make use of fulltextsearch IN BOOLEAN MODE for this?
162-
* PMA_backquote
163-
* DatabaseInterface::fetchAssoc
164-
* $GLOBALS['db']
165-
* explode
166-
* count
167-
* strlen
168158
*/
169-
private function getSearchSqls($table)
159+
private function getSearchSqls(string $table): array
170160
{
171161
// Statement types
172162
$sqlstr_select = 'SELECT';
173163
$sqlstr_delete = 'DELETE';
174164
// Table to use
175-
$sqlstr_from = ' FROM '
176-
. Util::backquote($GLOBALS['db']) . '.'
177-
. Util::backquote($table);
165+
$sqlstr_from = ' FROM ' . Util::backquote($GLOBALS['db']) . '.' . Util::backquote($table);
178166
// Gets where clause for the query
179167
$where_clause = $this->getWhereClause($table);
180168
// Builds complete queries
181169
$sql = [];
182-
$sql['select_columns'] = $sqlstr_select . ' * ' . $sqlstr_from
183-
. $where_clause;
170+
$sql['select_columns'] = $sqlstr_select . ' *' . $sqlstr_from . $where_clause;
184171
// here, I think we need to still use the COUNT clause, even for
185172
// VIEWs, anyway we have a WHERE clause that should limit results
186-
$sql['select_count'] = $sqlstr_select . ' COUNT(*) AS `count`'
187-
. $sqlstr_from . $where_clause;
173+
$sql['select_count'] = $sqlstr_select . ' COUNT(*) AS `count`' . $sqlstr_from . $where_clause;
188174
$sql['delete'] = $sqlstr_delete . $sqlstr_from . $where_clause;
189175

190176
return $sql;
@@ -197,7 +183,7 @@ private function getSearchSqls($table)
197183
*
198184
* @return string The generated where clause
199185
*/
200-
private function getWhereClause($table)
186+
private function getWhereClause(string $table): string
201187
{
202188
// Columns to select
203189
$allColumns = $this->dbi->getColumns($GLOBALS['db'], $table);
@@ -208,65 +194,57 @@ private function getWhereClause($table)
208194
// For "as regular expression" (search option 5), LIKE won't be used
209195
// Usage example: If user is searching for a literal $ in a regexp search,
210196
// they should enter \$ as the value.
211-
$criteriaSearchStringEscaped = $this->dbi->escapeString($this->criteriaSearchString);
212197
// Extract search words or pattern
213198
$search_words = $this->criteriaSearchType > 2
214-
? [$criteriaSearchStringEscaped]
215-
: explode(' ', $criteriaSearchStringEscaped);
199+
? [$this->criteriaSearchString]
200+
: explode(' ', $this->criteriaSearchString);
216201

217202
foreach ($search_words as $search_word) {
218203
// Eliminates empty values
219-
if (strlen($search_word) === 0) {
204+
if ($search_word === '') {
220205
continue;
221206
}
222207

223208
$likeClausesPerColumn = [];
224209
// for each column in the table
225210
foreach ($allColumns as $column) {
226211
if (
227-
isset($this->criteriaColumnName)
228-
&& strlen($this->criteriaColumnName) !== 0
212+
$this->criteriaColumnName !== ''
229213
&& $column['Field'] != $this->criteriaColumnName
230214
) {
231215
continue;
232216
}
233217

234-
$column = 'CONVERT(' . Util::backquote($column['Field'])
235-
. ' USING utf8)';
218+
$column = 'CONVERT(' . Util::backquote($column['Field']) . ' USING utf8)';
236219
$likeClausesPerColumn[] = $column . ' ' . $like_or_regex . ' '
237-
. "'"
238-
. $automatic_wildcard . $search_word . $automatic_wildcard
239-
. "'";
220+
. $this->dbi->quoteString($automatic_wildcard . $search_word . $automatic_wildcard);
240221
}
241222

242-
if (count($likeClausesPerColumn) <= 0) {
223+
if ($likeClausesPerColumn === []) {
243224
continue;
244225
}
245226

246227
$likeClauses[] = implode(' OR ', $likeClausesPerColumn);
247228
}
248229

249-
// Use 'OR' if 'at least one word' is to be searched, else use 'AND'
250-
$implode_str = ($this->criteriaSearchType == 1 ? ' OR ' : ' AND ');
251-
if (empty($likeClauses)) {
230+
if ($likeClauses === []) {
252231
// this could happen when the "inside column" does not exist
253232
// in any selected tables
254-
$where_clause = ' WHERE FALSE';
255-
} else {
256-
$where_clause = ' WHERE ('
257-
. implode(') ' . $implode_str . ' (', $likeClauses)
258-
. ')';
233+
return ' WHERE FALSE';
259234
}
260235

261-
return $where_clause;
236+
// Use 'OR' if 'at least one word' is to be searched, else use 'AND'
237+
$implode_str = ($this->criteriaSearchType == 1 ? ' OR ' : ' AND ');
238+
239+
return ' WHERE (' . implode(') ' . $implode_str . ' (', $likeClauses) . ')';
262240
}
263241

264242
/**
265243
* Displays database search results
266244
*
267245
* @return string HTML for search results
268246
*/
269-
public function getSearchResults()
247+
public function getSearchResults(): string
270248
{
271249
$resultTotal = 0;
272250
$rows = [];
@@ -275,13 +253,11 @@ public function getSearchResults()
275253
// Gets the SQL statements
276254
$newSearchSqls = $this->getSearchSqls($eachTable);
277255
// Executes the "COUNT" statement
278-
$resultCount = intval($this->dbi->fetchValue(
279-
$newSearchSqls['select_count']
280-
));
256+
$resultCount = (int) $this->dbi->fetchValue($newSearchSqls['select_count']);
281257
$resultTotal += $resultCount;
282258
// Gets the result row's HTML for a table
283259
$rows[] = [
284-
'table' => htmlspecialchars($eachTable),
260+
'table' => $eachTable,
285261
'new_search_sqls' => $newSearchSqls,
286262
'result_count' => $resultCount,
287263
];
@@ -292,7 +268,7 @@ public function getSearchResults()
292268
'rows' => $rows,
293269
'result_total' => $resultTotal,
294270
'criteria_tables' => $this->criteriaTables,
295-
'criteria_search_string' => htmlspecialchars($this->criteriaSearchString),
271+
'criteria_search_string' => $this->criteriaSearchString,
296272
'search_type_description' => $this->searchTypeDescription,
297273
]);
298274
}
@@ -310,7 +286,7 @@ public function getMainHtml()
310286
'criteria_search_type' => $this->criteriaSearchType,
311287
'criteria_tables' => $this->criteriaTables,
312288
'tables_names_only' => $this->tablesNamesOnly,
313-
'criteria_column_name' => $this->criteriaColumnName ?? null,
289+
'criteria_column_name' => $this->criteriaColumnName,
314290
]);
315291
}
316292
}

‎phpstan-baseline.neon‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2875,26 +2875,6 @@ parameters:
28752875
count: 3
28762876
path: libraries/classes/Database/Routines.php
28772877

2878-
-
2879-
message: "#^Method PhpMyAdmin\\\\Database\\\\Search\\:\\:getSearchSqls\\(\\) return type has no value type specified in iterable type array\\.$#"
2880-
count: 1
2881-
path: libraries/classes/Database/Search.php
2882-
2883-
-
2884-
message: "#^Property PhpMyAdmin\\\\Database\\\\Search\\:\\:\\$criteriaColumnName \\(string\\) in isset\\(\\) is not nullable\\.$#"
2885-
count: 1
2886-
path: libraries/classes/Database/Search.php
2887-
2888-
-
2889-
message: "#^Property PhpMyAdmin\\\\Database\\\\Search\\:\\:\\$criteriaColumnName \\(string\\) on left side of \\?\\? is not nullable\\.$#"
2890-
count: 1
2891-
path: libraries/classes/Database/Search.php
2892-
2893-
-
2894-
message: "#^Property PhpMyAdmin\\\\Database\\\\Search\\:\\:\\$criteriaTables type has no value type specified in iterable type array\\.$#"
2895-
count: 1
2896-
path: libraries/classes/Database/Search.php
2897-
28982878
-
28992879
message: "#^Property PhpMyAdmin\\\\Database\\\\Search\\:\\:\\$searchTypes type has no value type specified in iterable type array\\.$#"
29002880
count: 1

‎psalm-baseline.xml‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5467,31 +5467,18 @@
54675467
</PossiblyUndefinedArrayOffset>
54685468
</file>
54695469
<file src="libraries/classes/Database/Search.php">
5470-
<DeprecatedMethod occurrences="2">
5471-
<code>escapeString</code>
5472-
<code>escapeString</code>
5473-
</DeprecatedMethod>
5474-
<MixedArgument occurrences="4">
5470+
<InvalidPropertyAssignmentValue occurrences="1">
5471+
<code>array_intersect($_POST['criteriaTables'], $this-&gt;tablesNamesOnly)</code>
5472+
</InvalidPropertyAssignmentValue>
5473+
<MixedArgument occurrences="1">
54755474
<code>$column['Field']</code>
5476-
<code>$eachTable</code>
5477-
<code>$eachTable</code>
5478-
<code>$newSearchSqls['select_count']</code>
54795475
</MixedArgument>
5480-
<MixedAssignment occurrences="2">
5481-
<code>$eachTable</code>
5476+
<MixedAssignment occurrences="1">
54825477
<code>$this-&gt;searchTypeDescription</code>
54835478
</MixedAssignment>
5484-
<PropertyNotSetInConstructor occurrences="2">
5485-
<code>$criteriaColumnName</code>
5479+
<PropertyNotSetInConstructor occurrences="1">
54865480
<code>$searchTypeDescription</code>
54875481
</PropertyNotSetInConstructor>
5488-
<RedundantConditionGivenDocblockType occurrences="1">
5489-
<code>isset($this-&gt;criteriaColumnName)</code>
5490-
</RedundantConditionGivenDocblockType>
5491-
<RedundantPropertyInitializationCheck occurrences="2">
5492-
<code>$this-&gt;criteriaColumnName</code>
5493-
<code>null</code>
5494-
</RedundantPropertyInitializationCheck>
54955482
</file>
54965483
<file src="libraries/classes/Database/Triggers.php">
54975484
<DeprecatedMethod occurrences="4">

‎templates/database/search/main.twig‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@
7575
</fieldset>
7676
</form>
7777
<div id="togglesearchformdiv">
78-
<a id="togglesearchformlink"></a>
78+
<button id="togglesearchformlink" class="btn btn-primary my-1"></button>
7979
</div>
8080
<div id="searchresults"></div>
81-
<div id="togglesearchresultsdiv"><a id="togglesearchresultlink"></a></div>
81+
<div id="togglesearchresultsdiv"><button id="togglesearchresultlink" class="btn btn-primary"></button></div>
8282
<br class="clearfloat">
8383
{# These two table-image and table-link elements display the table name in browse search results #}
8484
<div id="table-info">

‎templates/database/search/results.twig‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,46 @@
11
<table class="table table-striped caption-top w-auto">
22
<caption class="tblHeaders">
3-
{{ 'Search results for "<em>%s</em>" %s:'|format(
4-
criteria_search_string,
3+
{{- 'Search results for "<em>%s</em>" %s:'|format(
4+
criteria_search_string|e,
55
search_type_description
6-
)|raw }}
6+
)|raw -}}
77
</caption>
88
{% for row in rows %}
99
<tr class="noclick">
1010
<td>
11-
{% set result_message %}
11+
{%- set result_message -%}
1212
{% trans %}
1313
%1$s match in <strong>%2$s</strong>
1414
{% plural row.result_count %}
1515
%1$s matches in <strong>%2$s</strong>
1616
{% endtrans %}
17-
{% endset %}
18-
{{ result_message|format(row.result_count, row.table)|raw }}
17+
{%- endset -%}
18+
{{- result_message|format(row.result_count, row.table|e)|raw -}}
1919
</td>
20-
{% if row.result_count > 0 %}
21-
{% set url_params = {
20+
{%- if row.result_count > 0 -%}
21+
{%- set url_params = {
2222
'db': db,
2323
'table': row.table,
2424
'goto': url('/database/sql'),
2525
'pos': 0,
2626
'is_js_confirmed': 0
27-
} %}
28-
<td>
29-
<a name="browse_search"
30-
class="ajax browse_results"
31-
href="{{ url('/sql', url_params) }}"
27+
} -%}
28+
<td><button
29+
class="btn btn-link p-0 ajax browse_results"
30+
data-href="{{ url('/sql', url_params) }}"
3231
data-browse-sql="{{ row.new_search_sqls.select_columns }}"
33-
data-table-name="{{ row.table }}">
34-
{% trans 'Browse' %}
35-
</a>
32+
data-table-name="{{ row.table }}">{% trans 'Browse' %}</button>
3633
</td>
37-
<td>
38-
<a name="delete_search"
39-
class="ajax delete_results"
40-
href="{{ url('/sql', url_params) }}"
34+
<td><button
35+
class="btn btn-link p-0 ajax delete_results"
36+
data-href="{{ url('/sql', url_params) }}"
4137
data-delete-sql="{{ row.new_search_sqls.delete }}"
42-
data-table-name="{{ row.table }}">
43-
{% trans 'Delete' %}
44-
</a>
38+
data-table-name="{{ row.table }}">{% trans 'Delete' %}</button>
4539
</td>
46-
{% else %}
40+
{%- else -%}
4741
<td></td>
4842
<td></td>
49-
{% endif %}
43+
{%- endif -%}
5044
</tr>
5145
{% endfor %}
5246
</table>

0 commit comments

Comments
 (0)