ohler55 / ojg

Optimized JSON for Go
MIT License
839 stars 50 forks source link

Expr.String() doesn't escape strings #129

Closed paulo-raca closed 1 year ago

paulo-raca commented 1 year ago

This is a follow-up to https://github.com/ohler55/ojg/issues/123 (Thansks a lot by the way :pray:)

I want to build a JsonPath string given a path in my datastructure.

E.g., if I want a path to data["field1"][2]["😊"] I should get $.field1[2]['😊']

As you suggested, I'm building it as jp.R().Child("field1").N(2).Child("😊").String().

However looks like Child.String() has a few issues:

ohler55 commented 1 year ago

If you want the string to always be bracket format you can start with the jp.B() function.

I'm not sure why you want to force bracket as strictly as you suggest. If there is an issue such as ''' that is a valid issue. I'll look at that tonight or tomorrow. The output should switch to bracket format on new line though. Non-ascii does not need to be escaped though. Many languages use non-ascii but valid unicode.

paulo-raca commented 1 year ago

I'm not sure why you want to force bracket as strictly as you suggest

I don't really, but when the rules aren't clearly stated I am usually very conservative. I also misread the implementation and assumed it only checked for "." :man_facepalming:

Now that I'm looking at it again I can see I was wrong and it does check for line breaks and etc. Sorry! :man_facepalming:

There is, however, another issue: Child.tokenOK() should disallow empty strings

jp.R().Child("").Child("a").String() -> $..a

paulo-raca commented 1 year ago

I also was wrong when I said it doesn't escape strings, everything else I've throw at it seems to be properly escaped.

' was the first thing I tried and I apparently got lucky :sweat:

ohler55 commented 1 year ago

If you find any more let me know. I'll get the ''' escaped as it should.

ohler55 commented 1 year ago

Fixed in release v1.18.4 just now. It required one change to a character mapping table.

paulo-raca commented 1 year ago

Thanks!