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

backport #1782 to 3.x #1785

Merged
merged 1 commit into from
Oct 28, 2019
Merged

backport #1782 to 3.x #1785

merged 1 commit into from
Oct 28, 2019

Conversation

mitsos1os
Copy link
Contributor

Backport #1782 to 3.x branch

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • Commit messages are following our guidelines

@dhmlau
Copy link
Member

dhmlau commented Oct 21, 2019

Changes are the same as 4.x. Let's wait for CI to run.

@dhmlau dhmlau added the community-contribution Patches contributed by community label Oct 21, 2019
@dhmlau
Copy link
Member

dhmlau commented Oct 21, 2019

@slnode test please

1 similar comment
@emonddr
Copy link
Contributor

emonddr commented Oct 22, 2019

@slnode test please

@dhmlau
Copy link
Member

dhmlau commented Oct 22, 2019

@strongloop/loopback-maintainers , could you please review this PR?

@dhmlau
Copy link
Member

dhmlau commented Oct 25, 2019

@raymondfeng @jannyHou, do you think it's ok to force merge this PR? See my above comment.

@dhmlau
Copy link
Member

dhmlau commented Oct 28, 2019

Confirmed with @jannyHou, it's ok to force merge.

@dhmlau dhmlau merged commit b925e8c into loopbackio:3.x Oct 28, 2019
dhmlau added a commit that referenced this pull request Oct 28, 2019
 * backport #1782 to 3.x (#1785) (Dimitris Halatsis)
@dhmlau
Copy link
Member

dhmlau commented Oct 28, 2019

[email protected] has been released.

@FedericoMassaioli
Copy link

FedericoMassaioli commented Sep 24, 2020

Hi, unfortunately this back-port broke like, nlike, ilike and nilike operators for SQL DBs (PostgreSQL in my case) as the usual query strings using %, like those in LB 3.x documentation are deemed invalid and 'sanitized', causing empty results in some queries.

@dhmlau
Copy link
Member

dhmlau commented Sep 24, 2020

@FedericoMassaioli, sorry to hear that. Could you please open a new issue to discuss? Thanks.

@dhmlau
Copy link
Member

dhmlau commented Sep 24, 2020

@mitsos1os, I wonder how it can slip through CI (looking at the tests in CI, they seemed to be passing as well). Could you please take a look? Thanks.

@FedericoMassaioli
Copy link

FedericoMassaioli commented Sep 24, 2020

@dhmlau I opened #1869 per your request.
The very presence of () parentheses or other characters deemed invalid by the escaping function cause a modification of the string and the query to fail.
Maybe sanitization should be connector specific.

@mitsos1os
Copy link
Contributor Author

@dhmlau I am not sure how it slipped through... Yes I will take a look and let you know under issue #1869

@FedericoMassaioli
Copy link

FedericoMassaioli commented Sep 25, 2020

@dhmlau the case of an SQL like query containing characters that trigger escapeRegEx() is not covered in the tests in b925e8c.

It also looks like LB3.x unit tests only exercise memory, mock, and test connectors, no SQL connector. The SQLite 3 connector configured to use memory could be good enough to these purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution Patches contributed by community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants