Core: "@" and "*" crashing forum if searched

[ardacebi]

Bug Report

Current Behavior
I know, this is not something that prevents usability, but it crashes the forum: if I search for @ or * using the search bar, the forum will crash until the ?q=%40 or ?q=* gets deleted from the URL bar.

Steps to Reproduce

1. Go to homepage
2. Search for @ or *
3. See error

Expected Behavior
The forum should really search for articles including those characters.

Screenshots

After searching for "@":

After searching for "*":

After refreshing the website without deleting the search query from URL:

Environment
Flarum Community
https://discuss.flarum.org

Additional Context
I only found these for now, but we may have other characters doing the same (or worse).

[ardacebi]

The issue is probably the "*" as "@" already gets replaced by it:
https://github.com/flarum/core/blob/master/src/Discussion/Search/Gambit/FulltextGambit.php#L34

[franzliedke]

As far as I can tell, the problem is the "*" character, which is a wildcard in MySQL's fulltext search. And that wildcard needs to have a "real" prefix.

So we should ensure that searches do not start with the "*" character. How should Flarum behave in that case, though?

[ardacebi]

I came up with something, before submitting the search, we can warn the user that the "*" character is removed from the search and after the user dismisses the pop-up, submit the search without the character.

The only solution that can be applied looks like this.

What do you think @franzliedke? Any other ideas?

[franzliedke]

Just returning an empty result set would be a possible solution as well.

[ardacebi]

Yes, pretty sure no one will use @ or * in their posts, so when the user submits a search, the core should detect the character and automatically return an empty result.

The pop-up idea is not that great as this, it might take useless time.

[franzliedke]

It's probably not the best solution, though, as an empty result might be unexpected to users.

My preferred solution:

• The backend returns an empty result and an explanatory message (not an error, we don't want to scare the users) detailing why there are no results.
• This might be driver-specific (because in this case, it is a limitation of MySQL's fulltext engine).

[ardacebi]

The warning message seems to be a great fix. We can have a simple text saying the user that this is a technical problem and some kind of drop-down that explains with a text to advanced users.

So regular users won't be confused with technical details and advanced users would understand why we return no results.

[datitisev]

Talking about ignoring those characters, Github does something similar where it ignores some characters when you search, which can turn out to be quite annoying when looking for specific code, however I don't think it would affect many users here.

[ardacebi]

Even if you search something like backupGlobals="false", the words you need to find that piece of code are enough so it finds it. This will be the same for Flarum users, if a user searches for Bugs, suggestions, and feedback, Flarum will get only Bugs suggestions and feedback, and list everything related to it.

I don't think it would affect many users here.

1 comma wouldn't change the results if a forum is based on all commas. So it won't.

[luceos]

linking to #1355

[tobyzerner]

I think an empty result set is reasonable. No need for a special message - it's not worth the extra translation. Searching for * is equivalent to searching for nothing. https://github.com/search?q=%2A&type=Code

[franzliedke]

I'd consider it going the extra mile in terms of user-friendliness, to avoid confusion (and bug reports ^^). But yeah, probably not important enough at this point.

[tobyzerner]

Not even google goes that extra mile: https://www.google.com/search?q=*
I don't think any user will expect to have meaningful search results when they search for just *.

[franzliedke]

So all that needs to be done here is the following, right?

• Remove leading * characters from the search term.
• Return an empty result set when the search term is an empty string.

[ardacebi]

@franzliedke I actually have a better solution: for Flarum to give users accurate and better search results, let's do what Discourse is doing:

I know we agreed on something, but this seems like a better solution.

[franzliedke]

I just came across an error caused by this bug on discuss.flarum.org, so I went ahead and fixed this for all the special search characters out there.

Reference:

• https://dev.mysql.com/doc/refman/5.7/en/fulltext-boolean.html
• https://stackoverflow.com/questions/26507087/escape-string-for-use-in-mysql-fulltext-search