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

Fix espace query on DatabaseEngine (#1355) #1360

Closed
wants to merge 9 commits into from

Conversation

lguichard
Copy link
Contributor

This PR fix issue #1355

DatabaseEngine no longer espace query on SQL

The engine execute query without quote and generate syntax error on database server

Before

SELECT
  *
FROM
  `search_index`
WHERE
  `index` = "demostore_products"
  AND MATCH (`content`) against (email@* IN boolean mode)

After

SELECT
  *
FROM
  `search_index`
WHERE
  `index` = "demostore_products"
  AND MATCH (`content`) against ('"email@*"' IN boolean mode)

Copy link

vercel bot commented Dec 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lunar-docs ❌ Failed (Inspect) Dec 22, 2023 4:40pm

@glennjacobs glennjacobs added the bug Something isn't working label Dec 18, 2023
@glennjacobs glennjacobs added this to the v0.7 milestone Dec 18, 2023
@glennjacobs
Copy link
Contributor

Do we need the extra inverted commas? I assumed Laravel would handle that.

E.g. would this work?

fn ($query) => $query->whereFullText('content', '"'.$builder->query.'*"', ['mode' => 'boolean'])

@glennjacobs
Copy link
Contributor

Also, could you add a test to show it working?

@lguichard
Copy link
Contributor Author

Do we need the extra inverted commas? I assumed Laravel would handle that.

E.g. would this work?

fn ($query) => $query->whereFullText('content', '"'.$builder->query.'*"', ['mode' => 'boolean'])

No, me too ! Laravel don't espace, i needed to do before inject on whereFullText.

fn ($query) => $query->whereFullText('content', '"'.$builder->query.'*"', ['mode' => 'boolean'])

Works fine too. More clean

But i found a new error with "@email string => Syntax error or access violation: 1064 syntax error, unexpected FTS_TERM, expecting FTS_NUMB.

I had make a quick fix, but the issue is also maybe a mysql injection security report when boolean mode is active on Laravel ? I'm not found any espace on Illuminate/Database/Query/Grammars/MySqlGrammar.php

The code works in any case. I think no user will use the + and - operators in their search

fn ($query) => $query->whereFullText('content', '"'.$builder->query.'*"')

@alecritson alecritson changed the base branch from 0.7 to 0.8 January 11, 2024 10:03
@alecritson alecritson modified the milestones: v0.7, v0.8 Jan 11, 2024
@lguichard lguichard closed this Jan 17, 2024
@lguichard lguichard deleted the 0.7 branch January 17, 2024 22:25
@lguichard lguichard restored the 0.7 branch January 17, 2024 22:25
@lguichard lguichard deleted the 0.7 branch January 17, 2024 22:28
@lguichard lguichard restored the 0.7 branch January 18, 2024 17:45
@lguichard lguichard deleted the 0.7 branch January 18, 2024 17:45
@lguichard lguichard restored the 0.7 branch January 18, 2024 17:46
@lguichard
Copy link
Contributor Author

@alecritson i closed accidentally this issue, oups

@lguichard lguichard reopened this Jan 18, 2024
@lguichard lguichard closed this Jan 19, 2024
@lguichard lguichard deleted the 0.7 branch January 19, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants