Skip to content

Commit 01b4082

Browse files
nilmergnilmerg
authored
SearchBar: Accept otherwise invalid input as quick search expression (#353)
Noticed a few other glitches regarding validation, and needed to enhance it anyway since we have to make sure we don't use a valid expression as quick filter. resolves #352
2 parents 13513a3 + 91221e2 commit 01b4082

5 files changed

Lines changed: 204 additions & 25 deletions

File tree

‎src/Compat/SearchControls.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ public function createSearchBar(Query $query, ...$params): SearchBar
7676
$searchBar->setRedirectUrl($redirectUrl);
7777
$searchBar->setAction($redirectUrl->getAbsoluteUrl());
7878
$searchBar->setIdProtector([$this->getRequest(), 'protectId']);
79+
$searchBar->setSearchColumns($query->getModel()->getSearchColumns());
7980
$searchBar->addWrapper(Html::tag('div', ['class' => 'search-controls']));
8081

8182
$moduleName = $this->getRequest()->getModuleName();
@@ -118,7 +119,13 @@ public function createSearchBar(Query $query, ...$params): SearchBar
118119
}
119120

120121
if (isset($definition)) {
121-
$column->setLabel($definition->getLabel());
122+
if (! $definition->getName()) {
123+
// Happens in case the searchPath is considered a valid relation, but without a column name
124+
$column->setMessage(t('Is not a valid column'));
125+
$column->setSearchValue($searchPath);
126+
} else {
127+
$column->setLabel($definition->getLabel());
128+
}
122129
}
123130
};
124131

‎src/Control/SearchBar.php‎

Lines changed: 97 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
use ipl\Web\Control\SearchBar\Terms;
1616
use ipl\Web\Control\SearchBar\ValidatedColumn;
1717
use ipl\Web\Control\SearchBar\ValidatedOperator;
18+
use ipl\Web\Control\SearchBar\ValidatedTerm;
1819
use ipl\Web\Control\SearchBar\ValidatedValue;
1920
use ipl\Web\Filter\ParseException;
21+
use ipl\Web\Filter\Parser;
2022
use ipl\Web\Filter\QueryString;
2123
use ipl\Web\Url;
2224
use ipl\Web\Widget\Icon;
@@ -53,6 +55,9 @@ class SearchBar extends Form
5355
/** @var string */
5456
protected $searchParameter;
5557

58+
/** @var string[] */
59+
protected array $searchColumns = [];
60+
5661
/** @var Url */
5762
protected $suggestionUrl;
5863

@@ -135,6 +140,30 @@ public function getSearchParameter()
135140
return $this->searchParameter ?: 'q';
136141
}
137142

143+
/**
144+
* Set the search columns to use
145+
*
146+
* @param string[] $columns
147+
*
148+
* @return $this
149+
*/
150+
public function setSearchColumns(array $columns): static
151+
{
152+
$this->searchColumns = $columns;
153+
154+
return $this;
155+
}
156+
157+
/**
158+
* Get the search columns in use
159+
*
160+
* @return string[]
161+
*/
162+
public function getSearchColumns(): array
163+
{
164+
return $this->searchColumns;
165+
}
166+
138167
/**
139168
* Set the suggestion url
140169
*
@@ -405,7 +434,7 @@ protected function assemble()
405434
if (isset($this->changes[1][$columnIndex])) {
406435
$change = $this->changes[1][$columnIndex];
407436
$condition->setColumn($change['search']);
408-
} elseif (empty($this->changes)) {
437+
} else {
409438
$column = ValidatedColumn::fromFilterCondition($condition);
410439
$operator = ValidatedOperator::fromFilterCondition($condition);
411440
$value = ValidatedValue::fromFilterCondition($condition);
@@ -449,25 +478,80 @@ protected function assemble()
449478
try {
450479
$filter = $parser->parse();
451480
} catch (ParseException $e) {
452-
$charAt = $e->getCharPos() - 1;
481+
$charAt = $e->getCharPos();
453482
$char = $e->getChar();
454483

484+
if ($char === Parser::EOL) {
485+
$value = $q;
486+
$title = t('Unexpected end of input');
487+
$pattern = sprintf(
488+
ValidatedTerm::DEFAULT_PATTERN,
489+
ValidatedTerm::escapeForHTMLPattern($q)
490+
);
491+
} else {
492+
$value = substr($q, $charAt);
493+
$title = sprintf(t('Unexpected %s at start of input'), $char);
494+
$pattern = sprintf('^(?!%s).*', ValidatedTerm::escapeForHTMLPattern($char));
495+
496+
if ($charAt > 0) {
497+
try {
498+
$this->setFilter(QueryString::parse(substr($q, 0, $charAt)));
499+
} catch (ParseException) {
500+
$value = $q;
501+
$title = sprintf(t('Unexpected %s at position %d'), $char, $charAt + 1);
502+
$pattern = sprintf(
503+
ValidatedTerm::DEFAULT_PATTERN,
504+
ValidatedTerm::escapeForHTMLPattern($q)
505+
);
506+
}
507+
}
508+
}
509+
455510
$this->getElement($this->getSearchParameter())
456511
->addAttributes([
457-
'title' => sprintf(t('Unexpected %s at start of input'), $char),
458-
'pattern' => sprintf('^(?!%s).*', $char === ')' ? '\)' : $char),
512+
'title' => $title,
513+
'pattern' => $pattern,
459514
'data-has-syntax-error' => true
460515
])
461516
->getAttributes()
462-
->registerAttributeCallback('value', function () use ($q, $charAt) {
463-
return substr($q, $charAt);
517+
->registerAttributeCallback('value', function () use ($value) {
518+
return $value;
464519
});
465520

466-
$probablyValidQueryString = substr($q, 0, $charAt);
467-
$this->setFilter(QueryString::parse($probablyValidQueryString));
468521
return false;
469522
}
470523

524+
if (
525+
$this->getSearchColumns()
526+
&& $filter instanceof Filter\Condition
527+
// The parser yields a boolean but the validation may cast this to a string -.-
528+
&& ($filter->getValue() === '1' || $filter->getValue() === true)
529+
&& $filter->metaData()->has('invalidColumnMessage')
530+
) {
531+
// A single expression that's invalid and has only a truthy value can be safely
532+
// be transformed to a quick search
533+
$changes = [];
534+
$change = 0;
535+
$filter = Filter::any();
536+
foreach ($this->getSearchColumns() as $column) {
537+
$condition = Filter::like($column, "*$q*");
538+
$column = ValidatedColumn::fromFilterCondition($condition);
539+
$operator = ValidatedOperator::fromFilterCondition($condition);
540+
$value = ValidatedValue::fromFilterCondition($condition);
541+
$this->emit(self::ON_ADD, [$column, $operator, $value]);
542+
543+
$condition->setColumn($column->getSearchValue());
544+
$condition->setValue($value->getSearchValue());
545+
$filter->add($condition);
546+
547+
$changes[$change++] = $column->toTermData();
548+
$changes[$change++] = $operator->toTermData();
549+
$changes[$change++] = $value->toTermData();
550+
}
551+
552+
$invalid = false;
553+
}
554+
471555
$this->getElement($this->getSearchParameter())
472556
->getAttributes()
473557
->registerAttributeCallback('value', function () {
@@ -476,7 +560,11 @@ protected function assemble()
476560
$this->setFilter($filter);
477561

478562
if (! empty($changes)) {
479-
$this->changes = ['#' . $searchInputId, $changes];
563+
if (empty($this->changes)) {
564+
$this->changes = ['#' . $searchInputId, $changes];
565+
} else {
566+
$this->changes[1] += $changes;
567+
}
480568
}
481569

482570
return ! $invalid;

‎src/Control/SearchBar/ValidatedTerm.php‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,14 @@
66

77
abstract class ValidatedTerm
88
{
9-
/** @var string The default validation constraint */
10-
public const DEFAULT_PATTERN = '^\s*(?!%s\b).*\s*$';
9+
/**
10+
* The default validation constraint
11+
*
12+
* Forbids what's inside an input unless non-whitespace chars are prepended or appended.
13+
*
14+
* @var string
15+
*/
16+
public const DEFAULT_PATTERN = '(?!\s*%s\s*$).*';
1117

1218
/** @var string The search value */
1319
protected $searchValue;
@@ -155,7 +161,10 @@ public function getPattern(): ?string
155161
return null;
156162
}
157163

158-
return $this->pattern ?? sprintf(self::DEFAULT_PATTERN, $this->getLabel() ?: $this->getSearchValue());
164+
return $this->pattern ?? sprintf(
165+
self::DEFAULT_PATTERN,
166+
static::escapeForHTMLPattern($this->getLabel() ?: $this->getSearchValue())
167+
);
159168
}
160169

161170
/**
@@ -172,6 +181,18 @@ public function setPattern(string $pattern): self
172181
return $this;
173182
}
174183

184+
/**
185+
* Escape the given subject for usage inside an HTML pattern attribute
186+
*
187+
* @param string $subject
188+
*
189+
* @return string
190+
*/
191+
public static function escapeForHTMLPattern(string $subject): string
192+
{
193+
return preg_quote($subject);
194+
}
195+
175196
/**
176197
* Get this term's data
177198
*

‎src/Filter/Parser.php‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ class Parser
99
{
1010
use Events;
1111

12+
/** @var string Representation used to indicate end of input */
13+
public const EOL = 'EOL';
14+
1215
/** @var string Emitted for every completely parsed condition */
1316
public const ON_CONDITION = 'on_condition';
1417

@@ -136,7 +139,6 @@ protected function readFilters($nestingLevel = 0, $op = null, $filters = null, $
136139
$this->termIndex++;
137140
$next = $this->nextChar();
138141
if ($next !== false && ! in_array($next, ['&', '|', ')'])) {
139-
$this->pos++;
140142
$this->parseError($next, 'Expected logical operator');
141143
}
142144
}
@@ -196,7 +198,7 @@ protected function readFilters($nestingLevel = 0, $op = null, $filters = null, $
196198
continue;
197199
}
198200

199-
$this->parseError($next, "$op level $nestingLevel");
201+
$this->parseError($next, sprintf("%slevel %d", $op ? "$op " : '', $nestingLevel));
200202
} else {
201203
if ($isNone) {
202204
$isNone = false;
@@ -234,7 +236,6 @@ protected function readFilters($nestingLevel = 0, $op = null, $filters = null, $
234236
$this->termIndex++;
235237
$next = $this->nextChar();
236238
if ($next !== false && ! in_array($next, ['&', '|', ')'])) {
237-
$this->pos++;
238239
$this->parseError($next, 'Expected logical operator');
239240
}
240241
}
@@ -404,6 +405,14 @@ protected function readCondition()
404405
}
405406

406407
$condition = $this->createCondition($column, $operator, $value);
408+
if ($condition === null) {
409+
// Rewind to the value start for accurate error reporting
410+
$this->pos -= is_array($value)
411+
? strlen(implode('|', $value)) + 2 // Account for the parentheses
412+
: strlen($value);
413+
$this->parseError(null, 'Invalid operator in column expression');
414+
}
415+
407416
$condition->metaData()
408417
->set('columnIndex', $columnIndex)
409418
->set('operatorIndex', $operatorIndex)
@@ -550,18 +559,21 @@ protected function parseError($char = null, $extraMsg = null)
550559
$extra = ': ' . $extraMsg;
551560
}
552561

562+
$pos = $this->pos;
553563
if ($char === null) {
554564
if ($this->pos < $this->length) {
555565
$char = $this->string[$this->pos];
556566
} else {
557-
$char = $this->string[--$this->pos];
567+
$char = self::EOL;
558568
}
569+
} elseif (! isset($this->string[$pos]) || $this->string[$pos] !== $char) {
570+
$pos--;
559571
}
560572

561573
throw new ParseException(
562574
$this->string,
563575
$char,
564-
$this->pos,
576+
$pos,
565577
$extra
566578
);
567579
}

0 commit comments

Comments
 (0)