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

Backslashes are not properly escaped in where/choose clause #319

Open
r-thomson opened this issue Sep 15, 2023 · 3 comments
Open

Backslashes are not properly escaped in where/choose clause #319

r-thomson opened this issue Sep 15, 2023 · 3 comments

Comments

@r-thomson
Copy link

build_where_clause() and build_choose_clause() both attempt to escape single quotes in the provided strings. However, they do not handle backslashes, which means it's possible to un-escape the single quote.

>>> build_where_clause(DisplayName=r"Smith's Shop")
"DisplayName = 'Smith\\'s Shop'"
>>> build_where_clause(DisplayName=r"Smith\'s Shop")
"DisplayName = 'Smith\\\\'s Shop'"

This results in an error from the QuickBooks API, and could also be used to inject additional clauses into the search query.

>>> Customer.filter(DisplayName=r"Smith\'s Shop", qb=qb)
quickbooks.exceptions.ValidationException: QB Validation Exception 4000: Error parsing query
@ej2
Copy link
Owner

ej2 commented Jan 3, 2024

The code in build_where_clause and build_choose_clause is not ideal. None of the filtering code does any sanitizing. That job is left up to the developer using this library. I will add a note in the readme.

If someone wants to work on a solution, I would be more than happy to get it merged in!

@r-thomson
Copy link
Author

For what it's worth, in the interim I have seen success performing escaping with .replace('\\', '\\\\').replace("'", "\\'"). Though that's probably not exhaustive, since I have yet to find a full list of control characters in the QBO docs.

Unfortunately, I think fixing this behavior would be a breaking change for this library, since anyone who was pre-escaping their text to work around this would be double-escaping.

@redblacktree
Copy link
Contributor

I'm having an issue with ampersand being properly escaped. I'm willing to work on a solution, but as @r-thomson points out, this may be a breaking change. I did find this list of "supported characters":

https://quickbooks.intuit.com/learn-support/en-us/help-article/account-management/acceptable-characters-quickbooks-online/L3CiHlD9J_US_en_US

Also, see stackoverlfow discussion: https://stackoverflow.com/questions/27693578/issues-with-special-characters-in-qbo-api-v3-net-sdk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants