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

Move FTS query encoding to the core #7075

Closed
devos50 opened this issue Oct 11, 2022 · 0 comments · Fixed by #8006
Closed

Move FTS query encoding to the core #7075

devos50 opened this issue Oct 11, 2022 · 0 comments · Fixed by #8006
Assignees

Comments

@devos50
Copy link
Contributor

devos50 commented Oct 11, 2022

I noticed that we are encoding FTS queries in the GUI, and at some point we are even encoding them twice. This also means we are sending FTS-encoded queries as part of the request over the HTTP API. Even though this is all fine, we could consider deferring encoding FTS queries to the core.

The following reasons were given for encoding FTS queries on the GUI side:

  • To avoid sending invalid search queries to remote peers if a user typed an invalid query (like just a single "double quote" symbol)
  • To successfully handle the case of query injection in RemoteQueryCommunity when a malicious peer sends a query with a wrong syntax (for example, when a closing double quote is missed)
  • To be sure that the local search endpoint works in the same way as remote searches via RemoteQueryCommunity.

I think, however, that it is more clean to send the unencoded query over the HTTP API and invoke the to_fts_query before doing the search in the database, or just before sending out messages in the RemoteQueryCommunity. Doing it this way also shouldn't break compatibility with the RemoteQueryCommunity.

@devos50 devos50 added this to the Backlog milestone Oct 18, 2022
@drew2a drew2a modified the milestones: Backlog, 7.15.0 Apr 25, 2024
@drew2a drew2a self-assigned this Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants