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

Unable to escape backslashes correctly #545

Closed
MitchellVermaning opened this issue Oct 6, 2021 · 6 comments
Closed

Unable to escape backslashes correctly #545

MitchellVermaning opened this issue Oct 6, 2021 · 6 comments
Assignees
Labels

Comments

@MitchellVermaning
Copy link

1. Description

Hello! Thank you for sharing this powerful library.

When I try to use the DynamicExpressionParser.ParseLambda-function with a string value that includes escaped backslashes, it will not parse correctly, which yields unescaped output

2. Exception

There is no exception thrown. Using input

"\"\\\\192.168.1.1\\audio\\new\""

The output is:

\192.168.1.1udio
ew

Adding more slashes does not seem to help, for instance given this input

"\"\\\\192.168.1.1\\\audio\\new\""

the following output will be shown

\192.168.1.1\udio
ew

As you can see it seems impossible to not escape the \a and \n. It would be great if you could have a look at this!

3. Fiddle or Project

You can see the problem on the following dotnetfidle:
https://dotnetfiddle.net/URflOo

@StefH StefH added the bug label Oct 6, 2021
@StefH StefH self-assigned this Oct 6, 2021
@StefH
Copy link
Collaborator

StefH commented Oct 6, 2021

Good afternoon @MitchellVermaning ,

Thank you for this issue, and I think I did solve it.

Please review the PR or take a look at the test case:
https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/StringParser/test/System.Linq.Dynamic.Core.Tests/Parser/StringParserTests.cs#L66

#546

@MitchellVermaning
Copy link
Author

Hi @StefH, thanks for the quick response and fix! The current output (based on the tests) seems a lot better, but it seems there the result is still not entirely correct. In

[InlineData("\"\\\\192.168.1.1\\audio\\new\"", "\\192.168.1.1\\audio\\new")]

I would expect the asserted result to be \\\\192.168.1.1\\audio\\new. Currently, 4 backslashes (so 2 backslashes with escape-character) have the same output as 2 backslashes. That does not seem entirely correct to me.

Actually I think the method should, in the end, only remove the leading and trailing double quote?

@StefH
Copy link
Collaborator

StefH commented Oct 7, 2021

Good morning @MitchellVermaning,

I think it's correct, because in the source-string, you need to escape the \ using \\. So if you want 2 backslashes in a normal string, you also need to use \\\\. See https://dotnetfiddle.net/L5cqe9

The purpose of the StringParser is to parse the (escaped) string + remove the leading and trailing double quote.

@MitchellVermaning
Copy link
Author

But you are also escaping the string in your assertions. So this:

[InlineData("\"\\\\192.168.1.1\\audio\\new\"", "\\192.168.1.1\\audio\\new")]

actually means:

[InlineData(""\\192.168.1.1\audio\new\"", "\192.168.1.1\audio\new")]

and that seems incorrect to me. I mean there are definitely 2 leading slashes but only one remains. Also, it would be strange that \\\\ would yield the same result (\\) as \\ (see image). Don't you agree?

image

@StefH
Copy link
Collaborator

StefH commented Oct 7, 2021

Eureka ? !

It seems that just using Regex.Escape does the trick ?

Can you please verify the PR / uni tests?

@MitchellVermaning
Copy link
Author

Wow, looking at the PR, you've cleaned up quite a bit of code there. And the existing tests are not failing, that's great! Everything seems fine on this side. Thank you so much for the quick action. Really appreciated!

@StefH StefH closed this as completed Oct 8, 2021
MitchellVermaning added a commit to MitchellVermaning/workflow-core that referenced this issue Oct 13, 2021
**Describe the bug**
Hello! Thank you for sharing this powerful library.

Last week I have reported a bug in the Linq.Dynamic.Core package (zzzprojects/System.Linq.Dynamic.Core#545). The `DynamicExpressionParser.ParseLambda()` function was unable to escape backslashes correctly.

Can you please update this package to the newest version v1.2.13.

Thank you!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants