Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lcg/parser bugs #32

Merged
merged 4 commits into from
Nov 23, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

### Bugs fixed

* fixes bare object parsing in issue #2 and #6
* fixes parsing nil object to not coredump the MRI ruby VM (issue #15)

## 1.2.0 (10/09/2014)

### Changes
Expand Down
6 changes: 2 additions & 4 deletions ext/ffi_yajl/ext/parser/parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ void set_value(CTX *ctx, VALUE val) {
rb_hash_aset(last, key, val);
break;
default:
rb_ary_push(stack, val);
break;
}
}
Expand All @@ -49,8 +50,6 @@ void end_object(CTX *ctx) {
rb_ivar_set(ctx->self, rb_intern("key"), rb_ary_pop(key_stack));
if ( RARRAY_LEN(stack) > 1 ) {
set_value(ctx, rb_ary_pop(stack));
} else {
rb_ivar_set(ctx->self, rb_intern("finished"), rb_ary_pop(stack));
}
}

Expand Down Expand Up @@ -211,7 +210,7 @@ static VALUE mParser_do_yajl_parse(VALUE self, VALUE str, VALUE yajl_opts) {
goto raise;
}
yajl_free(hand);
return rb_ivar_get(self, rb_intern("finished"));
return rb_ary_pop(rb_ivar_get(self, rb_intern("stack")));

raise:
if (hand) {
Expand All @@ -230,4 +229,3 @@ void Init_parser() {
utf8Encoding = rb_utf8_encoding();
#endif
}

7 changes: 2 additions & 5 deletions lib/ffi_yajl/ffi/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,13 @@ def set_value(val)
when Array
stack.last.push(val)
else
raise FFI_Yajl::ParseError.new("internal error: object not a hash or array")
stack.push(val)
end
end

def stack_pop
if stack.length > 1
set_value( stack.pop )
else
@finished = stack.pop
end
end

Expand Down Expand Up @@ -138,11 +136,10 @@ def do_yajl_parse(str, yajl_opts = {})
error = ::FFI_Yajl.yajl_get_error(yajl_handle, 1, str, str.bytesize)
raise ::FFI_Yajl::ParseError.new(error)
end
finished
stack.pop
ensure
::FFI_Yajl.yajl_free(yajl_handle) if yajl_handle
end
end
end
end

2 changes: 2 additions & 0 deletions lib/ffi_yajl/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ def initialize(opts = {})
end

def parse(str)
raise FFI_Yajl::ParseError, "input must be a string or IO" unless str.is_a?(String) || str.respond_to?(:read)

# initialization that we can do in pure ruby
yajl_opts = {}

Expand Down
50 changes: 49 additions & 1 deletion spec/ffi_yajl/parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,55 @@
end
end

context "when parsing nil" do
let(:json) { nil }
it "should not coredump ruby" do
expect{ parser }.to raise_error(FFI_Yajl::ParseError)
end
end

context "when parsing bare int" do
let(:json) { "1" }
it "should parse to the int value" do
expect( parser ).to eq(1)
end
end

context "when parsing bare string" do
let(:json) { '"a"' }
it "should parse to the string value" do
expect( parser ).to eq("a")
end
end

context "when parsing bare true" do
let(:json) { "true" }
it "should parse to the true value" do
expect( parser ).to eq(true)
end
end

context "when parsing bare false" do
let(:json) { "false" }
it "should parse to the false value" do
expect( parser ).to eq(false)
end
end

context "when parsing bare null" do
let(:json) { "null" }
it "should parse to the nil value" do
expect( parser ).to eq(nil)
end
end

context "when parsing bare float" do
let(:json) { "1.1" }
it "should parse to the a float" do
expect( parser ).to eq(1.1)
end
end

context "when json has comments" do
let(:json) { '{"key": /* this is a comment */ "value"}' }

Expand Down Expand Up @@ -482,4 +531,3 @@
end
end
end