View Issue Details

IDProjectCategoryView StatusLast Update
0030870FPCFCLpublic2016-11-07 00:46
ReporterLuiz AmericoAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product VersionProduct Build 
Target Version3.2.0Fixed in Version3.1.1 
Summary0030870: Do not escape / character when encoding a string as JSON
DescriptionCurrently, the character '/' is escaped when encoding a string as JSON, so is encoded as \/.

The '\' , '"' and Control Characters must be escaped in a JSON string. All other characters, including / can be encoded, optionally. See https://tools.ietf.org/html/rfc7159#section-7

In fact most implementations does not encode '/'. The main example is the parser/encoder of ECMAScript (JavaScript) that is standardized: http://www.ecma-international.org/ecma-262/6.0/#sec-json.stringify

For a simple and practical example open a Browser console (F12) and execute

JSON.stringify(location.href)

One example that shows the issue is when generating JSON Schema (http://jsonschema.net) with fpjson:

It will encode
"$schema": "http://json-schema.org/draft-04/schema#", as
"$schema": "http:\/\/json-schema.org\/draft-04\/schema#",

This breaks readability, make the size bigger, without any advantage

Attached patch
TagsNo tags attached.
Fixed in Revision34819
FPCOldBugId0
FPCTarget
Attached Files
  • remove-escape.diff (1,178 bytes)
    diff --git packages/fcl-json/src/fpjson.pp packages/fcl-json/src/fpjson.pp
    index 3c26b84..9767fd1 100644
    --- packages/fcl-json/src/fpjson.pp
    +++ packages/fcl-json/src/fpjson.pp
    @@ -678,12 +678,11 @@ begin
       While I<=L do
         begin
         C:=AnsiChar(P^);
    -    if (C in ['"','/','\',#0..#31]) then
    +    if (C in ['"','\',#0..#31]) then
           begin
           Result:=Result+Copy(S,J,I-J);
           Case C of
             '\' : Result:=Result+'\\';
    -        '/' : Result:=Result+'\/';
             '"' : Result:=Result+'\"';
             #8  : Result:=Result+'\b';
             #9  : Result:=Result+'\t';
    diff --git packages/fcl-json/tests/testjsondata.pp packages/fcl-json/tests/testjsondata.pp
    index 339bb41..0201389 100644
    --- packages/fcl-json/tests/testjsondata.pp
    +++ packages/fcl-json/tests/testjsondata.pp
    @@ -4092,7 +4092,6 @@ begin
       TestTo('AB','AB');
       TestTo('ABC','ABC');
       TestTo('\','\\');
    -  TestTo('/','\/');
       TestTo('"','\"');
       TestTo(#8,'\b');
       TestTo(#9,'\t');
    @@ -4115,7 +4114,6 @@ begin
       TestTo('A'#12'BC','A\fBC');
       TestTo('A'#13'BC','A\rBC');
       TestTo('\\','\\\\');
    -  TestTo('//','\/\/');
       TestTo('""','\"\"');
       TestTo(#8#8,'\b\b');
       TestTo(#9#9,'\t\t');
    
    remove-escape.diff (1,178 bytes)
  • remove-escape2.diff (1,220 bytes)
    diff --git packages/fcl-json/src/fpjson.pp packages/fcl-json/src/fpjson.pp
    index 3c26b84..9767fd1 100644
    --- packages/fcl-json/src/fpjson.pp
    +++ packages/fcl-json/src/fpjson.pp
    @@ -678,12 +678,11 @@ begin
       While I<=L do
         begin
         C:=AnsiChar(P^);
    -    if (C in ['"','/','\',#0..#31]) then
    +    if (C in ['"','\',#0..#31]) then
           begin
           Result:=Result+Copy(S,J,I-J);
           Case C of
             '\' : Result:=Result+'\\';
    -        '/' : Result:=Result+'\/';
             '"' : Result:=Result+'\"';
             #8  : Result:=Result+'\b';
             #9  : Result:=Result+'\t';
    diff --git packages/fcl-json/tests/testjsondata.pp packages/fcl-json/tests/testjsondata.pp
    index 339bb41..49d4e34 100644
    --- packages/fcl-json/tests/testjsondata.pp
    +++ packages/fcl-json/tests/testjsondata.pp
    @@ -4092,7 +4092,7 @@ begin
       TestTo('AB','AB');
       TestTo('ABC','ABC');
       TestTo('\','\\');
    -  TestTo('/','\/');
    +  TestTo('/','/');
       TestTo('"','\"');
       TestTo(#8,'\b');
       TestTo(#9,'\t');
    @@ -4115,7 +4115,7 @@ begin
       TestTo('A'#12'BC','A\fBC');
       TestTo('A'#13'BC','A\rBC');
       TestTo('\\','\\\\');
    -  TestTo('//','\/\/');
    +  TestTo('//','//');
       TestTo('""','\"\"');
       TestTo(#8#8,'\b\b');
       TestTo(#9#9,'\t\t');
    
    remove-escape2.diff (1,220 bytes)
  • remove-escape3.diff (1,624 bytes)
    diff --git packages/fcl-json/src/fpjson.pp packages/fcl-json/src/fpjson.pp
    index 3c26b84..9767fd1 100644
    --- packages/fcl-json/src/fpjson.pp
    +++ packages/fcl-json/src/fpjson.pp
    @@ -678,12 +678,11 @@ begin
       While I<=L do
         begin
         C:=AnsiChar(P^);
    -    if (C in ['"','/','\',#0..#31]) then
    +    if (C in ['"','\',#0..#31]) then
           begin
           Result:=Result+Copy(S,J,I-J);
           Case C of
             '\' : Result:=Result+'\\';
    -        '/' : Result:=Result+'\/';
             '"' : Result:=Result+'\"';
             #8  : Result:=Result+'\b';
             #9  : Result:=Result+'\t';
    diff --git packages/fcl-json/tests/testjsondata.pp packages/fcl-json/tests/testjsondata.pp
    index 339bb41..b0b85b6 100644
    --- packages/fcl-json/tests/testjsondata.pp
    +++ packages/fcl-json/tests/testjsondata.pp
    @@ -4054,6 +4054,7 @@ begin
       TestFrom('ABC','ABC');
       TestFrom('\\','\');
       TestFrom('\/','/');
    +  TestFrom('/','/');
       TestFrom('\"','"');
       TestFrom('\b',#8);
       TestFrom('\t',#9);
    @@ -4077,6 +4078,7 @@ begin
       TestFrom('A\rBC','A'#13'BC');
       TestFrom('\\\\','\\');
       TestFrom('\/\/','//');
    +  TestFrom('//','//');
       TestFrom('\"\"','""');
       TestFrom('\b\b',#8#8);
       TestFrom('\t\t',#9#9);
    @@ -4092,7 +4094,7 @@ begin
       TestTo('AB','AB');
       TestTo('ABC','ABC');
       TestTo('\','\\');
    -  TestTo('/','\/');
    +  TestTo('/','/');
       TestTo('"','\"');
       TestTo(#8,'\b');
       TestTo(#9,'\t');
    @@ -4115,7 +4117,7 @@ begin
       TestTo('A'#12'BC','A\fBC');
       TestTo('A'#13'BC','A\rBC');
       TestTo('\\','\\\\');
    -  TestTo('//','\/\/');
    +  TestTo('//','//');
       TestTo('""','\"\"');
       TestTo(#8#8,'\b\b');
       TestTo(#9#9,'\t\t');
    
    remove-escape3.diff (1,624 bytes)

Activities

Luiz Americo

2016-11-04 22:30

developer  

remove-escape.diff (1,178 bytes)
diff --git packages/fcl-json/src/fpjson.pp packages/fcl-json/src/fpjson.pp
index 3c26b84..9767fd1 100644
--- packages/fcl-json/src/fpjson.pp
+++ packages/fcl-json/src/fpjson.pp
@@ -678,12 +678,11 @@ begin
   While I<=L do
     begin
     C:=AnsiChar(P^);
-    if (C in ['"','/','\',#0..#31]) then
+    if (C in ['"','\',#0..#31]) then
       begin
       Result:=Result+Copy(S,J,I-J);
       Case C of
         '\' : Result:=Result+'\\';
-        '/' : Result:=Result+'\/';
         '"' : Result:=Result+'\"';
         #8  : Result:=Result+'\b';
         #9  : Result:=Result+'\t';
diff --git packages/fcl-json/tests/testjsondata.pp packages/fcl-json/tests/testjsondata.pp
index 339bb41..0201389 100644
--- packages/fcl-json/tests/testjsondata.pp
+++ packages/fcl-json/tests/testjsondata.pp
@@ -4092,7 +4092,6 @@ begin
   TestTo('AB','AB');
   TestTo('ABC','ABC');
   TestTo('\','\\');
-  TestTo('/','\/');
   TestTo('"','\"');
   TestTo(#8,'\b');
   TestTo(#9,'\t');
@@ -4115,7 +4114,6 @@ begin
   TestTo('A'#12'BC','A\fBC');
   TestTo('A'#13'BC','A\rBC');
   TestTo('\\','\\\\');
-  TestTo('//','\/\/');
   TestTo('""','\"\"');
   TestTo(#8#8,'\b\b');
   TestTo(#9#9,'\t\t');
remove-escape.diff (1,178 bytes)

Luiz Americo

2016-11-04 22:34

developer   ~0095574

In page 4 of http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf , the / is cited as an example that can be encoded in different ways:

The following four cases all produce the same result:
"\u002F"
"\u002f"
"\/"
"/"

Balázs Székely

2016-11-05 06:57

reporter   ~0095580

@Luiz
This will break existing code, please make it optional.

Thaddy de Koning

2016-11-05 08:54

reporter   ~0095582

Last edited: 2016-11-05 09:24

View 5 revisions

And since the ONLY json source should be its root at http://json.org/
which clearly states that / must be escaped there is simply a bug in the json-schema source. Not in the fpc implementation. It should not even be an option ;)

The example in the documentation is wrong. It clearly states that solidus isone of the characters that *must* be escaped as per the same section above the example. It's a standards specification. Must = must, must is not *should* (nor *may*).
So that example is wrong and the text is leading. There is no option to read it in any other way. Solidus is excluded from the set of characters treated without escaping.

OTOH it also says: control characters U+0000 to U+001F and Solidus is U+002F
Bit of a contradiction there....

Thaddy de Koning

2016-11-05 09:39

reporter   ~0095583

and the spec is still draft status. But the draft specification contradicts itself.

Luiz Americo

2016-11-05 21:05

developer   ~0095598

Some clarifications:

- It will not break existing code: all string encoded as "test\/test" will still be read as 'test/test'

- Currently fpc already support all formats: it reads as the same both "test\/test" and "test/test" -> 'test/test'

- http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf document that clearly points the option to encode / as "\u002f" or "\/" or "/" is referenced in json.org

- Is not specific to JSON schema any string with '/' will be encoded by fpc as "\/", the fact is that fpc is the only implementation that encodes / as \/

- Yes, the JSON specification is a mess.

Luiz Americo

2016-11-05 21:21

developer   ~0095599

"All characters may be
placed within the quotation marks except for the characters that must be escaped: quotation mark (U+0022),
reverse solidus (U+005C), and the control characters U+0000 to U+001F"

The only characters that MUST be escaped are the above. There are the option for escaping the others:

\/
 represents the solidus character (U+002F).
\b
 represents the backspace character (U+0008).
\f
 represents the form feed character (U+000C).
\n
 represents the line feed character (U+000A).
\r
 represents the carriage return character (U+000D).
\t
 represents the character tabulation character (U+0009).

Technically, fpc could encode form feed as U+000C but it encodes as \f for convenience easy read, same as the Javascript implementations does

The same for '/' it can encode as / or \/, both are correct, but IMO, for readability and making smaller is more convenient to encode as /. There's no drawback,or compatibility breaking as mentioned above

Luiz Americo

2016-11-05 21:52

developer  

remove-escape2.diff (1,220 bytes)
diff --git packages/fcl-json/src/fpjson.pp packages/fcl-json/src/fpjson.pp
index 3c26b84..9767fd1 100644
--- packages/fcl-json/src/fpjson.pp
+++ packages/fcl-json/src/fpjson.pp
@@ -678,12 +678,11 @@ begin
   While I<=L do
     begin
     C:=AnsiChar(P^);
-    if (C in ['"','/','\',#0..#31]) then
+    if (C in ['"','\',#0..#31]) then
       begin
       Result:=Result+Copy(S,J,I-J);
       Case C of
         '\' : Result:=Result+'\\';
-        '/' : Result:=Result+'\/';
         '"' : Result:=Result+'\"';
         #8  : Result:=Result+'\b';
         #9  : Result:=Result+'\t';
diff --git packages/fcl-json/tests/testjsondata.pp packages/fcl-json/tests/testjsondata.pp
index 339bb41..49d4e34 100644
--- packages/fcl-json/tests/testjsondata.pp
+++ packages/fcl-json/tests/testjsondata.pp
@@ -4092,7 +4092,7 @@ begin
   TestTo('AB','AB');
   TestTo('ABC','ABC');
   TestTo('\','\\');
-  TestTo('/','\/');
+  TestTo('/','/');
   TestTo('"','\"');
   TestTo(#8,'\b');
   TestTo(#9,'\t');
@@ -4115,7 +4115,7 @@ begin
   TestTo('A'#12'BC','A\fBC');
   TestTo('A'#13'BC','A\rBC');
   TestTo('\\','\\\\');
-  TestTo('//','\/\/');
+  TestTo('//','//');
   TestTo('""','\"\"');
   TestTo(#8#8,'\b\b');
   TestTo(#9#9,'\t\t');
remove-escape2.diff (1,220 bytes)

Luiz Americo

2016-11-05 21:52

developer   ~0095600

Added a new patch that instead of removing the tests, adapt the expected value

Luiz Americo

2016-11-05 23:51

developer  

remove-escape3.diff (1,624 bytes)
diff --git packages/fcl-json/src/fpjson.pp packages/fcl-json/src/fpjson.pp
index 3c26b84..9767fd1 100644
--- packages/fcl-json/src/fpjson.pp
+++ packages/fcl-json/src/fpjson.pp
@@ -678,12 +678,11 @@ begin
   While I<=L do
     begin
     C:=AnsiChar(P^);
-    if (C in ['"','/','\',#0..#31]) then
+    if (C in ['"','\',#0..#31]) then
       begin
       Result:=Result+Copy(S,J,I-J);
       Case C of
         '\' : Result:=Result+'\\';
-        '/' : Result:=Result+'\/';
         '"' : Result:=Result+'\"';
         #8  : Result:=Result+'\b';
         #9  : Result:=Result+'\t';
diff --git packages/fcl-json/tests/testjsondata.pp packages/fcl-json/tests/testjsondata.pp
index 339bb41..b0b85b6 100644
--- packages/fcl-json/tests/testjsondata.pp
+++ packages/fcl-json/tests/testjsondata.pp
@@ -4054,6 +4054,7 @@ begin
   TestFrom('ABC','ABC');
   TestFrom('\\','\');
   TestFrom('\/','/');
+  TestFrom('/','/');
   TestFrom('\"','"');
   TestFrom('\b',#8);
   TestFrom('\t',#9);
@@ -4077,6 +4078,7 @@ begin
   TestFrom('A\rBC','A'#13'BC');
   TestFrom('\\\\','\\');
   TestFrom('\/\/','//');
+  TestFrom('//','//');
   TestFrom('\"\"','""');
   TestFrom('\b\b',#8#8);
   TestFrom('\t\t',#9#9);
@@ -4092,7 +4094,7 @@ begin
   TestTo('AB','AB');
   TestTo('ABC','ABC');
   TestTo('\','\\');
-  TestTo('/','\/');
+  TestTo('/','/');
   TestTo('"','\"');
   TestTo(#8,'\b');
   TestTo(#9,'\t');
@@ -4115,7 +4117,7 @@ begin
   TestTo('A'#12'BC','A\fBC');
   TestTo('A'#13'BC','A\rBC');
   TestTo('\\','\\\\');
-  TestTo('//','\/\/');
+  TestTo('//','//');
   TestTo('""','\"\"');
   TestTo(#8#8,'\b\b');
   TestTo(#9#9,'\t\t');
remove-escape3.diff (1,624 bytes)

Luiz Americo

2016-11-05 23:53

developer   ~0095603

A third patch that also adds a test for decoding / and \/.

There's no modification to JSONStringToString, added these tests for completeness

CudaText man

2016-11-06 17:35

reporter   ~0095608

>breaks readability, make the size bigger...
I agree with this, so donot escape /, patch needed

Michael Van Canneyt

2016-11-06 18:29

administrator   ~0095611

I have introduced an option 'Strict' for StringToJSONString.
When set, it will escape solidus.

The class variable TJSONString.StrictEscaping controls what .asJSON will pass to StringToJSONString. By default it is set to false, meaning .AsJSON will not escape the solidus.

DumpAsJSON will ignore StrictEscaping and will pass False for strict, as it results in smaller JSON.

Michael Van Canneyt

2016-11-06 18:30

administrator   ~0095612

I have adapted the tests to cater for this.

Issue History

Date Modified Username Field Change
2016-11-04 22:30 Luiz Americo New Issue
2016-11-04 22:30 Luiz Americo File Added: remove-escape.diff
2016-11-04 22:34 Luiz Americo Note Added: 0095574
2016-11-04 23:43 Michael Van Canneyt Assigned To => Michael Van Canneyt
2016-11-04 23:43 Michael Van Canneyt Status new => assigned
2016-11-05 06:57 Balázs Székely Note Added: 0095580
2016-11-05 08:54 Thaddy de Koning Note Added: 0095582
2016-11-05 08:55 Thaddy de Koning Note Edited: 0095582 View Revisions
2016-11-05 09:10 Thaddy de Koning Note Edited: 0095582 View Revisions
2016-11-05 09:11 Thaddy de Koning Note Edited: 0095582 View Revisions
2016-11-05 09:24 Thaddy de Koning Note Edited: 0095582 View Revisions
2016-11-05 09:39 Thaddy de Koning Note Added: 0095583
2016-11-05 21:05 Luiz Americo Note Added: 0095598
2016-11-05 21:21 Luiz Americo Note Added: 0095599
2016-11-05 21:52 Luiz Americo File Added: remove-escape2.diff
2016-11-05 21:52 Luiz Americo Note Added: 0095600
2016-11-05 23:51 Luiz Americo File Added: remove-escape3.diff
2016-11-05 23:53 Luiz Americo Note Added: 0095603
2016-11-06 17:35 CudaText man Note Added: 0095608
2016-11-06 18:29 Michael Van Canneyt Fixed in Revision => 34819
2016-11-06 18:29 Michael Van Canneyt Note Added: 0095611
2016-11-06 18:29 Michael Van Canneyt Status assigned => resolved
2016-11-06 18:29 Michael Van Canneyt Fixed in Version => 3.1.1
2016-11-06 18:29 Michael Van Canneyt Resolution open => fixed
2016-11-06 18:29 Michael Van Canneyt Target Version => 3.2.0
2016-11-06 18:30 Michael Van Canneyt Note Added: 0095612
2016-11-07 00:46 Luiz Americo Status resolved => closed