#1485 json_escape eats away double quotes - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#1485 ✓stale

json_escape eats away double quotes

Reported by Vlad R | November 26th, 2008 @ 08:53 PM | in 3.x

json_escape("hello".to_json) will incorrectly produce 'hello' instead of '"hello"'

rails/actionpack/lib/action_view/template_handlers/erb.rb or rails/actionpack/lib/action_view/erb/util.rb

def json_escape(s)
  s.to_s.gsub(/[&"><]/) { |special| JSON_ESCAPE[special] }
end

should probably read:

def json_escape(s)
  s.to_s.gsub(/[&><]/) { |special| JSON_ESCAPE[special] }
end

Comments and changes to this ticket

Pratik March 12th, 2009 @ 03:51 PM
- State changed from “new” to “incomplete”
Could you please make that a patch with a test case ? http://guides.rails.info/contrib... should be helpful.

Thanks!
You flagged this item as spam.
ion April 1st, 2009 @ 08:17 PM
- Tag set to “patch”
Attaching a patch with a test case.
- json_escape-quotation-marks.patch 2.4 KB
CancelProfileIsBroken April 2nd, 2009 @ 11:38 AM
- State changed from “incomplete” to “new”
CancelProfileIsBroken August 6th, 2009 @ 12:55 PM
- Tag changed from “patch” to “bugmash, patch”
Rizwan Reza August 7th, 2009 @ 09:18 PM
verified

+1 The above patch works. Applied it to 2-3-stable and ran tests to see it working.
You flagged this item as spam.
Rajesh August 9th, 2009 @ 12:30 AM
verified
You flagged this item as spam.
Matt Duncan August 9th, 2009 @ 12:57 AM
-1 This is the expected behavior according to Michael Koziarski.
Michael Koziarski August 9th, 2009 @ 12:58 AM
- State changed from “new” to “invalid”
This is intended functionality of json_escape. It returns the strings without dangerous characters, not escaped to be json values themselves.
Jeremy Kemper August 9th, 2009 @ 01:18 AM
- Tag changed from “bugmash, patch” to “patch”
You flagged this item as spam.
Kristofor Selden April 28th, 2010 @ 01:47 AM
In the example in the RDoc for json_escape, assuming @person was an Active Record object, to_json would serialize it into a JSON object in which the keys are quoted and string values are too, json_escape would make the JSON invalid.

From the documentation:
```
  <%=j @person.to_json %>
```
The resulting JSON object for @person would become invalid after json_escape.
It would output something like this:
```
  {person:{name:John Doe,created_at:2010-04-28T00:15:24Z,updated_at:2010-04-28T00:19:30Z,id:1}}
```
The only use I can think of is (as long as you don't need quotes in the string):
```
  <script type="text/javascript">
  var my_string = "My templated JSON string within HTML <%=j @ruby_string.to_json %>";
  </script>
```
But even then quotes would be stripped but to_json would have added the slash. So even in the above example, if @ruby_string ended in a quote you'd be still left with the backslash, causing problems. You can't omit the to_json because json_escape doesn't escape backslashes.
Is the output of json_escape intended to not output a complete JSON value? Just partial JSON string value without any backslash handling?

I guess I don't really understand when someone would use json_escape. At least, this ticket should not have been marked invalid without changing the RDoc.

Seems more useful if to_json could take an option to encode HTML special chars as it was generating the json.
Neeraj Singh April 28th, 2010 @ 03:00 AM
- Tag changed from “patch” to “doc, documenation, patch, rails3”
I fully agree with Kristofor Selden that json_escape documentation should explicitly mention that after that operation, the output is not a valid JSON.

Attached is a documentation patch which fixes that. Patch is against rails3 edge.

Also the doc should have an example of the most common way a JSON structure is used and what the value would be after json_escape.
- doc_patch_for_json_escape_against_rails3_edge.diff 2.7 KB
Jeremy Kemper April 28th, 2010 @ 06:04 AM
- Milestone changed from “2.x” to “2.3.6”
- State changed from “invalid” to “open”
- Assigned user set to “Rick”
I don't follow why json_escape('"') == '' in the first place and the old commits don't reveal why. Rick?
Rizwan Reza May 16th, 2010 @ 02:41 AM
- Tag changed from “doc, documenation, patch, rails3” to “bugmash, doc, documenation, patch, rails3”
[bulk edit]
Jeremy Kemper May 23rd, 2010 @ 05:54 PM
- Milestone changed from “2.3.6” to “2.3.7”
[bulk edit]
Jeremy Kemper May 24th, 2010 @ 09:40 AM
- Milestone changed from “2.3.7” to “2.3.8”
[bulk edit]
Jeremy Kemper May 25th, 2010 @ 11:45 PM
- Milestone changed from “2.3.8” to “2.3.9”
[bulk edit]
Rohit Arondekar June 19th, 2010 @ 02:09 AM
Was this doc patch applied?
You flagged this item as spam.
Tohru Hashimoto August 27th, 2010 @ 03:02 AM
- Importance changed from “” to “”
+1 why is not this ticket applied?

You should use escape_javascript if you want to escape double quotes.
Jeremy Kemper August 30th, 2010 @ 02:28 AM
- Milestone changed from “2.3.9” to “2.3.10”
[bulk edit]
You flagged this item as spam.
Andrea Campi October 9th, 2010 @ 06:15 PM
+1 to Neeraj's patch, but I guess you should reroll it to only include this change (it contains an unrelated patch that has already been committed).
Rohit Arondekar October 14th, 2010 @ 10:30 AM
- Assigned user changed from “Rick” to “Neeraj Singh”
Neeraj, could you please make the necessary changes? :)
Neeraj Singh October 14th, 2010 @ 03:22 PM
- Assigned user cleared.
@Andrea Since it is a documentation patch go ahead and make the necessary changes and apply it. :-)

Thanks
Rohit Arondekar October 14th, 2010 @ 03:29 PM
Andrea, You can make changes directly here: http://github.com/lifo/docrails the changes are later merged into the main Rails repo after review. Also remember to follow the guidelines provided here — http://guides.rubyonrails.org/api_documentation_guidelines.html — and do report back so that this ticket can be closed. Thanks! :)
Jeff Kreeftmeijer October 15th, 2010 @ 07:15 AM
- Tag cleared.
Automatic cleanup of spam.
You flagged this item as spam.
Andrea Campi October 16th, 2010 @ 09:20 PM
Committed in a38e653a6d512de3d221
You flagged this item as spam.
Andrea Campi October 19th, 2010 @ 10:00 PM
- Assigned user set to “Neeraj Singh”
Neeraj Singh November 10th, 2010 @ 10:21 PM
- State changed from “open” to “resolved”
You flagged this item as spam.
Fjan November 20th, 2010 @ 11:22 AM
The incorrect behaviour is still in Rails 3.0.3. So <%=j @person.to_json %> is still in the 3.0.3 docs and still produces invalid output. Do we need a new ticket for this?
Neeraj Singh November 20th, 2010 @ 01:29 PM
- State changed from “resolved” to “open”
- Milestone changed from “2.3.10” to “3.x”
- Assigned user changed from “Neeraj Singh” to “Xavier Noria”
Commit https://github.com/rails/rails/commit/a38e653a6d512de3d221 is present in rails master but for some reason it is not present in rails-3-0-stable branch.

Someone from rails core need to cherry-pick this commit.
Repository November 20th, 2010 @ 02:07 PM
- State changed from “open” to “resolved”
(from [54828a17034d6ffd3a5b203d1b4233c18e818030]) json_escape makes json invalid doc change [#1485 state:resolved] https://github.com/rails/rails/commit/54828a17034d6ffd3a5b203d1b423...
Xavier Noria November 20th, 2010 @ 03:30 PM
- State changed from “resolved” to “open”
- Importance changed from “” to “Low”
Docs are more complete now regarding how the helper works. That is, describing what it does.

Nevertheless, if the behavior of this helper is correct, I think we should document why it is helpful. Does anybody have a use case for it? Why are quotes removed? Why are characters escaped using JavaScript \uXXXX literals?
You flagged this item as spam.
Fjan November 20th, 2010 @ 03:39 PM
I can't think of any prominent use case for an escape helper that eats double quotes. It even eats quotes when they are escaped with " and then leaves the \ which results in odd control characters in your JSON string that potentially do more harm then good.

I think it should be fixed to leave the quotes. The use case in the docs <%= j @person.to_json %> is not covered by any of the other escape helpers right now. I went back to setting ActiveSupport.escape_html_entities_in_json = true in the initializers and removing the j altogether.
You flagged this item as spam.
Preston Marshall December 16th, 2010 @ 01:47 AM
This is STILL a fail for me. Is there ANY point to json_escape at all? It seems completely useless to me.
You flagged this item as spam.
Fjan December 16th, 2010 @ 10:59 AM
@Michael Koziarski "It returns the strings without dangerous characters"

That's not quite true:
json_escape('hi \"now\"') # => 'hi \now\'

This will insert a spurious new line, but you can insert any other control character this way, typically breaking stuff.
rails March 17th, 2011 @ 12:00 AM
This issue has been automatically marked as stale because it has not been commented on for at least three months.

The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
rails March 17th, 2011 @ 12:00 AM
- State changed from “open” to “stale”