Don't escape character for JSONFunction

[bitstorm]

Hi,

JSONStringer doesn't wrap JSONFunction object in order to preserve JavaScript code. This is ok but if we have quotes in our JavaScript (Example: { "value": window.myfunction(var str ="str") }) we get broken code as quotes are escaped (i.e. { "value": window.myfunction(var str =\"str\") }). We should avoid character escaping for this entity. We could change the following line

https://github.com/tdunning/open-json/blob/master/src/main/java/org/json/JSONStringer.java#L262

to out.append(value)

[tdunning]

Have you tested this change?

[tdunning]

I just pushed a fix and a test. Can you verify?

[bitstorm]

Hi,

sorry but I don't see this commit. can you link it ?

Thank you.

[bitstorm]

I did a PR, so we can better discuss the issue

https://github.com/tdunning/open-json/pull/3

[klopfdreh]

Great. This looks better to me. @bitstorm Maybe we can revert the change with the boolean to the string function, because we append the string as raw input in case of JSONFunction.

I would suggest the following reverts: https://github.com/tdunning/open-json/blob/2530de15ee87bd7e4aaa17668777d7938f9e6fca/src/main/java/org/json/JSONStringer.java#L323 - remove the boolean arg https://github.com/tdunning/open-json/blob/2530de15ee87bd7e4aaa17668777d7938f9e6fca/src/main/java/org/json/JSONStringer.java#L324 - remove the if condition https://github.com/tdunning/open-json/blob/2530de15ee87bd7e4aaa17668777d7938f9e6fca/src/main/java/org/json/JSONStringer.java#L383 - remove the if condition

As @bitstorm mentioned correctly other values should not been escaped, too because it causes javascript issues.

[bitstorm]

@klopfdreh Looks good to me as well: With this commit I was able to fix the problem with ModalWindow and with WicketStuff module select2. I'm +1 to to remove the flag for surroundingQuotes as it's not used any more (it's always set to true). But we should ask @tdunning if it might be helpful in future.

[tdunning]

I don't see any future use. Let me look at the current state of the code and get back.

On Wed, Nov 30, 2016 at 6:11 PM, Andrea Del Bene notifications@github.com wrote:

@klopfdreh https://github.com/klopfdreh Looks good to me as well: With this commit I was able to fix the problem with ModalWindow and with WicketStuff module select2. I'm +1 to to remove the flag for surroundingQuotes as it's not used any more (it's always set to true). But we should ask @tdunning https://github.com/tdunning if it might be helpful in future.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tdunning/open-json/issues/2#issuecomment-263821314, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPSeofrmYbmVYkl9Ts8lXj7OEbqd-ayks5rDT3bgaJpZM4K-DLv .

[tdunning]

Nuked it. Intellij allows inlining of parameters and then semi-automated optimization of the if statements. Very nice.

This will appear in next release which should be soon.

On Fri, Dec 2, 2016 at 7:44 PM, Ted Dunning ted.dunning@gmail.com wrote:

I don't see any future use. Let me look at the current state of the code and get back.

On Wed, Nov 30, 2016 at 6:11 PM, Andrea Del Bene <notifications@github.com

wrote:

@klopfdreh https://github.com/klopfdreh Looks good to me as well: With this commit I was able to fix the problem with ModalWindow and with WicketStuff module select2. I'm +1 to to remove the flag for surroundingQuotes as it's not used any more (it's always set to true). But we should ask @tdunning https://github.com/tdunning if it might be helpful in future.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tdunning/open-json/issues/2#issuecomment-263821314, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPSeofrmYbmVYkl9Ts8lXj7OEbqd-ayks5rDT3bgaJpZM4K-DLv .

[klopfdreh]

Only for information / reference: https://github.com/apache/wicket/pull/193

[bitstorm]

@tdunning @klopfdreh
thank you!