This project is archived and is in readonly mode.

#2182 ✓stale
Marcello Nuccio

[PATCH] ActiveSupport::JSON.decode cannot parse output from to_json()

Reported by Marcello Nuccio | March 9th, 2009 @ 05:10 PM | in 3.x

ActiveSupport::JSON.decode({"1110-39-10"=>1}.to_json) throws ActiveSupport::JSON::ParseError: Invalid JSON string

The problem is that "1110-39-10" looks like a date, but is not. For example removing the trailing '0' in the key, works: ActiveSupport::JSON.decode({"1110-39-1"=>1}.to_json)

Comments and changes to this ticket

  • Rishav Rastogi

    Rishav Rastogi April 10th, 2010 @ 08:46 PM

    fails in 2.3.X as well. Care to submit a patch ?

  • Nathaniel Bibler

    Nathaniel Bibler April 11th, 2010 @ 04:40 AM

    This actually looks like (in Rails 2.3.5) it's not so much an error in the JSON parsing, as it is in YAML parsing:

    >> ActiveSupport::JSON.decode({"1110-39-10"=>1}.to_json)
    StandardError: Invalid JSON string
    >> ActiveSupport::JSON::Backends::Yaml.send(:convert_json_to_yaml, "{'1110-39-10': 'foo'}")
    => "{ 1110-39-10 :  'foo'}"
    >> YAML.load _
    ArgumentError: invalid date
    

    So, it's actually the YAML.load call in decode for the YAML backend that is throwing the ArgumentError, which is being caught and re-raised as a ParseError. However, using the packaged JSONGem, backend, again in Rails 2.3.5:

    >> require 'active_support/json/backends/jsongem'
    >> json = {'1110-39-10' => 1}.to_json
    => "{\"1110-39-10\":1}"
    >> ActiveSupport::JSON::Backends::JSONGem.decode(json)
    => {"1110-39-10"=>1}
    
  • Nathaniel Bibler

    Nathaniel Bibler April 11th, 2010 @ 05:04 AM

    • Tag changed from active_support to active_support, patch

    Attached is a patch which updates the ActiveSupport::JSON::DATE_REGEX to more accurately match date strings. I only modified the date (month and day) portions of the regular expression and left the time matching alone. It now only matches on months of 01-12 and days of 01-31 instead of any two-digit numeric combination.

  • Nathaniel Bibler

    Nathaniel Bibler April 11th, 2010 @ 05:12 AM

    That patch, by the way, was made against the 2.3-stable branch.

  • Nathaniel Bibler

    Nathaniel Bibler April 11th, 2010 @ 05:20 AM

    Attached is a patch against master (rails 3), where the tests do still fail for this, as well.

  • Nathaniel Bibler

    Nathaniel Bibler April 11th, 2010 @ 05:47 PM

    • Tag changed from active_support, patch to active_support, json, patch
    • Title changed from “ActiveSupport::JSON.decode cannot parse output from to_json()” to “[PATCH] ActiveSupport::JSON.decode cannot parse output from to_json()”
  • Jeremy Kemper

    Jeremy Kemper May 4th, 2010 @ 06:48 PM

    • Milestone changed from 2.x to 3.x
  • AlekSi

    AlekSi August 27th, 2010 @ 12:05 PM

    • Importance changed from “” to “”

    Actually, it's possible to switch off this behavior (at least in 2.3.8):

    ActiveSupport.parse_json_times = false
    
  • AlekSi

    AlekSi August 27th, 2010 @ 12:39 PM

    Well, this mattr is present, but doesn't works (weird).

  • tribalvibes

    tribalvibes September 30th, 2010 @ 11:30 PM

    • Tag changed from active_support, json, patch to active_support, datetime, iso8601, json, patch

    Hello, there is a further problem with this ActiveSupport::JSON::DATE_REGEX in that it purports to be ISO8601 compliant but does not parse the <date>T<time>Z joining character as per the specification: http://www.w3.org/TR/NOTE-datetime

    Thus, it fails e.g.:

    ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(Time.now))
    
    produces a String not a DateTime, even though ActiveSupport.parse_json_times = true

    Nat, if you could change the midsection from [ \t]+ to something like (T|[ \t]+) it might do the trick. I.e.

        DATE_REGEX = /^(?:\d{4}-(0[1-9]|1[012])-(0[1-9]|[12]\d|3[01])|\d{4}-\d{1,2}-\d{1,2}(T|[ \t]+)\d{1,2}:\d{2}:\d{2}(.[0-9])?(([ \t])Z|[-+]\d{2}?(:\d{2})?))$/
    

    Those of you in the know could advise whether this would hose anything else. We're using only Yagl serialization in production and not the YAML backend.

  • Elliot Winkler

    Elliot Winkler October 14th, 2010 @ 06:05 PM

    I need to check if this is still a problem in 3.x, but I can confirm this is a problem in 2.3.5. tribalvibes' fix above didn't quite seem to work (unless formatting affected the regex). The issue is that the current regex expects a space to be present before the "Z". Here's a fix for that:

    ActiveSupport::JSON::DATE_REGEX = /^(?:\d{4}-(0[1-9]|1[012])-(0[1-9]|[12]\d|3[01])|\d{4}-\d{1,2}-\d{1,2}(T|[ \t]+)\d{1,2}:\d{2}:\d{2}(.[0-9])?(([ \t])?Z|[-+]\d{2}?(:\d{2})?))$/
    
  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:30 PM

    • State changed from “new” to “open”

    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.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:30 PM

    • State changed from “open” to “stale”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Referenced by

Pages