System.Linq.Dynamic.Core icon indicating copy to clipboard operation
System.Linq.Dynamic.Core copied to clipboard

Parser interpreting the two consecutive escape sequences \"\" as a single escape sequence

Open brettwinters opened this issue 1 year ago • 11 comments

Don't ask me why I'm doing this (but the output needs to be escaped json) but when I do the following:

DynamicExpressionParser
    .ParseLambda(typeof(object), "\"{\\\"PropertyA\\\":\\\"\\\"}\"")
    .Compile()
    .DynamicInvoke()

The parser outputs: "{\"PropertyA\":\"}" vs "{\"PropertyA\":\"\"}" expected

The only thing that seems to work is if the string is as follows "\"{\\\\\\\"PropertyA\\\\\\\":\\\\\\\"\\\\\\\"}\""

Note: Parsing "\"{\\\"PropertyA\\\":\\\"abc\\\"}\"" and "\"{\\\"PropertyA\\\":\\\" \\\"}\"" (space) works correctly

brettwinters avatar Apr 01 '24 14:04 brettwinters

#163 #307

brettwinters avatar Apr 01 '24 14:04 brettwinters

BTW I'm using System.Linq.Dynamic.Core v1.3.10

brettwinters avatar Apr 03 '24 12:04 brettwinters

#788

StefH avatar Apr 05 '24 14:04 StefH

@brettwinters Can you please test preview version 1.3.11-preview-01?

From: https://www.myget.org/F/system-linq-dynamic-core/api/v3/index.json

StefH avatar Apr 08 '24 11:04 StefH

Hi @StefH - I'm having some trouble using myget. Sorry about this but would you mind publishing to nuget? otherwise please could you point me to a online c# ide so I can write some tests?

brettwinters avatar Apr 09 '24 16:04 brettwinters

This page describes how to use myget: https://github.com/WireMock-Net/WireMock.Net/wiki/MyGet-preview-versions

StefH avatar Apr 09 '24 16:04 StefH

ok, got it.

When I run these tests using 1.3.10:

[Theory]
[InlineData("\"{\\\"PropertyA\\\":\\\"abc\\\"}\"", "{\"PropertyA\":\"abc\"}")]
[InlineData("\"{\\\"PropertyA\\\":\\\" \\\"}\"", "{\"PropertyA\":\" \"}")]
[InlineData("\"{\\\"PropertyA\\\":\\\"\\\"}\"", "{\"PropertyA\":\"\"}")]
[InlineData("\"{\\\\\\\"PropertyA\\\\\\\":\\\\\\\"\\\\\\\"}\"", "{\\\"PropertyA\\\":\\\"\\\"}")]
public void GivenJsonString_WhenParseLambda_ThenShouldOutputEscapedJson(
	string jsonString,
	string expectedResult)
{
	var actualResult = DynamicExpressionParser
		.ParseLambda(typeof(object), jsonString)
		.Compile()
		.DynamicInvoke();
	
	Assert.Equal(
		expectedResult,
		actualResult?.ToString()
	);
}

They fail as originally described: image

Then when I run the same tests using 1.3.11-preview-01

image

All pass!

Thank you very much for your quick fix, I'll look out for 1.3.11

brettwinters avatar Apr 09 '24 17:04 brettwinters

@brettwinters Some advice is needed.

I did introduce a new config enum like below to make sure that this solution is now the default, but if users want to use the old parsing, they can configure it.

Do you think it's a good naming?

namespace System.Linq.Dynamic.Core.Config;

/// <summary>
/// Defines the types of string literal parsing that can be performed.
/// </summary>
public enum StringLiteralParsingType : byte
{
    /// <summary>
    /// Represents the default string literal parsing type. Double quotes should be escaped using the default escaping.
    /// E.G. var expression = "StaticHelper.Filter(\"UserName == \\\"x\\\"\")";
    /// 
    /// [Default]
    /// </summary>
    Default = 0,

    /// <summary>
    /// Represents a string literal parsing type where a double quotes should be escaped by two double quotes.
    /// E.G. var expression = "StaticHelper.Filter(\"UserName == "\"\"x\"\"\")";
    /// </summary>
    EscapeDoubleQuoteByTwoDoubleQuotes = 1
}

StefH avatar Apr 09 '24 19:04 StefH

Hi @StefH - escaping and unescaping serialised strings get really confusing quickly haha

The rule I think is that each pair of backslashes \\ turns into a single backslash \ and \" turns into " right? so combining, like this \\\" turns into \" (i.e. " when deserialised again into memory).

So \"UserName == \\\"x\\\"\" -> "UserName == \"x\"" (i.e. UserName == "x" in memory)

But the EscapeDoubleQuoteByTwoDoubleQuotes \"UserName == "\"\"x\"\"\" -> "UserName == "x"" (i.e. UserName == x" in memory) which to me looks wrong so I'd call it "Legacy" with an [Obsolete] attribute but I'm not sure who depends on this behaviour...

brettwinters avatar Apr 10 '24 13:04 brettwinters

wait I just confused myself. You're right \"UserName == "\"\"x\"\"\" -> UserName == "x" - it's totally correct and valid since it's another method of escaping: Escape with backslash (\" and \\) vs escape with double quotes ("" i.e. ""abc"") (so by default EscapeWithBackslash and option EscapeWithDoubleQuotes. In the second example you're using both methods. Can you combine the parser logic to handle both cases?

brettwinters avatar Apr 10 '24 14:04 brettwinters

The parser cannot handle both cases automatically. You need to use the config.

(I did not notice the error in the example, I will verify it..)

StefH avatar Apr 10 '24 14:04 StefH