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

Issue: Using escaped strings is not working correctly #163

Closed
hypetsch opened this issue May 4, 2018 · 14 comments
Closed

Issue: Using escaped strings is not working correctly #163

hypetsch opened this issue May 4, 2018 · 14 comments
Labels

Comments

@hypetsch
Copy link

hypetsch commented May 4, 2018

In issue #65 some issues regarding the usage of escaped strings have been fixed. I am currently facing another issue in combination with escaped characters (backslashes and quotation marks).
TextParser.cs seems to not handle those scenarios correctly.
I have been able to reproduce the issue in unit tests:

[Fact]
public void ParseLambda_StringLiteral_Backslash()
{
    string expectedLeftValue = "Property1.IndexOf(\"\\\")";
    string expectedRightValue = "0";
    var expression = DynamicExpressionParser.ParseLambda(
        new[] { Expression.Parameter(typeof(string), "Property1") },
        typeof(Boolean),
        string.Format("{0} >= {1}", expectedLeftValue, expectedRightValue));

    string leftValue = ((BinaryExpression)expression.Body).Left.ToString();
    string rightValue = ((BinaryExpression)expression.Body).Right.ToString();
    Assert.Equal(typeof(Boolean), expression.Body.Type);
    Assert.Equal(expectedLeftValue, leftValue);
    Assert.Equal(expectedRightValue, rightValue);
}

[Fact]
public void ParseLambda_StringLiteral_QuotationMark()
{
    string expectedLeftValue = "Property1.IndexOf(\"\\\"\")";
    string expectedRightValue = "0";
    var expression = DynamicExpressionParser.ParseLambda(
        new[] { Expression.Parameter(typeof(string), "Property1") },
        typeof(Boolean),
        string.Format("{0} >= {1}", expectedLeftValue, expectedRightValue));

    string leftValue = ((BinaryExpression)expression.Body).Left.ToString();
    string rightValue = ((BinaryExpression)expression.Body).Right.ToString();
    Assert.Equal(typeof(Boolean), expression.Body.Type);
    Assert.Equal(expectedLeftValue, leftValue);
    Assert.Equal(expectedRightValue, rightValue);
}

I think the issue is that the current implementation assumes that it is always a full string (starting and ending with an escaped quotation mark) and not only a single escaped character.

Edit: Updated unit tests code

@StefH StefH changed the title Using escaped strings is not working correctly Issue: Using escaped strings is not working correctly May 4, 2018
@StefH StefH added the bug label May 4, 2018
@StefH StefH removed the bug label May 11, 2018
@ghost
Copy link

ghost commented Apr 16, 2019

Hey, I got this issue again but only for backslashes, when switching from the NuGet package

wilx.System.Linq.Dynamic.Core 

(https://www.nuget.org/packages/wilx.System.Linq.Dynamic.Core/)
to

System.Linq.Dynamic.Core (Version 1.0.12)

I think the later one should be used?

Error:

System.Linq.Dynamic.Core.Exceptions.ParseException: 'Unterminated string literal'

Console app code to reproduce:

using System;
using System.Linq;
using System.Linq.Dynamic.Core;

namespace Test
{
    class Program
    {
        static void Main(string[] args)
        {
            var strings = new[] { "test \\ string" }.AsQueryable();

            var count = strings.Count("ToString().Contains('\\')");
            //var count = strings.Count(s => s.ToString().Contains('\\'));

            Console.WriteLine("Elements found (should be 1): " + count);
            Console.ReadKey();
        }
    }
}

@StefH
Copy link
Collaborator

StefH commented Apr 16, 2019

As a workaround you can use:
var count = strings.Count("Contains(@0)", "\\");

@ghost
Copy link

ghost commented Apr 16, 2019

Thank you for the answer, this worked for me in the given example. At a different point we use an input string to filter out elements, so it would be quite complicated to get it to work that way, having multiple inputs for each argument in the string predicate, as well as that filter string containing the arguments.

Is there a plan to fix this in the future?

Thank you for your help!

@StefH
Copy link
Collaborator

StefH commented Apr 17, 2019

I see your point, it seems that the parsing code in this library is different regarding escaping. I'll take a look if I can find and fix the logic.

@StefH
Copy link
Collaborator

StefH commented Apr 17, 2019

@hypetsch and @SolidWhiteSven
I've changed the code, please try version 1.0.13-ci-11242 from MyGet.

https://www.myget.org/feed/system-linq-dynamic-core/package/nuget/System.Linq.Dynamic.Core

@ghost
Copy link

ghost commented Apr 23, 2019

Hey @StefH, I've added the prerelease package with version 1.0.13-ci-11244 to the solution, but sadly I still get the 'Unterminated string literal' exception. I also thought the changes were only in the mentioned 11242 version, but that one also throws the error for this code:

using System;
using System.Linq;
using System.Linq.Dynamic.Core;

namespace Test
{
    class Program
    {
        static void Main(string[] args)
        {
            var strings = new[] { "test \\ string" }.AsQueryable();

            var count = strings.Count("Contains('\\')");

            Console.WriteLine("Elements found (should be 1): " + count);
            Console.ReadKey();
        }
    }
}

@StefH
Copy link
Collaborator

StefH commented Apr 23, 2019

You are correct, this test does not work. I've to double check the code again...

@StefH
Copy link
Collaborator

StefH commented Apr 27, 2019

@SolidWhiteSven
Can you try 1.0.13-ci-11260 ?

@ghost
Copy link

ghost commented Apr 29, 2019

@StefH Works like a charm, thank you!

@StefH
Copy link
Collaborator

StefH commented Apr 29, 2019

@hypetsch Can you also do some tests please?

@hypetsch
Copy link
Author

hypetsch commented Apr 30, 2019

@StefH, thanks for your work. Just did a quick test: the test case ParseLambda_StringLiteral_Backslash() I added in the first comment still fails with a ParseException ('Unterminated string literal'). Think the test itself should be ok?

@StefH
Copy link
Collaborator

StefH commented Apr 30, 2019

@hypetsch
Yes I noticed that also, so I changed the unit-test to be :

string expectedLeftValue = "Property1.IndexOf(\"\\\\\")";

Which means (to my knowledge):

@hypetsch
Copy link
Author

Sorry, think you are right.
Some other tests I did meanwhile worked well. Thanks.

@StefH
Copy link
Collaborator

StefH commented Apr 30, 2019

Thank you both for your help !

I will close this issue and merge the PR; in some time, I'll create a new official NuGet.

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