-
-
Notifications
You must be signed in to change notification settings - Fork 231
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
Found problem with backslashes parsing #307
Comments
Hello @StefH, I just ran into this issue as well. The filter Are there any workaround for parsing a dynamic linq lambda which contains a quote, other than substitutions? If not, would you please be able to point me towards this area in the code? I might be able to provide a PR, with some guidance. |
@StefH I was able to work around this by running an ExpressionVisitor on the parsed lambda which simply calls Regex.Unescape() on all string constants. I wonder if something this simple would solve this issue? It ensures that all escape sequences inside string literals are parsed as escape sequences, not the literal characters. Do you think this is too drastic to apply everywhere? The regex unescaping ensures that the expression |
Please make a PullRequest with unit tests or a new sample project. |
Test: #325 |
@hypetsch and @SolidWhiteSven : you also where involved in string parsing issues, can you also verify if this fix is also in line with your usage? |
Hey, I've updated to 1.0.20-ci-12230 and all my tests were successful. |
@SolidWhiteSven The version you mention is a different one, you need |
Ah, I see, thought the changes were also included there. |
Closing as solved in PR #326 |
@alexweav --> Can you please test if version System.Linq.Dynamic.Core.1.0.20-ci-12219.nupkg from MyGet solves your issue? |
Hi, testing this version, the problem persists. query.Where("name.contains(" \ ")") , I´ve got System.Linq.Dynamic.Core.Exceptions.ParseException: 'Syntax error ' \ ' '. Can you help me with this? |
@rppmartinsCollab To be able to use Contains to check if a string contains a query.Where("name.contains(\"\\\")")
Does this work for you? |
Hey, thanks for the response. This way I get the error "Syntax error ' \\ ' " once TextParser does not recognize the character. |
@rppmartinsCollab Sorry for the confusion, you actually need to escape the |
The problem seems to be in scaping the quote character, I get error in \" |
Hello guys, I've just upgraded it to 1.0.21 via nuget and I can confirm that the issue still exists. It only breaks if the filter is a check for a single backslash e.g. #1 if I search for
e.g. #2 if I search for
e.g. #3 if I search for System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.Linq.Dynamic.Core.Exceptions.ParseException: Unterminated string literal |
@silarmani Did you use latest from normal NuGet ? Can you please try version System.Linq.Dynamic.Core.1.0.20-ci-12980.nupkg from MyGet ? |
@StefH, yes, it does, as long as I escape all backlashes, which makes sense. For clarity for everyone I am going to repeat all the scenarios I ran before but with this special version and then show the new queries in EF e.g. #1 if I search for
e.g. #2 if I search for
e.g. #3 if I search for
|
I have also tested permutations using backslashes and double quotes and they all work flawlessly. Is there a plan for making it available in the next release via NuGet? |
e.g. #4 if I search for
|
@silarmani Thanks for extended testing! I want to release this to normal NuGet, however I think I need to change the version from this library to 1.1 because it can be a breaking change. Also I'm doubting if I should add a new dependency ( |
Yes, it is breaking change. I would changed it to 1.1. e.g. the syntax for searching for In regards to adding a new dependency, I don't see it as a problem. I'll stick with the pre-release for now since it doesn't break anything so far. Thank you!!! |
External dependency to Sprache which was introduced is reverted. New version (1.1) will be released in some days. |
You closed my previous issue as a question so I'm opening a new one.
I think I found the problem in your parsing. I can't see the reason to do this in string literal parsing except the case when I try to find " in the result.
Because if I use query
"contains(\"\\\")"
it fails with unterminated literal, but if I change query to"contains(\"\\a\")"
it works finding "\a" string. And also if I use"contains(\"\\\\\")"
it works with string "\\" (string with two real slashes).So I think the correct solution will be to remove this if statement and just replace
\"
to"
in the result token if it was a string literal. In this case if user will want to put " in query string it should be escaped inside this string. At least it will not break any existing code.The better approach will be (in my opinion) to escape all the characters in the string by user and to unescape by parser. That means
\\\\
will evaluate to\
and not to\\
.I understand your workaround by using
@0
but it's not the case when search string comes outside my app.The text was updated successfully, but these errors were encountered: