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.

Comments

wojtha’s picture

Version: 7.9 » 7.x-dev
Status: Active » Needs review
Issue tags: +Needs tests
StatusFileSize
new750 bytes

Initial fix for Drupal 7.

wojtha’s picture

Issue summary: View changes

Issue summary template

ParisLiakos’s picture

Thanks wojtha!
This was killing me together with dblog....more than 150 entries/request (i use search module for related articles)

Digital Direct Security’s picture

wojtha’s picture

Version: 7.x-dev » 8.x-dev
StatusFileSize
new1.66 KB
new2.41 KB

Lets fix it in D8 first.

Patch "ONLY TESTS" contains only tests and should generate the Warnings.

Status: Needs review » Needs work

The last submitted patch, 1335388-4_search_empty_delimiter_ONLY_TESTS.patch, failed testing.

wojtha’s picture

Status: Needs work » Needs review

This is ok, 1335388-4_search_empty_delimiter_ONLY_TESTS.patch was expected to fail - it shows the current issue.

SilviuChingaru’s picture

Nice patch. Working well. Thank you!

SilviuChingaru’s picture

Issue summary: View changes

Modifying the remaining tasks.

wojtha’s picture

Issue tags: -Needs tests

Removing "Needs test" since the test is present in #4.

wojtha’s picture

bvanmeurs’s picture

Priority: Normal » Major

Also 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.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

pretty straightforward i guess?
i think its RTBC

catch’s picture

Status: Reviewed & tested by the community » Needs work

The comments could use some improvement, but otherwise this looks fine to me I think.

+    // Test combination of the valid keyword and keyword which consists only
+    // from characters which are being truncated during simplification.

This doesn't read quite right to me. Should it be "only of" instead of "only from".

+  // Check if simplified key and text have some value. If one of them is empty
+  // skip this.

This could be simplified (:p), something like "Return immediately if neither simplified key nor text have a positive value."?

wojtha’s picture

StatusFileSize
new2.37 KB

Ok.

I've changed the comments:

  // Return immediately if simplified key or text are empty.
  // Test combination of the valid keyword and keyword containing only
  // characters which are being truncated during simplification.
wojtha’s picture

Status: Needs work » Needs review
catch’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs backport to D7

Much better, moving back to RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Looks good here as well!

Committed and pushed to 8.x and 7.x.

webchick’s picture

Issue summary: View changes

Marking "Needs tests" as done.

bvanmeurs’s picture

Great work, thanks to all involved!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Marking as fixed for D7 and D8