Skip Menu |

This queue is for tickets about the YAML-Syck CPAN distribution.

Report information
The Basics
Id: 42390
Status: resolved
Priority: 0/
Queue: YAML-Syck

People
Owner: Nobody in particular
Requestors: MMIMS [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 1.05
Fixed in: (no value)



Subject: [PATCH] JSON::Syck does not properly escape/unescape JSON strings
JSON::Syck fails to decode JSON strings with escaped characters correctly. For example: JSON::Syck::load('"http:\/\/example.com\/"') should decode to 'http://example.com/' I patched t/json-basic.t to produce failing tests.
Subject: json-syck.patch
diff --git a/t/json-basic.t b/t/json-basic.t index dda0afc..6390c43 100644 --- a/t/json-basic.t +++ b/t/json-basic.t @@ -42,6 +42,14 @@ my @tests = ( '{"foo":""}', '["\"://\""]', '"~foo"', + '"\/"', # escaped solidus + '"\""', + '"\b"', + '"\f"', + '"\n"', + '"\r"', + '"\t"', + '"\u0001"', ); plan tests => scalar @tests * (2 + $HAS_JSON) * 2;
On Tue Jan 13 17:23:57 2009, MMIMS wrote: Show quoted text
> JSON::Syck fails to decode JSON strings with escaped characters > correctly. For example: > > JSON::Syck::load('"http:\/\/example.com\/"') should decode to > 'http://example.com/' > > I patched t/json-basic.t to produce failing tests.
Can you make these tests TODO so that they don't fail, unless you have a patch to fix them that is. Also, see this boring form letter I'm pasting around: (This is a form-reply that isn't specific to your particular report) YAML::Syck has just acquired one new maintainer (me), it still doesn't have anyone that *cares* about it. But I'm willing to help solve your report & release a new version with the fix if it's easy for me. It now has a Git repository at: http://github.com/avar/YAML-Syck If your report is a patch that fixes a problem, great. Please remake the patch against Git by forking that repo and sending me a pull request on GitHub (or an update to this bug if you prefer git-format-patch(1) or some other repo provider..). Make sure to include a test for what you fixed. If your report is some code that fails (and you have a testcase for it) a patch against the test suite to demonstrate that failure would be very useful. It's OK if the test crashes and burns, see Test::More's docs for how to make TODO tests that fail now, but shouldn't. Even if it segfaults perl C<system $^X => qw/ -Mblib -MYAML::Syck .../> or something like that and checking the return value will do.
TODO tests pushed to github in commit a25d69e72828c1dcc80d8f07190cb1b4d1068c2f
On Tue Jan 13 17:23:57 2009, MMIMS wrote: Show quoted text
> JSON::Syck fails to decode JSON strings with escaped characters > correctly. For example: > > JSON::Syck::load('"http:\/\/example.com\/"') should decode to > 'http://example.com/' > > I patched t/json-basic.t to produce failing tests.
Greetings, discussing with the other Syck developers, we have a question for you. JSON::XS is 5 times faster than this module. Is there any reason you prefer JSON::Syck to JSON::XS?
Subject: Re: [rt.cpan.org #42390] [PATCH] JSON::Syck does not properly escape/unescape JSON strings
Date: Thu, 23 Sep 2010 19:36:00 -0700
To: Todd Rinaldo via RT <bug-YAML-Syck [...] rt.cpan.org>
From: Marc Mims <marc [...] questright.com>
* Todd Rinaldo via RT <bug-YAML-Syck@rt.cpan.org> [100923 14:39]: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=42390 > > > On Tue Jan 13 17:23:57 2009, MMIMS wrote:
> > JSON::Syck fails to decode JSON strings with escaped characters > > correctly. For example: > > > > JSON::Syck::load('"http:\/\/example.com\/"') should decode to > > 'http://example.com/' > > > > I patched t/json-basic.t to produce failing tests.
> > Greetings, discussing with the other Syck developers, we have a question for you. JSON::XS is 5 > times faster than this module. Is there any reason you prefer JSON::Syck to JSON::XS?
No. I prefer JSON::XS. I use JSON::Any, however, in my Net::Twitter module in oder to offer users to select their own preference. If I'm not mistaken, JSON::Syck has been demoted to an optional in JSON::Any if not remove entirely. -Marc
Ticket migrated to github as https://github.com/toddr/YAML-Syck/issues/30