ruby-changes:22734
From: tenderlove <ko1@a...>
Date: Fri, 24 Feb 2012 13:55:46 +0900 (JST)
Subject: [ruby-changes:22734] tenderlove:r34783 (trunk): * ext/psych/parser.c: prevent a memory leak by protecting calls to
tenderlove 2012-02-24 13:55:33 +0900 (Fri, 24 Feb 2012) New Revision: 34783 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=34783 Log: * ext/psych/parser.c: prevent a memory leak by protecting calls to handler callbacks. * test/psych/test_parser.rb: test to demonstrate leak. Modified files: trunk/ChangeLog trunk/ext/psych/parser.c trunk/test/psych/test_parser.rb Index: ChangeLog =================================================================== --- ChangeLog (revision 34782) +++ ChangeLog (revision 34783) @@ -1,3 +1,9 @@ +Fri Feb 24 13:54:33 2012 Aaron Patterson <aaron@t...> + + * ext/psych/parser.c: prevent a memory leak by protecting calls to + handler callbacks. + * test/psych/test_parser.rb: test to demonstrate leak. + Fri Feb 24 12:07:34 2012 Ayumu AIZAWA <ayumu.aizawa@g...> * lib/net/http.rb: Fix documentation. Patched from Florian Mhun Index: ext/psych/parser.c =================================================================== --- ext/psych/parser.c (revision 34782) +++ ext/psych/parser.c (revision 34783) @@ -154,6 +154,68 @@ #endif +static VALUE protected_start_stream(VALUE pointer) +{ + VALUE *args = (VALUE *)pointer; + return rb_funcall(args[0], id_start_stream, 1, args[1]); +} + +static VALUE protected_start_document(VALUE pointer) +{ + VALUE *args = (VALUE *)pointer; + return rb_funcall3(args[0], id_start_document, 3, args + 1); +} + +static VALUE protected_end_document(VALUE pointer) +{ + VALUE *args = (VALUE *)pointer; + return rb_funcall(args[0], id_end_document, 1, args[1]); +} + +static VALUE protected_alias(VALUE pointer) +{ + VALUE *args = (VALUE *)pointer; + return rb_funcall(args[0], id_alias, 1, args[1]); +} + +static VALUE protected_scalar(VALUE pointer) +{ + VALUE *args = (VALUE *)pointer; + return rb_funcall3(args[0], id_scalar, 6, args + 1); +} + +static VALUE protected_start_sequence(VALUE pointer) +{ + VALUE *args = (VALUE *)pointer; + return rb_funcall3(args[0], id_start_sequence, 4, args + 1); +} + +static VALUE protected_end_sequence(VALUE handler) +{ + return rb_funcall(handler, id_end_sequence, 0); +} + +static VALUE protected_start_mapping(VALUE pointer) +{ + VALUE *args = (VALUE *)pointer; + return rb_funcall3(args[0], id_start_mapping, 4, args + 1); +} + +static VALUE protected_end_mapping(VALUE handler) +{ + return rb_funcall(handler, id_end_mapping, 0); +} + +static VALUE protected_empty(VALUE handler) +{ + return rb_funcall(handler, id_empty, 0); +} + +static VALUE protected_end_stream(VALUE handler) +{ + return rb_funcall(handler, id_end_stream, 0); +} + /* * call-seq: * parser.parse(yaml) @@ -170,6 +232,7 @@ yaml_event_t event; int done = 0; int tainted = 0; + int state = 0; int parser_encoding = YAML_ANY_ENCODING; #ifdef HAVE_RUBY_ENCODING_H int encoding = rb_utf8_encindex(); @@ -223,14 +286,18 @@ } switch(event.type) { - case YAML_STREAM_START_EVENT: + case YAML_STREAM_START_EVENT: + { + VALUE args[2]; - rb_funcall(handler, id_start_stream, 1, - INT2NUM((long)event.data.stream_start.encoding) - ); - break; + args[0] = handler; + args[1] = INT2NUM((long)event.data.stream_start.encoding); + rb_protect(protected_start_stream, (VALUE)args, &state); + } + break; case YAML_DOCUMENT_START_EVENT: { + VALUE args[4]; /* Get a list of tag directives (if any) */ VALUE tag_directives = rb_ary_new(); /* Grab the document version */ @@ -268,19 +335,25 @@ rb_ary_push(tag_directives, rb_ary_new3((long)2, handle, prefix)); } } - rb_funcall(handler, id_start_document, 3, - version, tag_directives, - event.data.document_start.implicit == 1 ? Qtrue : Qfalse - ); + args[0] = handler; + args[1] = version; + args[2] = tag_directives; + args[3] = event.data.document_start.implicit == 1 ? Qtrue : Qfalse; + rb_protect(protected_start_document, (VALUE)args, &state); } break; case YAML_DOCUMENT_END_EVENT: - rb_funcall(handler, id_end_document, 1, - event.data.document_end.implicit == 1 ? Qtrue : Qfalse - ); + { + VALUE args[2]; + + args[0] = handler; + args[1] = event.data.document_end.implicit == 1 ? Qtrue : Qfalse; + rb_protect(protected_end_document, (VALUE)args, &state); + } break; case YAML_ALIAS_EVENT: { + VALUE args[2]; VALUE alias = Qnil; if(event.data.alias.anchor) { alias = rb_str_new2((const char *)event.data.alias.anchor); @@ -290,11 +363,14 @@ #endif } - rb_funcall(handler, id_alias, 1, alias); + args[0] = handler; + args[1] = alias; + rb_protect(protected_alias, (VALUE)args, &state); } break; case YAML_SCALAR_EVENT: { + VALUE args[7]; VALUE anchor = Qnil; VALUE tag = Qnil; VALUE plain_implicit, quoted_implicit, style; @@ -332,12 +408,19 @@ style = INT2NUM((long)event.data.scalar.style); - rb_funcall(handler, id_scalar, 6, - val, anchor, tag, plain_implicit, quoted_implicit, style); + args[0] = handler; + args[1] = val; + args[2] = anchor; + args[3] = tag; + args[4] = plain_implicit; + args[5] = quoted_implicit; + args[6] = style; + rb_protect(protected_scalar, (VALUE)args, &state); } break; case YAML_SEQUENCE_START_EVENT: { + VALUE args[5]; VALUE anchor = Qnil; VALUE tag = Qnil; VALUE implicit, style; @@ -363,15 +446,21 @@ style = INT2NUM((long)event.data.sequence_start.style); - rb_funcall(handler, id_start_sequence, 4, - anchor, tag, implicit, style); + args[0] = handler; + args[1] = anchor; + args[2] = tag; + args[3] = implicit; + args[4] = style; + + rb_protect(protected_start_sequence, (VALUE)args, &state); } break; case YAML_SEQUENCE_END_EVENT: - rb_funcall(handler, id_end_sequence, 0); + rb_protect(protected_end_sequence, handler, &state); break; case YAML_MAPPING_START_EVENT: { + VALUE args[5]; VALUE anchor = Qnil; VALUE tag = Qnil; VALUE implicit, style; @@ -396,22 +485,28 @@ style = INT2NUM((long)event.data.mapping_start.style); - rb_funcall(handler, id_start_mapping, 4, - anchor, tag, implicit, style); + args[0] = handler; + args[1] = anchor; + args[2] = tag; + args[3] = implicit; + args[4] = style; + + rb_protect(protected_start_mapping, (VALUE)args, &state); } break; case YAML_MAPPING_END_EVENT: - rb_funcall(handler, id_end_mapping, 0); + rb_protect(protected_end_mapping, handler, &state); break; case YAML_NO_EVENT: - rb_funcall(handler, id_empty, 0); + rb_protect(protected_empty, handler, &state); break; case YAML_STREAM_END_EVENT: - rb_funcall(handler, id_end_stream, 0); + rb_protect(protected_end_stream, handler, &state); done = 1; break; } yaml_event_delete(&event); + if (state) rb_jump_tag(state); } return self; Index: test/psych/test_parser.rb =================================================================== --- test/psych/test_parser.rb (revision 34782) +++ test/psych/test_parser.rb (revision 34783) @@ -32,6 +32,36 @@ @handler.parser = @parser end + def test_exception_memory_leak + yaml = <<-eoyaml +%YAML 1.1 +%TAG ! tag:tenderlovemaking.com,2009: +--- &ponies +- first element +- *ponies +- foo: bar +... + eoyaml + + [:start_stream, :start_document, :end_document, :alias, :scalar, + :start_sequence, :end_sequence, :start_mapping, :end_mapping, + :end_stream].each do |method| + + klass = Class.new(Psych::Handler) do + define_method(method) do |*args| + raise + end + end + + parser = Psych::Parser.new klass.new + 2.times { + assert_raises(RuntimeError, method.to_s) do + parser.parse yaml + end + } + end + end + def test_multiparse 3.times do @parser.parse '--- foo' -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/