Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Searching for "orange" will find "angel" in non-fulltext search #2349

Open
Destroy666x opened this issue Feb 28, 2016 · 16 comments · May be fixed by #4707
Open

Searching for "orange" will find "angel" in non-fulltext search #2349

Destroy666x opened this issue Feb 28, 2016 · 16 comments · May be fixed by #4707
Labels
b:1.8 Branch: 1.8.x p:medium Priority: Medium. Issues to be resolved with normal preference s:confirmed Status: Confirmed. Retested and found the issue exists t:bug Type: Bug. An issue causing error / flaw / malfunction

Comments

@Destroy666x
Copy link
Contributor

Non-Fulltext-Search in MyBB ignores "and" and "or" of the first entered keyword of the search.
If I search for "orange", MyBB will actually search for "ange". I can't search for "andi", because Mybb will complain that my search keyword is only one character long.

The reason is a bug in clean_keywords function:

https://github.com/mybb/mybb/blob/featur...h.php#L243
https://github.com/mybb/mybb/blob/featur...h.php#L248

At these lines, "and" and "or" will be removed from the search string, if they are at the beginning of the first keyword.

I suggest removing the two "if"-statements completely, as they are broken in many ways. Not only will they currently unintentionally remove the first characters of a matching keyword, they also don't protect against "and" and "or" keywords at the first position of a search query (the intention of these lines) if fixed to not cut off longer keywords:

"or test" would become "test" (correct)
"and test" would become "test" (correct)
"or or test" would become "or test" (wrong)
"and and test" would become "and test" (wrong)
"or and test" would become "test" (correct)
"and or test" woud become "or test" (wrong)
and so on ...

Original thread: Searching for "orange" will find "angel" in non-fulltext search

@Destroy666x Destroy666x added t:bug Type: Bug. An issue causing error / flaw / malfunction s:confirmed Status: Confirmed. Retested and found the issue exists b:1.8 Branch: 1.8.x p:medium Priority: Medium. Issues to be resolved with normal preference labels Feb 28, 2016
@Destroy666x Destroy666x added this to the 1.8.7 milestone Feb 28, 2016
@Destroy666x
Copy link
Contributor Author

I guess something like:

$stop = false;
while($stop == false)
{
    if(substr($keywords, 0, 3) == 'or ')
    {
        $keywords = substr($keywords, 3);
    }
    elseif(substr($keywords, 0, 4) == 'and ')
    {
        $keywords = substr($keywords, 4);
    }
    else
    {
        $stop = true;
    }
}

would work, but then I'm not entirely sure what's the purpose of it (what exactly does it prevent?).

@Stefan-MyBB
Copy link
Contributor

and and or are keywords and can't be used as search term. This code is needed because the regex to split the search term doesn't find and and or in the beginning: #\s(and|or)\s#
Your suggestion would not catch a simple and. I'd probably use a regex here, something like:

$keywords = preg_replace('#^(and|or)(\s|$)((and|or)\s)*#', '', $keywords);

@Destroy666x
Copy link
Contributor Author

Ah yes, forgot about single keyword.

Are those MyBB-specific keywords? What's their function or rather usage (it could be documented somewhere)?
EDIT: nvm, figured it's keyword or keyword and keyword and keyword, but I'm still not sure how's a regular admin/user supposed to know about this functionality.
As for regex, this is slightly simplier:

$keywords = preg_replace('/^((and|or)(\s|$))+/', '', $keywords);

@Destroy666x Destroy666x self-assigned this Feb 28, 2016
@Stefan-MyBB
Copy link
Contributor

While a regular user doesn't need to know, searching for and or or would be quite useless given to the fact both words are that common. And you can still search for it in a phrase: "foo and bar"

btw: These keywords and search expressions are quite common. Fulltext search supports some more things, too.

@Destroy666x
Copy link
Contributor Author

While a regular user doesn't need to know

Noone needs to know, but then what's the point of having an unused feature?

I think it wouldn't hurt to add some tips under the keyword textbox: http://community.mybb.com/search.php depending on the search type. I'd do that in the PR I'm just preparing but then I'm not sure of all tricks myself. Could you list them?

@Stefan-MyBB
Copy link
Contributor

It depends whether fulltext search is used or not.
Both search systems support AND, OR, *, and phrases ("foo bar"), fulltext additionally allows NOT, +, -, ~, <, >, @ and parentheses groups (explanations and examples: http://dev.mysql.com/doc/refman/5.6/en/fulltext-boolean.html)

@Destroy666x
Copy link
Contributor Author

Doesn't regular search also support *?

@Stefan-MyBB
Copy link
Contributor

You are right. I've missed that.

Destroy666x added a commit to Destroy666x/mybb that referenced this issue Feb 28, 2016
As the title says. Also added advcanced mechanics descriptions in
search.php.

mybb#2349
@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.7, 1.8.8 Mar 2, 2016
@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.8, 1.8.9 Aug 31, 2016
@JN-Jones
Copy link
Contributor

@Destroy666x You seem to have made a commit which seems to fix this but never created a PR with it? Is there anything missing?

@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.9, 1.8.10 Dec 15, 2016
@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.10, 1.8.11 Jan 8, 2017
@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.11, 1.8.10 Jan 8, 2017
@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.11, 1.8.12 Apr 1, 2017
@Stefan-MyBB Stefan-MyBB modified the milestone: 1.8.12 May 22, 2017
@Eldenroot
Copy link
Contributor

@Destroy666x @effone @Shade- and others devs - there is a FIX from Destroy in his repo, can you check if it is fine? I am trying to test it on my board, seems fine but needs proper testing. I can recreate the PR if it will be fine. Let me know, thank you!

@BettyIsBoop
Copy link

BettyIsBoop commented Apr 30, 2023

@Destroy666x @effone @Shade- and others devs - there is a FIX from Destroy in his repo, can you check if it is fine? I am trying to test it on my board, seems fine but needs proper testing. I can recreate the PR if it will be fine. Let me know, thank you!

It broke SQL request with ' inside, no SQL filter. Maybe there are security issue : tested with Cahiers d'Ulysse

This one is OK #2349 (comment) and really more simple.

I think its the best fix.

@Eldenroot
Copy link
Contributor

Hmm, maybe some discussion?

@BettyIsBoop
Copy link

Hmm, maybe some discussion?

PS : #4532 but still not fixed in feature :

if(my_strpos($keywords, "or") === 0)

@Sama34
Copy link
Contributor

Sama34 commented May 27, 2023

If users can't use these words (and, or) we could add some warning while typing (there is no such feature altogether as of now).

Regardless of ultimate fix regarding the front-end some in-line form verification should be added at some point and consider this specific case if so.

BettyIsBoop added a commit to BettyIsBoop/mybb that referenced this issue May 27, 2023
@BettyIsBoop BettyIsBoop linked a pull request May 27, 2023 that will close this issue
@BettyIsBoop
Copy link

If users can't use these words (and, or) we could add some warning while typing (there is no such feature altogether as of now).

It's not THIS word, it's allw aord styarting by or and and …

orange for example

Fixed by a simple edit #4707

@BettyIsBoop
Copy link

@Destroy666x @effone @Shade- and others devs - there is a FIX from Destroy in his repo, can you check if it is fine? I am trying to test it on my board, seems fine but needs proper testing. I can recreate the PR if it will be fine. Let me know, thank you!

It broke SQL request with ' inside, no SQL filter. Maybe there are security issue : tested with Cahiers d'Ulysse

I can not reproduce with the fix and search "Cahiers d'Ulysse" return Cahiers d'Ulysse topics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:1.8 Branch: 1.8.x p:medium Priority: Medium. Issues to be resolved with normal preference s:confirmed Status: Confirmed. Retested and found the issue exists t:bug Type: Bug. An issue causing error / flaw / malfunction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@Stefan-MyBB @Eldenroot @Sama34 @JN-Jones @Destroy666x @BettyIsBoop and others