Add optional test for psych parser and fix for psych parsing error. #32

Closed
wants to merge 1 commit into from

2 participants

@matt-glover

When using the Crack gem with ruby 1.9.2 you may see Psych::Syntax errors bubble up through the gem (see issue 29 #29) and to the calling code. This pull request attempts to demonstrate that issue via a psych test (which is only executed when your machine is configured to support psych) and a potential fix for that issue.

When you build ruby 1.9.2 with libyaml on your system ruby automatically includes the psych YAML parser as the default instead of syck which is used in previous versions of ruby (and those machines building ruby without libyaml). The error bubbled up from syck when bogus YAML is provided, which crack catches and wraps, is an argument error. The error bubbled up from psych is a derivative of the SyntaxError type.

So the fix was to catch both error types since the syck parser is currently in use and needs to be supported at least for a while but it is no longer being maintained and presumably will not be used by default in some future release of ruby.

@voxik voxik commented on the diff Mar 11, 2013
lib/crack/json.rb
@@ -10,7 +10,7 @@ module Crack
class JSON
def self.parse(json)
YAML.load(unescape(convert_json_to_yaml(json)))
- rescue ArgumentError => e
+ rescue ArgumentError, SyntaxError => e
@voxik
voxik added a note Mar 11, 2013

This should be rescue ArgumentError, Psych::SyntaxError => e for Ruby 2.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jnunemaker
Owner

Closing this due to age and neglect on my part.

@jnunemaker jnunemaker closed this Jun 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment