Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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 CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: rosinegrean commentedComment #9
rosinegrean CreditAttribution: 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 CreditAttribution: mashermike at Genuine commentedThere was a typo in the variable name and the spaces were removed from around the $text.
Comment #15
rosinegrean CreditAttribution: 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!