Problem/Motivation
When you enter some partially successful search query, dozens of the following warning raises:
Warning: strpos(): Empty delimiter in search_simplify_excerpt_match() (line 1315 of /www/modules/search/search.module).
There are some public examples indexed by google as well: http://www.google.com/search?gcx=c&ie=UTF-8&q=Empty+delimiter+in+search_...
The error message says that second parameter of the strpos is null. This happen only under certain circumstances. After further research I found that more than one search keyword is needed because the affected code is in the part which cares about marking keywords in the resulting items. So it happens only when some results are found and at least one keyword is null or empty array after simplification. So the following search queries trigger this error:
- keyword OR 0
- keyword AND _
- keyword AND _OR 0
Searching for the "empty keywords" individually results in "at least one positive keyword with 3 characters or more" form validation error so the bug doesn't appear.
Proposed resolution
Solution is skip the search_simplify_excerpt_match() when simplified keyword or text are empty.
Remaining tasks
Write the test.Fix in Drupal 8.Fixed!Fix in Drupal 7.Fixed!
User interface changes
None.
API changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 1335388-13_search_empty_delimiter.patch | 2.37 KB | wojtha |
| #4 | 1335388-4_search_empty_delimiter.patch | 2.41 KB | wojtha |
| #4 | 1335388-4_search_empty_delimiter_ONLY_TESTS.patch | 1.66 KB | wojtha |
| #1 | 1335388-1_search_empty_delimiter.patch | 750 bytes | wojtha |
Comments
Comment #1
wojtha commentedInitial fix for Drupal 7.
Comment #1.0
wojtha commentedIssue summary template
Comment #2
ParisLiakos commentedThanks wojtha!
This was killing me together with dblog....more than 150 entries/request (i use search module for related articles)
Comment #3
Digital Direct Security commentedThanks wojtha!!
www.digitaldirectsecurity.co.uk
Comment #4
wojtha commentedLets fix it in D8 first.
Patch "ONLY TESTS" contains only tests and should generate the Warnings.
Comment #6
wojtha commentedThis is ok, 1335388-4_search_empty_delimiter_ONLY_TESTS.patch was expected to fail - it shows the current issue.
Comment #7
SilviuChingaru commentedNice patch. Working well. Thank you!
Comment #7.0
SilviuChingaru commentedModifying the remaining tasks.
Comment #8
wojtha commentedRemoving "Needs test" since the test is present in #4.
Comment #9
wojtha commented#4: 1335388-4_search_empty_delimiter.patch queued for re-testing.
Comment #10
bvanmeurs commentedAlso the query 'pijler 0' issues this error. I just noticed that this happened on one of our sites, and it filled up my error log completely just with a couple of searches.
Comment #11
ParisLiakos commentedpretty straightforward i guess?
i think its RTBC
Comment #12
catchThe comments could use some improvement, but otherwise this looks fine to me I think.
This doesn't read quite right to me. Should it be "only of" instead of "only from".
This could be simplified (:p), something like "Return immediately if neither simplified key nor text have a positive value."?
Comment #13
wojtha commentedOk.
I've changed the comments:
Comment #14
wojtha commentedComment #15
catchMuch better, moving back to RTBC.
Comment #16
webchickLooks good here as well!
Committed and pushed to 8.x and 7.x.
Comment #16.0
webchickMarking "Needs tests" as done.
Comment #17
bvanmeurs commentedGreat work, thanks to all involved!
Comment #18.0
(not verified) commentedMarking as fixed for D7 and D8