VELTOOLS-184

[diggernet]

$esc.json() escapes ", \, and /

[arkanovicz]

I see in StringEscapeUtils the following comment for escapeEcmaScript():

The only difference between Java strings and EcmaScript strings
is that in EcmaScript, a single quote and forward-slash (/) are escaped.

Is that what we want for json? Isn't it more pertinent to just call escapeJava()?

[diggernet]

That's a fair question. I suppose the lazy answer is "because escapeJson() exists". :)

Comparing escapeJava() and escapeJson() shows they differ on two characters: / and \u007F. I don't pretend to know why those differences exist, and in a quick test escapeJava() works without error. But they do produce different output strings. Without knowing the reason for this, I'd hesitate to assume that the output of escapeJava() is compatible with all existing JSON consuming code. So using escapeJson() seems the safer choice.

[arkanovicz]

Well, EcmaScript is not Json. I don't know of any valid reason to escape <'> and </> in Json, so I don't mind having an esc.json() method, but it should just ba a synonym of $esc.java() IMHO.

[diggernet]

I'm fine with a synonym too, unless some future issue is raised for compatibility. The main point is to have $esc.json() present, since the most obvious alternative of $esc.javascript() causes errors. Which is what led Maurice Perry to create VELTOOLS-184 two years ago, and why I discovered it this week.

[ChristopherSchultz]

I think buggy libraries which ignore RFC-MUST should be fixed or made to die. There is no reason this patch should be necessary.

[arkanovicz]

The current state of the patch is to propose $esc.json() (which didn't exist) as an alias for $esc.java(). I'm reluctant to escape things that don't have to be, not in regard to broken Json parsers, but because if we go this way, why not escape each and every character in the string?!

[ChristopherSchultz]

Because that would require more code.

IMO less code is always better if you can get away with it. This is adding code just for the sake of adding code.

I'm not -1'ing this or anything, just complaining that it should not be necessary.

[diggernet]

While I totally agree with you in general about libraries ignoring RFC, I'm not sure I understand your frustration with regards to this change. This has nothing to do with any RFC.
The purpose of this change is user discoverability. When a user has a Velocity template that is a JSON file, and they need to escape the text they are putting into that JSON, what do they use? Ah, there's $esc.javascript() and JSON comes from Javascript, so that should work. It seems to work. But then they hit a string with ' and suddenly it doesn't work. Well, there's $esc.java(), but Java has nothing to do with JSON, so why trust that any more than $esc.javascript()?
By adding $esc.json(), no matter the implementation, the user will see something obviously correct for their use case, and can move on to other things. I happened to choose to use escapeJson() to implement it because that struck me as more correct, but Claude makes a good argument that the difference shouldn't matter. Either way, the average Velocity user shouldn't need to care about those details.