`to_json` generates malformed json on Python 2.4-2.5 if contained single quotes

[Jun Omae]

to_json generates malformed json on Python 2.4-2.5 if the argument is contained single quotes. to_json uses javascript_quote because Python 2.4-2.5 don't have json library.

Python 2.4.3 (#1, Jun 18 2012, 08:55:31)
[GCC 4.1.2 20080704 (Red Hat 4.1.2-52)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from trac.util.presentation import to_json
>>> print to_json({'json': "a ' single quote"})
{"json":"a \' single quote"}

However, single quote cannot be encoded to \', according to ​http://www.json.org/.

And, jQuery.parseJSON raises SyntaxError for such a json string. This brings a problem when Trac and Trac plugins response json as contents.

[12:46:48.548] jQuery.parseJSON("{\"json\": \"a \\' single quote\"}")
[12:46:48.553] SyntaxError: JSON.parse: bad escaped character
[12:46:51.864] jQuery.parseJSON("{\"json\": \"a ' single quote\"}")
[12:46:51.868] ({json:"a ' single quote"})
[12:52:32.423] jQuery.parseJSON("{\"json\": \"a \\u0027 single quote\"}")
[12:52:32.427] ({json:"a ' single quote"})

I propose that javascript_quote encodes a single quote to \u0027 (neither ' nor \') in order to avoid \' in json text.

[Jun Omae]

The patch [e615826b/jomae.git] for 1.0dev is here. I think we should apply for 0.12-stable.

[Remy Blank]

Good catch! But why encode the single quote, then? The JSON specification allows it as-is in strings, as the string delimiter is always the double quote. Is it to allow using both the single and the double quote around quoted JavaScript strings?

[Jun Omae]

You're right about JSON.

If a plugin uses javascript_quote like the following, it will get a problem.

<script>
  <!--! Enclose single quote -->
  blah_func('${ javascript_quote(blah_plugin_var) }');
</script>

[Christian Boos]

Replying to jomae:

You're right about JSON.

If a plugin uses javascript_quote like the following, it will get a problem.

<script>
  <!--! Enclose single quote -->
  blah_func('${ javascript_quote(blah_plugin_var) }');
</script>

But that would be a problem in the script then, as JSON can contain plain single quotes. So if this is really what they want to do (stringify some JSON instead of letting it be interpreted as Javascript code, what it is normally supposed to be used for), they ought to do the escaping themselves:

blah_func('${ javascript_quote(blah_plugin_var).replace("'", "\'")}');

[Jun Omae]

Replying to cboos:

But that would be a problem in the script then, as JSON can contain plain single quotes. So if this is really what they want to do (stringify some JSON instead of letting it be interpreted as Javascript code, what it is normally supposed to be used for), they ought to do the escaping themselves:

blah_func('${ javascript_quote(blah_plugin_var).replace("'", "\'")}');

If possible, yes. And the following is simple than it if plugin maintainers will change.

blah_func("${ javascript_quote(blah_plugin_var) }");

Or

blah_func(${ to_json(blah_plugin_var) });

If we don't expect to use javascript_quote at enclosing with single quotes, the function shouldn't have escaped single quotes….

Ok. But I think that we should keep escaping the single quotes in javascript_quote for the compatibility.

New patch, [78218477/jomae.git], adds safe parameter to javvscript_quote. As the following, specifies the characters that should not be quoted.

     def to_json(value):
         """Encode `value` to JSON."""
         if isinstance(value, basestring):
            return '"%s"' % javascript_quote(value, safe="'")

[Christian Boos]

Replying to jomae:

But I think that we should keep escaping the single quotes in javascript_quote for the compatibility.

I rather think there's a misunderstanding about the intent of javascript_quote. The doc string and our usage says: "Quote strings for inclusion in javascript", not "Quote strings for inclusion in Javascript strings"! So I think not encoding single quotes would be more correct.

We could have a javascript_string_quote helper function, and if you really want to have blah_func('${ javascript_quote(blah_plugin_var) }') working as you expect, then I'd rather have something explicit like:

def javascript_code_quote(text):
    # same as current javascript_quote, "'" not present in _js_quote 

def javascript_string_quote(text):
    return javascript_code_quote.replace("'", "\'")

javascript_quote = javascript_string_quote # compatibility with buggy usage...

[Christian Boos]

… or javascript_quote_for_code and javascript_quote_for_string.

[Jun Omae]

Replying to cboos:

… So I think not encoding single quotes would be more correct.

Hmm, Ok. I rewrited the patch, repos:jomae.git:ticket10771/well-formed-json.3/trunk. In the patch, javascript_quote does no longer encode single quotes.

Do we have no problems other than it? I'm wondering if it would be ok to apply. I think that we should apply both 0.12-stable and trunk.

[Christian Boos]

Replying to cboos:

Replying to jomae:

But I think that we should keep escaping the single quotes in javascript_quote for the compatibility.

I rather think there's a misunderstanding about the intent of javascript_quote. The doc string and our usage says: "Quote strings for inclusion in javascript", not "Quote strings for inclusion in Javascript strings"!

Actually, I managed to get confused myself, mixing javascript_quote and to_json itself… Sorry about that. Of course it's meant to be used within Javascript strings as that's the only way you can embed arbitrary text in Javascript code.

[Christian Boos]

Replying to jomae:

Do we have no problems other than it? I'm wondering if it would be ok to apply. I think that we should apply both 0.12-stable and trunk.

What about this alternative solution:

• leaving javascript_quote as is
• have a new to_js_string which leaves the ' character unescaped and produces a Javascript string complete with the double quotes
• use to_js_string in to_json

Fix the API doc accordingly so that easily confusable people like me could understand it ;-)

• javascript_quote:

  """Quote strings for inclusion in single or double quote delimited Javascript strings
  """
  

• to_js_string:

  """Embed the given string in a double quote delimited Javascript string
  (conform to the JSON spec)
  """
  

With this approach, I think we can address both the JSON conformance issue and the use case of comment:3.

[Christian Boos]

(fix typos)

[Jun Omae]

Your suggestion looks good. I reworked at [8dfc6fd4/jomae.git].

[Christian Boos]

Replying to jomae:

Your suggestion looks good. I reworked at [8dfc6fd4/jomae.git].

Thanks Jun, this is nearly exactly what I meant, only I suggested in comment:10 that to_js_string should produce the complete Javascript string, i.e. the double quotes as well, which leaves no possibility of misuse.

[Jun Omae]

Oh, my misreading…. See [13a0427a/jomae.git].

[Christian Boos]

Replying to jomae:

Oh, my misreading…. See [13a0427a/jomae.git].

Perfect! All tests pass, feel free to commit.

[Jun Omae]

Thanks a lot, Christian. Applied.