[GitHub] [johnzon] rmannibucau commented on a change in pull request #40: JOHNZON-209 Fix JsonObject#toString() to escape key names.

[GitBox <gi...@apache.org>]

rmannibucau commented on a change in pull request #40: JOHNZON-209 Fix JsonObject#toString() to escape key names.
URL: https://github.com/apache/johnzon/pull/40#discussion_r277176951
 
 

 ##########
 File path: johnzon-core/src/main/java/org/apache/johnzon/core/JsonObjectImpl.java
 ##########
 @@ -147,7 +147,7 @@ public String toString() {
         while (hasNext) {
             final Map.Entry<String, JsonValue> entry = it.next();
 
-            builder.append('"').append(entry.getKey()).append("\":");
+            builder.append('"').append(Strings.escape(entry.getKey())).append("\":");
 
 Review comment:
   This relies on a global buffer cache, we likely want to use the builder - object builder or parser factory - buffer factory and not the global one which is there for compatibility IIRC.
   It also slows down default case - not escaped - which is a concern because toString is a serialization solution for jsonobject for jsonb and some other cases.
   
   From memory, caching was not efficient until you are lock free and hash free.
   
   Alternatively we can make Strings with a method taking a builder and without the buffer logic we could call if the key contains characters to convert in unicode or escapable, in this last case the check should be fast and conversion can surely be done without buffer since it is very unlikely it needs to be done. Needs a small benchmark but it sounds the simplest and fastest to me assuming escaping is rare (from my experience it is for keys). Also iterating over the char[] instead of String with chartAt will be faster.
   
   Wdyt? Can you try to make this escape call a bit less impacting?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services