Followup to #2159011. Search input with slashes is still not escaped properly, result in the notice:
Warning: preg_match(): Unknown modifier '(' in SearchApiHighlight->createExcerpt() (line 312 in /[...]/search_api/includes/processor_highlight.inc).
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | search_api-escape-input-2168713-14.patch | 947 bytes | mashermike |
| #9 | search_api-escape-input-2168713-9.patch | 933 bytes | rosinegrean |
| #1 | search_api-escape-input-2168713-1.patch | 1.29 KB | idebr |
Comments
Comment #1
idebr commentedAttached patch preg_quotes input with the '/'-delimiter
Comment #2
drunken monkeyThanks a lot for spotting and fixing this! This time, I also made sure that this really is all of it, so we'll hopefully have no more of these issues.
So, committed. Thanks again!
Comment #3
feyp commentedUnfortunately, patch #1 does not fix the issue completely. The problem is in this line:
$keys = implode('|', array_map('preg_quote', $keys, array('/')));What it probably should do: Apply preg_quote with second parameter '/' to every value of array $keys.
What it actually does: Apply preg_quote with second parameter '/' to the first value of array $keys and apply preg_quote without a second parameter to the rest of the values.
So the patch will work, if the slash to be escaped is in the first key, but not if it is e.g. in the second key or later.
Comment #4
drunken monkeyThanks for spotting that, I'm rather embarassed I didn't catch it myself!
Fix committed, I hope now it's finally permanently resolved.
Comment #6
drunken monkeyNeeds to be ported to D8. (
createExcerpt()still uses a key verbatim inpreg_match().)Comment #7
drunken monkeyComment #8
rosinegrean commentedComment #9
rosinegrean commentedComment #13
drunken monkeyI can reproduce the test fails locally, though I cannot see what could cause them, either. But the patch definitely breaks the highlight tests (though they complete normally for me, that part is strange of the test bot).
Comment #14
mashermike commentedThere was a typo in the variable name and the spaces were removed from around the $text.
Comment #15
rosinegrean commented@mashermike Thanks, i did not notice them.
Looks good now
Comment #17
drunken monkeyAh, yes, of course. Thanks a lot for spotting that, mashermike!
Looks great now. Committed.
Thanks again, everyone!