Fix three defects in Psych::Parser#parse

Gemfile

@@ -6,6 +6,6 @@ group :development do
  gem 'rake-compiler', ">= 0.4.1"
  gem 'ruby-maven', :platforms => :jruby
  gem 'test-unit'
- gem 'test-unit-ruby-core', ">= 1.0.7"
+ gem 'test-unit-ruby-core', ">= 1.0.16"
  gem 'power_assert', '~> 2.0' if RUBY_VERSION < '3.0' # https://github.com/ruby/power_assert/pull/61
 end

ext/psych/psych_parser.c

@@ -52,19 +52,28 @@ static int io_reader(void * data, unsigned char *buf, size_t size, size_t *read)
  return 1;
 }
 
+/* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() reinitialises the
+ * parser it is handed, which would pull the input out from under the loop
+ * still driving it, so keep a flag to reject a reentrant call. */
+typedef struct {
+ yaml_parser_t yaml_parser;
+ int parsing;
+} psych_parser_t;
+
 static void dealloc(void * ptr)
 {
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- parser = (yaml_parser_t *)ptr;
- yaml_parser_delete(parser);
+ parser = (psych_parser_t *)ptr;
+ yaml_parser_delete(&parser->yaml_parser);
  xfree(parser);
 }
 
 #if 0
 static size_t memsize(const void *ptr)
 {
- const yaml_parser_t *parser = ptr;
+ const psych_parser_t *parser = ptr;
  /* TODO: calculate parser's size */
  return 0;
 }
@@ -81,10 +90,10 @@ static const rb_data_type_t psych_parser_type = {
 
 static VALUE allocate(VALUE klass)
 {
-yaml_parser_t * parser;
- VALUE obj = TypedData_Make_Struct(klass, yaml_parser_t, &psych_parser_type, parser);
+psych_parser_t * parser;
+ VALUE obj = TypedData_Make_Struct(klass, psych_parser_t, &psych_parser_type, parser);
 
- yaml_parser_initialize(parser);
+ yaml_parser_initialize(&parser->yaml_parser);
 
  return obj;
 }
@@ -257,18 +266,29 @@ static VALUE protected_event_location(VALUE pointer)
  return rb_funcall3(args[0], id_event_location, 4, args + 1);
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+struct parse_args {
+ psych_parser_t * psych_parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
 {
- yaml_parser_t * parser;
+ struct parse_args * pargs = (struct parse_args *)ptr;
+ yaml_parser_t * parser = &pargs->psych_parser->yaml_parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  yaml_event_t event;
  int done = 0;
  int state = 0;
  int parser_encoding = YAML_ANY_ENCODING;
  int encoding = rb_utf8_encindex();
  rb_encoding * internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
-
  yaml_parser_delete(parser);
  yaml_parser_initialize(parser);
 
@@ -312,6 +332,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = end_line;
  event_args[4] = end_column;
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ yaml_event_delete(&event);
+ rb_jump_tag(state);
+ }
 
  switch(event.type) {
  case YAML_STREAM_START_EVENT:
@@ -496,7 +520,11 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case YAML_NO_EVENT:
+ /* Once libyaml has produced the stream end, every later call
+ * succeeds with a zeroed event and YAML_STREAM_END_EVENT can no
+ * longer be reached. Stop rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case YAML_STREAM_END_EVENT:
  rb_protect(protected_end_stream, handler, &state);
@@ -510,6 +538,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_parser_t * parser = (psych_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_parser_t * parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.psych_parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>
@@ -521,13 +580,13 @@ static VALUE mark(VALUE self)
 {
  VALUE mark_klass;
  VALUE args[3];
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
  mark_klass = rb_const_get_at(cPsychParser, rb_intern("Mark"));
- args[0] = SIZET2NUM(parser->mark.index);
- args[1] = SIZET2NUM(parser->mark.line);
- args[2] = SIZET2NUM(parser->mark.column);
+ args[0] = SIZET2NUM(parser->yaml_parser.mark.index);
+ args[1] = SIZET2NUM(parser->yaml_parser.mark.line);
+ args[2] = SIZET2NUM(parser->yaml_parser.mark.column);
 
  return rb_class_new_instance(3, args, mark_klass);
 }

ext/psych/psych_parser_fy.c

@@ -41,6 +41,11 @@ typedef struct {
  size_t mark_line;
  size_t mark_column;
  size_t mark_index;
+ /* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() destroys and
+ * recreates fyp, which would pull the parser out from under the loop still
+ * driving it, so keep a flag to reject a reentrant call. */
+ int parsing;
 } psych_fy_parser_t;
 
 static const struct fy_parse_cfg psych_parse_cfg = {
@@ -256,17 +261,28 @@ static VALUE token_to_str(struct fy_token *tok, int encoding, rb_encoding *inter
  return str;
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
-{
+struct parse_args {
  psych_fy_parser_t *parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
+{
+ struct parse_args *pargs = (struct parse_args *)ptr;
+ psych_fy_parser_t *parser = pargs->parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  struct fy_event *event;
  int done = 0;
  int state = 0;
  int encoding = rb_utf8_encindex();
  rb_encoding *internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
-
  /* Use a pristine parser for each parse, like fy-tool does. Reusing a
  * parser across documents via fy_parser_reset() left the default tag
  * handles unset for bare (no "---") tag-led documents. */
@@ -348,6 +364,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = SIZET2NUM(em ? (size_t)em->line : 0);
  event_args[4] = SIZET2NUM(em ? (size_t)em->column : 0);
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ fy_parser_event_free(parser->fyp, event);
+ rb_jump_tag(state);
+ }
 
  switch (event->type) {
  case FYET_STREAM_START:
@@ -473,7 +493,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case FYET_NONE:
+ /* An event with no type cannot advance the stream, so stop
+ * rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case FYET_STREAM_END:
  rb_protect(protected_end_stream, handler, &state);
@@ -489,6 +512,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_fy_parser_t *parser = (psych_fy_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_fy_parser_t *parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>

test/psych/test_parser.rb

@@ -26,6 +26,39 @@ def #{m} *args
  end
  end
 
+ # Calls Parser#parse again, once, from inside a callback of the parse it is
+ # already handling.
+ class ReentrantHandler < Handler
+ attr_accessor :parser, :inner_yaml
+ attr_reader :inner_error, :scalars, :empty_calls
+
+ def initialize
+ @parser = nil
+ @inner_yaml = nil
+ @inner_error = nil
+ @scalars = []
+ @empty_calls = 0
+ end
+
+ def empty
+ @empty_calls += 1
+ raise "handler#empty keeps being called, the parse loop is not terminating" if @empty_calls > 1000
+ end
+
+ def scalar value, anchor, tag, plain, quoted, style
+ @scalars << value
+
+ inner, @inner_yaml = @inner_yaml, nil
+ return unless inner
+
+ begin
+ @parser.parse inner
+ rescue => e
+ @inner_error = e
+ end
+ end
+ end
+
  def setup
  super
  @handler = EventCatcher.new
@@ -70,6 +103,56 @@ def test_exception_memory_leak
  end
  end
 
+ def test_event_location_exception_is_propagated
+ klass = Class.new(Psych::Handler) do
+ def event_location start_line, start_column, end_line, end_column
+ raise "from event_location"
+ end
+ end
+
+ parser = Psych::Parser.new klass.new
+ 2.times do
+ ex = assert_raise(RuntimeError) { parser.parse "--- hello\n" }
+ assert_equal "from event_location", ex.message
+ end
+ end
+
+ def test_parse_is_not_reentrant
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- inner\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+
+ # The in-use flag is cleared when the parse finishes, so the same parser
+ # can be used again afterwards.
+ handler.scalars.clear
+ parser.parse "--- second\n"
+ assert_equal ['second'], handler.scalars
+ end
+
+ def test_parse_is_not_reentrant_with_invalid_inner_document
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- \x00bad\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+ end
+
  def test_multiparse
  3.times do
  @parser.parse '--- foo'

Gemfile

@@ -6,6 +6,6 @@ group :development do
  gem 'rake-compiler', ">= 0.4.1"
  gem 'ruby-maven', :platforms => :jruby
  gem 'test-unit'
- gem 'test-unit-ruby-core', ">= 1.0.7"
+ gem 'test-unit-ruby-core', ">= 1.0.16"
  gem 'power_assert', '~> 2.0' if RUBY_VERSION < '3.0' # https://github.com/ruby/power_assert/pull/61
 end

ext/psych/psych_parser.c

@@ -52,19 +52,28 @@ static int io_reader(void * data, unsigned char *buf, size_t size, size_t *read)
  return 1;
 }
 
+/* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() reinitialises the
+ * parser it is handed, which would pull the input out from under the loop
+ * still driving it, so keep a flag to reject a reentrant call. */
+typedef struct {
+ yaml_parser_t yaml_parser;
+ int parsing;
+} psych_parser_t;
+
 static void dealloc(void * ptr)
 {
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- parser = (yaml_parser_t *)ptr;
- yaml_parser_delete(parser);
+ parser = (psych_parser_t *)ptr;
+ yaml_parser_delete(&parser->yaml_parser);
  xfree(parser);
 }
 
 #if 0
 static size_t memsize(const void *ptr)
 {
- const yaml_parser_t *parser = ptr;
+ const psych_parser_t *parser = ptr;
  /* TODO: calculate parser's size */
  return 0;
 }
@@ -81,10 +90,10 @@ static const rb_data_type_t psych_parser_type = {
 
 static VALUE allocate(VALUE klass)
 {
-yaml_parser_t * parser;
- VALUE obj = TypedData_Make_Struct(klass, yaml_parser_t, &psych_parser_type, parser);
+psych_parser_t * parser;
+ VALUE obj = TypedData_Make_Struct(klass, psych_parser_t, &psych_parser_type, parser);
 
- yaml_parser_initialize(parser);
+ yaml_parser_initialize(&parser->yaml_parser);
 
  return obj;
 }
@@ -257,18 +266,29 @@ static VALUE protected_event_location(VALUE pointer)
  return rb_funcall3(args[0], id_event_location, 4, args + 1);
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+struct parse_args {
+ psych_parser_t * psych_parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
 {
- yaml_parser_t * parser;
+ struct parse_args * pargs = (struct parse_args *)ptr;
+ yaml_parser_t * parser = &pargs->psych_parser->yaml_parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  yaml_event_t event;
  int done = 0;
  int state = 0;
  int parser_encoding = YAML_ANY_ENCODING;
  int encoding = rb_utf8_encindex();
  rb_encoding * internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
-
  yaml_parser_delete(parser);
  yaml_parser_initialize(parser);
 
@@ -312,6 +332,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = end_line;
  event_args[4] = end_column;
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ yaml_event_delete(&event);
+ rb_jump_tag(state);
+ }
 
  switch(event.type) {
  case YAML_STREAM_START_EVENT:
@@ -496,7 +520,11 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case YAML_NO_EVENT:
+ /* Once libyaml has produced the stream end, every later call
+ * succeeds with a zeroed event and YAML_STREAM_END_EVENT can no
+ * longer be reached. Stop rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case YAML_STREAM_END_EVENT:
  rb_protect(protected_end_stream, handler, &state);
@@ -510,6 +538,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_parser_t * parser = (psych_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_parser_t * parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.psych_parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>
@@ -521,13 +580,13 @@ static VALUE mark(VALUE self)
 {
  VALUE mark_klass;
  VALUE args[3];
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
  mark_klass = rb_const_get_at(cPsychParser, rb_intern("Mark"));
- args[0] = SIZET2NUM(parser->mark.index);
- args[1] = SIZET2NUM(parser->mark.line);
- args[2] = SIZET2NUM(parser->mark.column);
+ args[0] = SIZET2NUM(parser->yaml_parser.mark.index);
+ args[1] = SIZET2NUM(parser->yaml_parser.mark.line);
+ args[2] = SIZET2NUM(parser->yaml_parser.mark.column);
 
  return rb_class_new_instance(3, args, mark_klass);
 }

ext/psych/psych_parser_fy.c

@@ -41,6 +41,11 @@ typedef struct {
  size_t mark_line;
  size_t mark_column;
  size_t mark_index;
+ /* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() destroys and
+ * recreates fyp, which would pull the parser out from under the loop still
+ * driving it, so keep a flag to reject a reentrant call. */
+ int parsing;
 } psych_fy_parser_t;
 
 static const struct fy_parse_cfg psych_parse_cfg = {
@@ -256,17 +261,28 @@ static VALUE token_to_str(struct fy_token *tok, int encoding, rb_encoding *inter
  return str;
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
-{
+struct parse_args {
  psych_fy_parser_t *parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
+{
+ struct parse_args *pargs = (struct parse_args *)ptr;
+ psych_fy_parser_t *parser = pargs->parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  struct fy_event *event;
  int done = 0;
  int state = 0;
  int encoding = rb_utf8_encindex();
  rb_encoding *internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
-
  /* Use a pristine parser for each parse, like fy-tool does. Reusing a
  * parser across documents via fy_parser_reset() left the default tag
  * handles unset for bare (no "---") tag-led documents. */
@@ -348,6 +364,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = SIZET2NUM(em ? (size_t)em->line : 0);
  event_args[4] = SIZET2NUM(em ? (size_t)em->column : 0);
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ fy_parser_event_free(parser->fyp, event);
+ rb_jump_tag(state);
+ }
 
  switch (event->type) {
  case FYET_STREAM_START:
@@ -473,7 +493,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case FYET_NONE:
+ /* An event with no type cannot advance the stream, so stop
+ * rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case FYET_STREAM_END:
  rb_protect(protected_end_stream, handler, &state);
@@ -489,6 +512,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_fy_parser_t *parser = (psych_fy_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_fy_parser_t *parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>

test/psych/test_parser.rb

@@ -26,6 +26,39 @@ def #{m} *args
  end
  end
 
+ # Calls Parser#parse again, once, from inside a callback of the parse it is
+ # already handling.
+ class ReentrantHandler < Handler
+ attr_accessor :parser, :inner_yaml
+ attr_reader :inner_error, :scalars, :empty_calls
+
+ def initialize
+ @parser = nil
+ @inner_yaml = nil
+ @inner_error = nil
+ @scalars = []
+ @empty_calls = 0
+ end
+
+ def empty
+ @empty_calls += 1
+ raise "handler#empty keeps being called, the parse loop is not terminating" if @empty_calls > 1000
+ end
+
+ def scalar value, anchor, tag, plain, quoted, style
+ @scalars << value
+
+ inner, @inner_yaml = @inner_yaml, nil
+ return unless inner
+
+ begin
+ @parser.parse inner
+ rescue => e
+ @inner_error = e
+ end
+ end
+ end
+
  def setup
  super
  @handler = EventCatcher.new
@@ -70,6 +103,56 @@ def test_exception_memory_leak
  end
  end
 
+ def test_event_location_exception_is_propagated
+ klass = Class.new(Psych::Handler) do
+ def event_location start_line, start_column, end_line, end_column
+ raise "from event_location"
+ end
+ end
+
+ parser = Psych::Parser.new klass.new
+ 2.times do
+ ex = assert_raise(RuntimeError) { parser.parse "--- hello\n" }
+ assert_equal "from event_location", ex.message
+ end
+ end
+
+ def test_parse_is_not_reentrant
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- inner\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+
+ # The in-use flag is cleared when the parse finishes, so the same parser
+ # can be used again afterwards.
+ handler.scalars.clear
+ parser.parse "--- second\n"
+ assert_equal ['second'], handler.scalars
+ end
+
+ def test_parse_is_not_reentrant_with_invalid_inner_document
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- \x00bad\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+ end
+
  def test_multiparse
  3.times do
  @parser.parse '--- foo'

Gemfile

@@ -6,6 +6,6 @@ group :development do
  gem 'rake-compiler', ">= 0.4.1"
  gem 'ruby-maven', :platforms => :jruby
  gem 'test-unit'
- gem 'test-unit-ruby-core', ">= 1.0.7"
+ gem 'test-unit-ruby-core', ">= 1.0.16"
  gem 'power_assert', '~> 2.0' if RUBY_VERSION < '3.0' # https://github.com/ruby/power_assert/pull/61
 end

ext/psych/psych_parser.c

@@ -52,19 +52,28 @@ static int io_reader(void * data, unsigned char *buf, size_t size, size_t *read)
  return 1;
 }
 
+/* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() reinitialises the
+ * parser it is handed, which would pull the input out from under the loop
+ * still driving it, so keep a flag to reject a reentrant call. */
+typedef struct {
+ yaml_parser_t yaml_parser;
+ int parsing;
+} psych_parser_t;
+
 static void dealloc(void * ptr)
 {
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- parser = (yaml_parser_t *)ptr;
- yaml_parser_delete(parser);
+ parser = (psych_parser_t *)ptr;
+ yaml_parser_delete(&parser->yaml_parser);
  xfree(parser);
 }
 
 #if 0
 static size_t memsize(const void *ptr)
 {
- const yaml_parser_t *parser = ptr;
+ const psych_parser_t *parser = ptr;
  /* TODO: calculate parser's size */
  return 0;
 }
@@ -81,10 +90,10 @@ static const rb_data_type_t psych_parser_type = {
 
 static VALUE allocate(VALUE klass)
 {
-yaml_parser_t * parser;
- VALUE obj = TypedData_Make_Struct(klass, yaml_parser_t, &psych_parser_type, parser);
+psych_parser_t * parser;
+ VALUE obj = TypedData_Make_Struct(klass, psych_parser_t, &psych_parser_type, parser);
 
- yaml_parser_initialize(parser);
+ yaml_parser_initialize(&parser->yaml_parser);
 
  return obj;
 }
@@ -257,18 +266,29 @@ static VALUE protected_event_location(VALUE pointer)
  return rb_funcall3(args[0], id_event_location, 4, args + 1);
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+struct parse_args {
+ psych_parser_t * psych_parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
 {
- yaml_parser_t * parser;
+ struct parse_args * pargs = (struct parse_args *)ptr;
+ yaml_parser_t * parser = &pargs->psych_parser->yaml_parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  yaml_event_t event;
  int done = 0;
  int state = 0;
  int parser_encoding = YAML_ANY_ENCODING;
  int encoding = rb_utf8_encindex();
  rb_encoding * internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
-
  yaml_parser_delete(parser);
  yaml_parser_initialize(parser);
 
@@ -312,6 +332,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = end_line;
  event_args[4] = end_column;
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ yaml_event_delete(&event);
+ rb_jump_tag(state);
+ }
 
  switch(event.type) {
  case YAML_STREAM_START_EVENT:
@@ -496,7 +520,11 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case YAML_NO_EVENT:
+ /* Once libyaml has produced the stream end, every later call
+ * succeeds with a zeroed event and YAML_STREAM_END_EVENT can no
+ * longer be reached. Stop rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case YAML_STREAM_END_EVENT:
  rb_protect(protected_end_stream, handler, &state);
@@ -510,6 +538,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_parser_t * parser = (psych_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_parser_t * parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.psych_parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>
@@ -521,13 +580,13 @@ static VALUE mark(VALUE self)
 {
  VALUE mark_klass;
  VALUE args[3];
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
  mark_klass = rb_const_get_at(cPsychParser, rb_intern("Mark"));
- args[0] = SIZET2NUM(parser->mark.index);
- args[1] = SIZET2NUM(parser->mark.line);
- args[2] = SIZET2NUM(parser->mark.column);
+ args[0] = SIZET2NUM(parser->yaml_parser.mark.index);
+ args[1] = SIZET2NUM(parser->yaml_parser.mark.line);
+ args[2] = SIZET2NUM(parser->yaml_parser.mark.column);
 
  return rb_class_new_instance(3, args, mark_klass);
 }

ext/psych/psych_parser_fy.c

@@ -41,6 +41,11 @@ typedef struct {
  size_t mark_line;
  size_t mark_column;
  size_t mark_index;
+ /* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() destroys and
+ * recreates fyp, which would pull the parser out from under the loop still
+ * driving it, so keep a flag to reject a reentrant call. */
+ int parsing;
 } psych_fy_parser_t;
 
 static const struct fy_parse_cfg psych_parse_cfg = {
@@ -256,17 +261,28 @@ static VALUE token_to_str(struct fy_token *tok, int encoding, rb_encoding *inter
  return str;
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
-{
+struct parse_args {
  psych_fy_parser_t *parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
+{
+ struct parse_args *pargs = (struct parse_args *)ptr;
+ psych_fy_parser_t *parser = pargs->parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  struct fy_event *event;
  int done = 0;
  int state = 0;
  int encoding = rb_utf8_encindex();
  rb_encoding *internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
-
  /* Use a pristine parser for each parse, like fy-tool does. Reusing a
  * parser across documents via fy_parser_reset() left the default tag
  * handles unset for bare (no "---") tag-led documents. */
@@ -348,6 +364,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = SIZET2NUM(em ? (size_t)em->line : 0);
  event_args[4] = SIZET2NUM(em ? (size_t)em->column : 0);
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ fy_parser_event_free(parser->fyp, event);
+ rb_jump_tag(state);
+ }
 
  switch (event->type) {
  case FYET_STREAM_START:
@@ -473,7 +493,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case FYET_NONE:
+ /* An event with no type cannot advance the stream, so stop
+ * rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case FYET_STREAM_END:
  rb_protect(protected_end_stream, handler, &state);
@@ -489,6 +512,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_fy_parser_t *parser = (psych_fy_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_fy_parser_t *parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>

test/psych/test_parser.rb

@@ -26,6 +26,39 @@ def #{m} *args
  end
  end
 
+ # Calls Parser#parse again, once, from inside a callback of the parse it is
+ # already handling.
+ class ReentrantHandler < Handler
+ attr_accessor :parser, :inner_yaml
+ attr_reader :inner_error, :scalars, :empty_calls
+
+ def initialize
+ @parser = nil
+ @inner_yaml = nil
+ @inner_error = nil
+ @scalars = []
+ @empty_calls = 0
+ end
+
+ def empty
+ @empty_calls += 1
+ raise "handler#empty keeps being called, the parse loop is not terminating" if @empty_calls > 1000
+ end
+
+ def scalar value, anchor, tag, plain, quoted, style
+ @scalars << value
+
+ inner, @inner_yaml = @inner_yaml, nil
+ return unless inner
+
+ begin
+ @parser.parse inner
+ rescue => e
+ @inner_error = e
+ end
+ end
+ end
+
  def setup
  super
  @handler = EventCatcher.new
@@ -70,6 +103,56 @@ def test_exception_memory_leak
  end
  end
 
+ def test_event_location_exception_is_propagated
+ klass = Class.new(Psych::Handler) do
+ def event_location start_line, start_column, end_line, end_column
+ raise "from event_location"
+ end
+ end
+
+ parser = Psych::Parser.new klass.new
+ 2.times do
+ ex = assert_raise(RuntimeError) { parser.parse "--- hello\n" }
+ assert_equal "from event_location", ex.message
+ end
+ end
+
+ def test_parse_is_not_reentrant
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- inner\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+
+ # The in-use flag is cleared when the parse finishes, so the same parser
+ # can be used again afterwards.
+ handler.scalars.clear
+ parser.parse "--- second\n"
+ assert_equal ['second'], handler.scalars
+ end
+
+ def test_parse_is_not_reentrant_with_invalid_inner_document
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- \x00bad\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+ end
+
  def test_multiparse
  3.times do
  @parser.parse '--- foo'

Gemfile

@@ -6,6 +6,6 @@ group :development do
  gem 'rake-compiler', ">= 0.4.1"
  gem 'ruby-maven', :platforms => :jruby
  gem 'test-unit'
- gem 'test-unit-ruby-core', ">= 1.0.7"
+ gem 'test-unit-ruby-core', ">= 1.0.16"
  gem 'power_assert', '~> 2.0' if RUBY_VERSION < '3.0' # https://github.com/ruby/power_assert/pull/61
 end

ext/psych/psych_parser.c

@@ -52,19 +52,28 @@ static int io_reader(void * data, unsigned char *buf, size_t size, size_t *read)
  return 1;
 }
 
+/* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() reinitialises the
+ * parser it is handed, which would pull the input out from under the loop
+ * still driving it, so keep a flag to reject a reentrant call. */
+typedef struct {
+ yaml_parser_t yaml_parser;
+ int parsing;
+} psych_parser_t;
+
 static void dealloc(void * ptr)
 {
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- parser = (yaml_parser_t *)ptr;
- yaml_parser_delete(parser);
+ parser = (psych_parser_t *)ptr;
+ yaml_parser_delete(&parser->yaml_parser);
  xfree(parser);
 }
 
 #if 0
 static size_t memsize(const void *ptr)
 {
- const yaml_parser_t *parser = ptr;
+ const psych_parser_t *parser = ptr;
  /* TODO: calculate parser's size */
  return 0;
 }
@@ -81,10 +90,10 @@ static const rb_data_type_t psych_parser_type = {
 
 static VALUE allocate(VALUE klass)
 {
-yaml_parser_t * parser;
- VALUE obj = TypedData_Make_Struct(klass, yaml_parser_t, &psych_parser_type, parser);
+psych_parser_t * parser;
+ VALUE obj = TypedData_Make_Struct(klass, psych_parser_t, &psych_parser_type, parser);
 
- yaml_parser_initialize(parser);
+ yaml_parser_initialize(&parser->yaml_parser);
 
  return obj;
 }
@@ -257,18 +266,29 @@ static VALUE protected_event_location(VALUE pointer)
  return rb_funcall3(args[0], id_event_location, 4, args + 1);
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+struct parse_args {
+ psych_parser_t * psych_parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
 {
- yaml_parser_t * parser;
+ struct parse_args * pargs = (struct parse_args *)ptr;
+ yaml_parser_t * parser = &pargs->psych_parser->yaml_parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  yaml_event_t event;
  int done = 0;
  int state = 0;
  int parser_encoding = YAML_ANY_ENCODING;
  int encoding = rb_utf8_encindex();
  rb_encoding * internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
-
  yaml_parser_delete(parser);
  yaml_parser_initialize(parser);
 
@@ -312,6 +332,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = end_line;
  event_args[4] = end_column;
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ yaml_event_delete(&event);
+ rb_jump_tag(state);
+ }
 
  switch(event.type) {
  case YAML_STREAM_START_EVENT:
@@ -496,7 +520,11 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case YAML_NO_EVENT:
+ /* Once libyaml has produced the stream end, every later call
+ * succeeds with a zeroed event and YAML_STREAM_END_EVENT can no
+ * longer be reached. Stop rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case YAML_STREAM_END_EVENT:
  rb_protect(protected_end_stream, handler, &state);
@@ -510,6 +538,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_parser_t * parser = (psych_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_parser_t * parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.psych_parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>
@@ -521,13 +580,13 @@ static VALUE mark(VALUE self)
 {
  VALUE mark_klass;
  VALUE args[3];
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
  mark_klass = rb_const_get_at(cPsychParser, rb_intern("Mark"));
- args[0] = SIZET2NUM(parser->mark.index);
- args[1] = SIZET2NUM(parser->mark.line);
- args[2] = SIZET2NUM(parser->mark.column);
+ args[0] = SIZET2NUM(parser->yaml_parser.mark.index);
+ args[1] = SIZET2NUM(parser->yaml_parser.mark.line);
+ args[2] = SIZET2NUM(parser->yaml_parser.mark.column);
 
  return rb_class_new_instance(3, args, mark_klass);
 }

ext/psych/psych_parser_fy.c

@@ -41,6 +41,11 @@ typedef struct {
  size_t mark_line;
  size_t mark_column;
  size_t mark_index;
+ /* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() destroys and
+ * recreates fyp, which would pull the parser out from under the loop still
+ * driving it, so keep a flag to reject a reentrant call. */
+ int parsing;
 } psych_fy_parser_t;
 
 static const struct fy_parse_cfg psych_parse_cfg = {
@@ -256,17 +261,28 @@ static VALUE token_to_str(struct fy_token *tok, int encoding, rb_encoding *inter
  return str;
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
-{
+struct parse_args {
  psych_fy_parser_t *parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
+{
+ struct parse_args *pargs = (struct parse_args *)ptr;
+ psych_fy_parser_t *parser = pargs->parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  struct fy_event *event;
  int done = 0;
  int state = 0;
  int encoding = rb_utf8_encindex();
  rb_encoding *internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
-
  /* Use a pristine parser for each parse, like fy-tool does. Reusing a
  * parser across documents via fy_parser_reset() left the default tag
  * handles unset for bare (no "---") tag-led documents. */
@@ -348,6 +364,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = SIZET2NUM(em ? (size_t)em->line : 0);
  event_args[4] = SIZET2NUM(em ? (size_t)em->column : 0);
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ fy_parser_event_free(parser->fyp, event);
+ rb_jump_tag(state);
+ }
 
  switch (event->type) {
  case FYET_STREAM_START:
@@ -473,7 +493,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case FYET_NONE:
+ /* An event with no type cannot advance the stream, so stop
+ * rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case FYET_STREAM_END:
  rb_protect(protected_end_stream, handler, &state);
@@ -489,6 +512,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_fy_parser_t *parser = (psych_fy_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_fy_parser_t *parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>

test/psych/test_parser.rb

@@ -26,6 +26,39 @@ def #{m} *args
  end
  end
 
+ # Calls Parser#parse again, once, from inside a callback of the parse it is
+ # already handling.
+ class ReentrantHandler < Handler
+ attr_accessor :parser, :inner_yaml
+ attr_reader :inner_error, :scalars, :empty_calls
+
+ def initialize
+ @parser = nil
+ @inner_yaml = nil
+ @inner_error = nil
+ @scalars = []
+ @empty_calls = 0
+ end
+
+ def empty
+ @empty_calls += 1
+ raise "handler#empty keeps being called, the parse loop is not terminating" if @empty_calls > 1000
+ end
+
+ def scalar value, anchor, tag, plain, quoted, style
+ @scalars << value
+
+ inner, @inner_yaml = @inner_yaml, nil
+ return unless inner
+
+ begin
+ @parser.parse inner
+ rescue => e
+ @inner_error = e
+ end
+ end
+ end
+
  def setup
  super
  @handler = EventCatcher.new
@@ -70,6 +103,56 @@ def test_exception_memory_leak
  end
  end
 
+ def test_event_location_exception_is_propagated
+ klass = Class.new(Psych::Handler) do
+ def event_location start_line, start_column, end_line, end_column
+ raise "from event_location"
+ end
+ end
+
+ parser = Psych::Parser.new klass.new
+ 2.times do
+ ex = assert_raise(RuntimeError) { parser.parse "--- hello\n" }
+ assert_equal "from event_location", ex.message
+ end
+ end
+
+ def test_parse_is_not_reentrant
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- inner\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+
+ # The in-use flag is cleared when the parse finishes, so the same parser
+ # can be used again afterwards.
+ handler.scalars.clear
+ parser.parse "--- second\n"
+ assert_equal ['second'], handler.scalars
+ end
+
+ def test_parse_is_not_reentrant_with_invalid_inner_document
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- \x00bad\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+ end
+
  def test_multiparse
  3.times do
  @parser.parse '--- foo'

Gemfile

@@ -6,6 +6,6 @@ group :development do
  gem 'rake-compiler', ">= 0.4.1"
  gem 'ruby-maven', :platforms => :jruby
  gem 'test-unit'
- gem 'test-unit-ruby-core', ">= 1.0.7"
+ gem 'test-unit-ruby-core', ">= 1.0.16"
  gem 'power_assert', '~> 2.0' if RUBY_VERSION < '3.0' # https://github.com/ruby/power_assert/pull/61
 end

ext/psych/psych_parser.c

@@ -52,19 +52,28 @@ static int io_reader(void * data, unsigned char *buf, size_t size, size_t *read)
  return 1;
 }
 
+/* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() reinitialises the
+ * parser it is handed, which would pull the input out from under the loop
+ * still driving it, so keep a flag to reject a reentrant call. */
+typedef struct {
+ yaml_parser_t yaml_parser;
+ int parsing;
+} psych_parser_t;
+
 static void dealloc(void * ptr)
 {
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- parser = (yaml_parser_t *)ptr;
- yaml_parser_delete(parser);
+ parser = (psych_parser_t *)ptr;
+ yaml_parser_delete(&parser->yaml_parser);
  xfree(parser);
 }
 
 #if 0
 static size_t memsize(const void *ptr)
 {
- const yaml_parser_t *parser = ptr;
+ const psych_parser_t *parser = ptr;
  /* TODO: calculate parser's size */
  return 0;
 }
@@ -81,10 +90,10 @@ static const rb_data_type_t psych_parser_type = {
 
 static VALUE allocate(VALUE klass)
 {
-yaml_parser_t * parser;
- VALUE obj = TypedData_Make_Struct(klass, yaml_parser_t, &psych_parser_type, parser);
+psych_parser_t * parser;
+ VALUE obj = TypedData_Make_Struct(klass, psych_parser_t, &psych_parser_type, parser);
 
- yaml_parser_initialize(parser);
+ yaml_parser_initialize(&parser->yaml_parser);
 
  return obj;
 }
@@ -257,18 +266,29 @@ static VALUE protected_event_location(VALUE pointer)
  return rb_funcall3(args[0], id_event_location, 4, args + 1);
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+struct parse_args {
+ psych_parser_t * psych_parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
 {
- yaml_parser_t * parser;
+ struct parse_args * pargs = (struct parse_args *)ptr;
+ yaml_parser_t * parser = &pargs->psych_parser->yaml_parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  yaml_event_t event;
  int done = 0;
  int state = 0;
  int parser_encoding = YAML_ANY_ENCODING;
  int encoding = rb_utf8_encindex();
  rb_encoding * internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
-
  yaml_parser_delete(parser);
  yaml_parser_initialize(parser);
 
@@ -312,6 +332,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = end_line;
  event_args[4] = end_column;
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ yaml_event_delete(&event);
+ rb_jump_tag(state);
+ }
 
  switch(event.type) {
  case YAML_STREAM_START_EVENT:
@@ -496,7 +520,11 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case YAML_NO_EVENT:
+ /* Once libyaml has produced the stream end, every later call
+ * succeeds with a zeroed event and YAML_STREAM_END_EVENT can no
+ * longer be reached. Stop rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case YAML_STREAM_END_EVENT:
  rb_protect(protected_end_stream, handler, &state);
@@ -510,6 +538,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_parser_t * parser = (psych_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_parser_t * parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.psych_parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>
@@ -521,13 +580,13 @@ static VALUE mark(VALUE self)
 {
  VALUE mark_klass;
  VALUE args[3];
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
  mark_klass = rb_const_get_at(cPsychParser, rb_intern("Mark"));
- args[0] = SIZET2NUM(parser->mark.index);
- args[1] = SIZET2NUM(parser->mark.line);
- args[2] = SIZET2NUM(parser->mark.column);
+ args[0] = SIZET2NUM(parser->yaml_parser.mark.index);
+ args[1] = SIZET2NUM(parser->yaml_parser.mark.line);
+ args[2] = SIZET2NUM(parser->yaml_parser.mark.column);
 
  return rb_class_new_instance(3, args, mark_klass);
 }

ext/psych/psych_parser_fy.c

@@ -41,6 +41,11 @@ typedef struct {
  size_t mark_line;
  size_t mark_column;
  size_t mark_index;
+ /* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() destroys and
+ * recreates fyp, which would pull the parser out from under the loop still
+ * driving it, so keep a flag to reject a reentrant call. */
+ int parsing;
 } psych_fy_parser_t;
 
 static const struct fy_parse_cfg psych_parse_cfg = {
@@ -256,17 +261,28 @@ static VALUE token_to_str(struct fy_token *tok, int encoding, rb_encoding *inter
  return str;
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
-{
+struct parse_args {
  psych_fy_parser_t *parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
+{
+ struct parse_args *pargs = (struct parse_args *)ptr;
+ psych_fy_parser_t *parser = pargs->parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  struct fy_event *event;
  int done = 0;
  int state = 0;
  int encoding = rb_utf8_encindex();
  rb_encoding *internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
-
  /* Use a pristine parser for each parse, like fy-tool does. Reusing a
  * parser across documents via fy_parser_reset() left the default tag
  * handles unset for bare (no "---") tag-led documents. */
@@ -348,6 +364,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = SIZET2NUM(em ? (size_t)em->line : 0);
  event_args[4] = SIZET2NUM(em ? (size_t)em->column : 0);
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ fy_parser_event_free(parser->fyp, event);
+ rb_jump_tag(state);
+ }
 
  switch (event->type) {
  case FYET_STREAM_START:
@@ -473,7 +493,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case FYET_NONE:
+ /* An event with no type cannot advance the stream, so stop
+ * rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case FYET_STREAM_END:
  rb_protect(protected_end_stream, handler, &state);
@@ -489,6 +512,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_fy_parser_t *parser = (psych_fy_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_fy_parser_t *parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>

test/psych/test_parser.rb

@@ -26,6 +26,39 @@ def #{m} *args
  end
  end
 
+ # Calls Parser#parse again, once, from inside a callback of the parse it is
+ # already handling.
+ class ReentrantHandler < Handler
+ attr_accessor :parser, :inner_yaml
+ attr_reader :inner_error, :scalars, :empty_calls
+
+ def initialize
+ @parser = nil
+ @inner_yaml = nil
+ @inner_error = nil
+ @scalars = []
+ @empty_calls = 0
+ end
+
+ def empty
+ @empty_calls += 1
+ raise "handler#empty keeps being called, the parse loop is not terminating" if @empty_calls > 1000
+ end
+
+ def scalar value, anchor, tag, plain, quoted, style
+ @scalars << value
+
+ inner, @inner_yaml = @inner_yaml, nil
+ return unless inner
+
+ begin
+ @parser.parse inner
+ rescue => e
+ @inner_error = e
+ end
+ end
+ end
+
  def setup
  super
  @handler = EventCatcher.new
@@ -70,6 +103,56 @@ def test_exception_memory_leak
  end
  end
 
+ def test_event_location_exception_is_propagated
+ klass = Class.new(Psych::Handler) do
+ def event_location start_line, start_column, end_line, end_column
+ raise "from event_location"
+ end
+ end
+
+ parser = Psych::Parser.new klass.new
+ 2.times do
+ ex = assert_raise(RuntimeError) { parser.parse "--- hello\n" }
+ assert_equal "from event_location", ex.message
+ end
+ end
+
+ def test_parse_is_not_reentrant
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- inner\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+
+ # The in-use flag is cleared when the parse finishes, so the same parser
+ # can be used again afterwards.
+ handler.scalars.clear
+ parser.parse "--- second\n"
+ assert_equal ['second'], handler.scalars
+ end
+
+ def test_parse_is_not_reentrant_with_invalid_inner_document
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- \x00bad\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+ end
+
  def test_multiparse
  3.times do
  @parser.parse '--- foo'

Gemfile

@@ -6,6 +6,6 @@ group :development do
  gem 'rake-compiler', ">= 0.4.1"
  gem 'ruby-maven', :platforms => :jruby
  gem 'test-unit'
- gem 'test-unit-ruby-core', ">= 1.0.7"
+ gem 'test-unit-ruby-core', ">= 1.0.16"
  gem 'power_assert', '~> 2.0' if RUBY_VERSION < '3.0' # https://github.com/ruby/power_assert/pull/61
 end

ext/psych/psych_parser.c

@@ -52,19 +52,28 @@ static int io_reader(void * data, unsigned char *buf, size_t size, size_t *read)
  return 1;
 }
 
+/* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() reinitialises the
+ * parser it is handed, which would pull the input out from under the loop
+ * still driving it, so keep a flag to reject a reentrant call. */
+typedef struct {
+ yaml_parser_t yaml_parser;
+ int parsing;
+} psych_parser_t;
+
 static void dealloc(void * ptr)
 {
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- parser = (yaml_parser_t *)ptr;
- yaml_parser_delete(parser);
+ parser = (psych_parser_t *)ptr;
+ yaml_parser_delete(&parser->yaml_parser);
  xfree(parser);
 }
 
 #if 0
 static size_t memsize(const void *ptr)
 {
- const yaml_parser_t *parser = ptr;
+ const psych_parser_t *parser = ptr;
  /* TODO: calculate parser's size */
  return 0;
 }
@@ -81,10 +90,10 @@ static const rb_data_type_t psych_parser_type = {
 
 static VALUE allocate(VALUE klass)
 {
-yaml_parser_t * parser;
- VALUE obj = TypedData_Make_Struct(klass, yaml_parser_t, &psych_parser_type, parser);
+psych_parser_t * parser;
+ VALUE obj = TypedData_Make_Struct(klass, psych_parser_t, &psych_parser_type, parser);
 
- yaml_parser_initialize(parser);
+ yaml_parser_initialize(&parser->yaml_parser);
 
  return obj;
 }
@@ -257,18 +266,29 @@ static VALUE protected_event_location(VALUE pointer)
  return rb_funcall3(args[0], id_event_location, 4, args + 1);
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+struct parse_args {
+ psych_parser_t * psych_parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
 {
- yaml_parser_t * parser;
+ struct parse_args * pargs = (struct parse_args *)ptr;
+ yaml_parser_t * parser = &pargs->psych_parser->yaml_parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  yaml_event_t event;
  int done = 0;
  int state = 0;
  int parser_encoding = YAML_ANY_ENCODING;
  int encoding = rb_utf8_encindex();
  rb_encoding * internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
-
  yaml_parser_delete(parser);
  yaml_parser_initialize(parser);
 
@@ -312,6 +332,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = end_line;
  event_args[4] = end_column;
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ yaml_event_delete(&event);
+ rb_jump_tag(state);
+ }
 
  switch(event.type) {
  case YAML_STREAM_START_EVENT:
@@ -496,7 +520,11 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case YAML_NO_EVENT:
+ /* Once libyaml has produced the stream end, every later call
+ * succeeds with a zeroed event and YAML_STREAM_END_EVENT can no
+ * longer be reached. Stop rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case YAML_STREAM_END_EVENT:
  rb_protect(protected_end_stream, handler, &state);
@@ -510,6 +538,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_parser_t * parser = (psych_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_parser_t * parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.psych_parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>
@@ -521,13 +580,13 @@ static VALUE mark(VALUE self)
 {
  VALUE mark_klass;
  VALUE args[3];
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
  mark_klass = rb_const_get_at(cPsychParser, rb_intern("Mark"));
- args[0] = SIZET2NUM(parser->mark.index);
- args[1] = SIZET2NUM(parser->mark.line);
- args[2] = SIZET2NUM(parser->mark.column);
+ args[0] = SIZET2NUM(parser->yaml_parser.mark.index);
+ args[1] = SIZET2NUM(parser->yaml_parser.mark.line);
+ args[2] = SIZET2NUM(parser->yaml_parser.mark.column);
 
  return rb_class_new_instance(3, args, mark_klass);
 }

ext/psych/psych_parser_fy.c

@@ -41,6 +41,11 @@ typedef struct {
  size_t mark_line;
  size_t mark_column;
  size_t mark_index;
+ /* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() destroys and
+ * recreates fyp, which would pull the parser out from under the loop still
+ * driving it, so keep a flag to reject a reentrant call. */
+ int parsing;
 } psych_fy_parser_t;
 
 static const struct fy_parse_cfg psych_parse_cfg = {
@@ -256,17 +261,28 @@ static VALUE token_to_str(struct fy_token *tok, int encoding, rb_encoding *inter
  return str;
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
-{
+struct parse_args {
  psych_fy_parser_t *parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
+{
+ struct parse_args *pargs = (struct parse_args *)ptr;
+ psych_fy_parser_t *parser = pargs->parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  struct fy_event *event;
  int done = 0;
  int state = 0;
  int encoding = rb_utf8_encindex();
  rb_encoding *internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
-
  /* Use a pristine parser for each parse, like fy-tool does. Reusing a
  * parser across documents via fy_parser_reset() left the default tag
  * handles unset for bare (no "---") tag-led documents. */
@@ -348,6 +364,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = SIZET2NUM(em ? (size_t)em->line : 0);
  event_args[4] = SIZET2NUM(em ? (size_t)em->column : 0);
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ fy_parser_event_free(parser->fyp, event);
+ rb_jump_tag(state);
+ }
 
  switch (event->type) {
  case FYET_STREAM_START:
@@ -473,7 +493,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case FYET_NONE:
+ /* An event with no type cannot advance the stream, so stop
+ * rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case FYET_STREAM_END:
  rb_protect(protected_end_stream, handler, &state);
@@ -489,6 +512,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_fy_parser_t *parser = (psych_fy_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_fy_parser_t *parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>

test/psych/test_parser.rb

@@ -26,6 +26,39 @@ def #{m} *args
  end
  end
 
+ # Calls Parser#parse again, once, from inside a callback of the parse it is
+ # already handling.
+ class ReentrantHandler < Handler
+ attr_accessor :parser, :inner_yaml
+ attr_reader :inner_error, :scalars, :empty_calls
+
+ def initialize
+ @parser = nil
+ @inner_yaml = nil
+ @inner_error = nil
+ @scalars = []
+ @empty_calls = 0
+ end
+
+ def empty
+ @empty_calls += 1
+ raise "handler#empty keeps being called, the parse loop is not terminating" if @empty_calls > 1000
+ end
+
+ def scalar value, anchor, tag, plain, quoted, style
+ @scalars << value
+
+ inner, @inner_yaml = @inner_yaml, nil
+ return unless inner
+
+ begin
+ @parser.parse inner
+ rescue => e
+ @inner_error = e
+ end
+ end
+ end
+
  def setup
  super
  @handler = EventCatcher.new
@@ -70,6 +103,56 @@ def test_exception_memory_leak
  end
  end
 
+ def test_event_location_exception_is_propagated
+ klass = Class.new(Psych::Handler) do
+ def event_location start_line, start_column, end_line, end_column
+ raise "from event_location"
+ end
+ end
+
+ parser = Psych::Parser.new klass.new
+ 2.times do
+ ex = assert_raise(RuntimeError) { parser.parse "--- hello\n" }
+ assert_equal "from event_location", ex.message
+ end
+ end
+
+ def test_parse_is_not_reentrant
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- inner\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+
+ # The in-use flag is cleared when the parse finishes, so the same parser
+ # can be used again afterwards.
+ handler.scalars.clear
+ parser.parse "--- second\n"
+ assert_equal ['second'], handler.scalars
+ end
+
+ def test_parse_is_not_reentrant_with_invalid_inner_document
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- \x00bad\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+ end
+
  def test_multiparse
  3.times do
  @parser.parse '--- foo'

Gemfile

@@ -6,6 +6,6 @@ group :development do
  gem 'rake-compiler', ">= 0.4.1"
  gem 'ruby-maven', :platforms => :jruby
  gem 'test-unit'
- gem 'test-unit-ruby-core', ">= 1.0.7"
+ gem 'test-unit-ruby-core', ">= 1.0.16"
  gem 'power_assert', '~> 2.0' if RUBY_VERSION < '3.0' # https://github.com/ruby/power_assert/pull/61
 end

ext/psych/psych_parser.c

@@ -52,19 +52,28 @@ static int io_reader(void * data, unsigned char *buf, size_t size, size_t *read)
  return 1;
 }
 
+/* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() reinitialises the
+ * parser it is handed, which would pull the input out from under the loop
+ * still driving it, so keep a flag to reject a reentrant call. */
+typedef struct {
+ yaml_parser_t yaml_parser;
+ int parsing;
+} psych_parser_t;
+
 static void dealloc(void * ptr)
 {
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- parser = (yaml_parser_t *)ptr;
- yaml_parser_delete(parser);
+ parser = (psych_parser_t *)ptr;
+ yaml_parser_delete(&parser->yaml_parser);
  xfree(parser);
 }
 
 #if 0
 static size_t memsize(const void *ptr)
 {
- const yaml_parser_t *parser = ptr;
+ const psych_parser_t *parser = ptr;
  /* TODO: calculate parser's size */
  return 0;
 }
@@ -81,10 +90,10 @@ static const rb_data_type_t psych_parser_type = {
 
 static VALUE allocate(VALUE klass)
 {
-yaml_parser_t * parser;
- VALUE obj = TypedData_Make_Struct(klass, yaml_parser_t, &psych_parser_type, parser);
+psych_parser_t * parser;
+ VALUE obj = TypedData_Make_Struct(klass, psych_parser_t, &psych_parser_type, parser);
 
- yaml_parser_initialize(parser);
+ yaml_parser_initialize(&parser->yaml_parser);
 
  return obj;
 }
@@ -257,18 +266,29 @@ static VALUE protected_event_location(VALUE pointer)
  return rb_funcall3(args[0], id_event_location, 4, args + 1);
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+struct parse_args {
+ psych_parser_t * psych_parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
 {
- yaml_parser_t * parser;
+ struct parse_args * pargs = (struct parse_args *)ptr;
+ yaml_parser_t * parser = &pargs->psych_parser->yaml_parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  yaml_event_t event;
  int done = 0;
  int state = 0;
  int parser_encoding = YAML_ANY_ENCODING;
  int encoding = rb_utf8_encindex();
  rb_encoding * internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
-
  yaml_parser_delete(parser);
  yaml_parser_initialize(parser);
 
@@ -312,6 +332,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = end_line;
  event_args[4] = end_column;
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ yaml_event_delete(&event);
+ rb_jump_tag(state);
+ }
 
  switch(event.type) {
  case YAML_STREAM_START_EVENT:
@@ -496,7 +520,11 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case YAML_NO_EVENT:
+ /* Once libyaml has produced the stream end, every later call
+ * succeeds with a zeroed event and YAML_STREAM_END_EVENT can no
+ * longer be reached. Stop rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case YAML_STREAM_END_EVENT:
  rb_protect(protected_end_stream, handler, &state);
@@ -510,6 +538,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_parser_t * parser = (psych_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_parser_t * parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.psych_parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>
@@ -521,13 +580,13 @@ static VALUE mark(VALUE self)
 {
  VALUE mark_klass;
  VALUE args[3];
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
  mark_klass = rb_const_get_at(cPsychParser, rb_intern("Mark"));
- args[0] = SIZET2NUM(parser->mark.index);
- args[1] = SIZET2NUM(parser->mark.line);
- args[2] = SIZET2NUM(parser->mark.column);
+ args[0] = SIZET2NUM(parser->yaml_parser.mark.index);
+ args[1] = SIZET2NUM(parser->yaml_parser.mark.line);
+ args[2] = SIZET2NUM(parser->yaml_parser.mark.column);
 
  return rb_class_new_instance(3, args, mark_klass);
 }

ext/psych/psych_parser_fy.c

@@ -41,6 +41,11 @@ typedef struct {
  size_t mark_line;
  size_t mark_column;
  size_t mark_index;
+ /* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() destroys and
+ * recreates fyp, which would pull the parser out from under the loop still
+ * driving it, so keep a flag to reject a reentrant call. */
+ int parsing;
 } psych_fy_parser_t;
 
 static const struct fy_parse_cfg psych_parse_cfg = {
@@ -256,17 +261,28 @@ static VALUE token_to_str(struct fy_token *tok, int encoding, rb_encoding *inter
  return str;
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
-{
+struct parse_args {
  psych_fy_parser_t *parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
+{
+ struct parse_args *pargs = (struct parse_args *)ptr;
+ psych_fy_parser_t *parser = pargs->parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  struct fy_event *event;
  int done = 0;
  int state = 0;
  int encoding = rb_utf8_encindex();
  rb_encoding *internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
-
  /* Use a pristine parser for each parse, like fy-tool does. Reusing a
  * parser across documents via fy_parser_reset() left the default tag
  * handles unset for bare (no "---") tag-led documents. */
@@ -348,6 +364,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = SIZET2NUM(em ? (size_t)em->line : 0);
  event_args[4] = SIZET2NUM(em ? (size_t)em->column : 0);
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ fy_parser_event_free(parser->fyp, event);
+ rb_jump_tag(state);
+ }
 
  switch (event->type) {
  case FYET_STREAM_START:
@@ -473,7 +493,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case FYET_NONE:
+ /* An event with no type cannot advance the stream, so stop
+ * rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case FYET_STREAM_END:
  rb_protect(protected_end_stream, handler, &state);
@@ -489,6 +512,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_fy_parser_t *parser = (psych_fy_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_fy_parser_t *parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>

test/psych/test_parser.rb

@@ -26,6 +26,39 @@ def #{m} *args
  end
  end
 
+ # Calls Parser#parse again, once, from inside a callback of the parse it is
+ # already handling.
+ class ReentrantHandler < Handler
+ attr_accessor :parser, :inner_yaml
+ attr_reader :inner_error, :scalars, :empty_calls
+
+ def initialize
+ @parser = nil
+ @inner_yaml = nil
+ @inner_error = nil
+ @scalars = []
+ @empty_calls = 0
+ end
+
+ def empty
+ @empty_calls += 1
+ raise "handler#empty keeps being called, the parse loop is not terminating" if @empty_calls > 1000
+ end
+
+ def scalar value, anchor, tag, plain, quoted, style
+ @scalars << value
+
+ inner, @inner_yaml = @inner_yaml, nil
+ return unless inner
+
+ begin
+ @parser.parse inner
+ rescue => e
+ @inner_error = e
+ end
+ end
+ end
+
  def setup
  super
  @handler = EventCatcher.new
@@ -70,6 +103,56 @@ def test_exception_memory_leak
  end
  end
 
+ def test_event_location_exception_is_propagated
+ klass = Class.new(Psych::Handler) do
+ def event_location start_line, start_column, end_line, end_column
+ raise "from event_location"
+ end
+ end
+
+ parser = Psych::Parser.new klass.new
+ 2.times do
+ ex = assert_raise(RuntimeError) { parser.parse "--- hello\n" }
+ assert_equal "from event_location", ex.message
+ end
+ end
+
+ def test_parse_is_not_reentrant
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- inner\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+
+ # The in-use flag is cleared when the parse finishes, so the same parser
+ # can be used again afterwards.
+ handler.scalars.clear
+ parser.parse "--- second\n"
+ assert_equal ['second'], handler.scalars
+ end
+
+ def test_parse_is_not_reentrant_with_invalid_inner_document
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- \x00bad\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+ end
+
  def test_multiparse
  3.times do
  @parser.parse '--- foo'

Gemfile

@@ -6,6 +6,6 @@ group :development do
  gem 'rake-compiler', ">= 0.4.1"
  gem 'ruby-maven', :platforms => :jruby
  gem 'test-unit'
- gem 'test-unit-ruby-core', ">= 1.0.7"
+ gem 'test-unit-ruby-core', ">= 1.0.16"
  gem 'power_assert', '~> 2.0' if RUBY_VERSION < '3.0' # https://github.com/ruby/power_assert/pull/61
 end

ext/psych/psych_parser.c

@@ -52,19 +52,28 @@ static int io_reader(void * data, unsigned char *buf, size_t size, size_t *read)
  return 1;
 }
 
+/* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() reinitialises the
+ * parser it is handed, which would pull the input out from under the loop
+ * still driving it, so keep a flag to reject a reentrant call. */
+typedef struct {
+ yaml_parser_t yaml_parser;
+ int parsing;
+} psych_parser_t;
+
 static void dealloc(void * ptr)
 {
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- parser = (yaml_parser_t *)ptr;
- yaml_parser_delete(parser);
+ parser = (psych_parser_t *)ptr;
+ yaml_parser_delete(&parser->yaml_parser);
  xfree(parser);
 }
 
 #if 0
 static size_t memsize(const void *ptr)
 {
- const yaml_parser_t *parser = ptr;
+ const psych_parser_t *parser = ptr;
  /* TODO: calculate parser's size */
  return 0;
 }
@@ -81,10 +90,10 @@ static const rb_data_type_t psych_parser_type = {
 
 static VALUE allocate(VALUE klass)
 {
-yaml_parser_t * parser;
- VALUE obj = TypedData_Make_Struct(klass, yaml_parser_t, &psych_parser_type, parser);
+psych_parser_t * parser;
+ VALUE obj = TypedData_Make_Struct(klass, psych_parser_t, &psych_parser_type, parser);
 
- yaml_parser_initialize(parser);
+ yaml_parser_initialize(&parser->yaml_parser);
 
  return obj;
 }
@@ -257,18 +266,29 @@ static VALUE protected_event_location(VALUE pointer)
  return rb_funcall3(args[0], id_event_location, 4, args + 1);
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+struct parse_args {
+ psych_parser_t * psych_parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
 {
- yaml_parser_t * parser;
+ struct parse_args * pargs = (struct parse_args *)ptr;
+ yaml_parser_t * parser = &pargs->psych_parser->yaml_parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  yaml_event_t event;
  int done = 0;
  int state = 0;
  int parser_encoding = YAML_ANY_ENCODING;
  int encoding = rb_utf8_encindex();
  rb_encoding * internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
-
  yaml_parser_delete(parser);
  yaml_parser_initialize(parser);
 
@@ -312,6 +332,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = end_line;
  event_args[4] = end_column;
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ yaml_event_delete(&event);
+ rb_jump_tag(state);
+ }
 
  switch(event.type) {
  case YAML_STREAM_START_EVENT:
@@ -496,7 +520,11 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case YAML_NO_EVENT:
+ /* Once libyaml has produced the stream end, every later call
+ * succeeds with a zeroed event and YAML_STREAM_END_EVENT can no
+ * longer be reached. Stop rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case YAML_STREAM_END_EVENT:
  rb_protect(protected_end_stream, handler, &state);
@@ -510,6 +538,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_parser_t * parser = (psych_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_parser_t * parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.psych_parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>
@@ -521,13 +580,13 @@ static VALUE mark(VALUE self)
 {
  VALUE mark_klass;
  VALUE args[3];
-yaml_parser_t * parser;
+psych_parser_t * parser;
 
- TypedData_Get_Struct(self, yaml_parser_t, &psych_parser_type, parser);
+ TypedData_Get_Struct(self, psych_parser_t, &psych_parser_type, parser);
  mark_klass = rb_const_get_at(cPsychParser, rb_intern("Mark"));
- args[0] = SIZET2NUM(parser->mark.index);
- args[1] = SIZET2NUM(parser->mark.line);
- args[2] = SIZET2NUM(parser->mark.column);
+ args[0] = SIZET2NUM(parser->yaml_parser.mark.index);
+ args[1] = SIZET2NUM(parser->yaml_parser.mark.line);
+ args[2] = SIZET2NUM(parser->yaml_parser.mark.column);
 
  return rb_class_new_instance(3, args, mark_klass);
 }

ext/psych/psych_parser_fy.c

@@ -41,6 +41,11 @@ typedef struct {
  size_t mark_line;
  size_t mark_column;
  size_t mark_index;
+ /* The parser calls back into Ruby for every event, so a handler can call
+ * Psych::Parser#parse again on the same object. parse() destroys and
+ * recreates fyp, which would pull the parser out from under the loop still
+ * driving it, so keep a flag to reject a reentrant call. */
+ int parsing;
 } psych_fy_parser_t;
 
 static const struct fy_parse_cfg psych_parse_cfg = {
@@ -256,17 +261,28 @@ static VALUE token_to_str(struct fy_token *tok, int encoding, rb_encoding *inter
  return str;
 }
 
-static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
-{
+struct parse_args {
  psych_fy_parser_t *parser;
+ VALUE self;
+ VALUE handler;
+ VALUE yaml;
+ VALUE path;
+};
+
+static VALUE parse_body(VALUE ptr)
+{
+ struct parse_args *pargs = (struct parse_args *)ptr;
+ psych_fy_parser_t *parser = pargs->parser;
+ VALUE self = pargs->self;
+ VALUE handler = pargs->handler;
+ VALUE yaml = pargs->yaml;
+ VALUE path = pargs->path;
  struct fy_event *event;
  int done = 0;
  int state = 0;
  int encoding = rb_utf8_encindex();
  rb_encoding *internal_enc = rb_default_internal_encoding();
 
- TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
-
  /* Use a pristine parser for each parse, like fy-tool does. Reusing a
  * parser across documents via fy_parser_reset() left the default tag
  * handles unset for bare (no "---") tag-led documents. */
@@ -348,6 +364,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  event_args[3] = SIZET2NUM(em ? (size_t)em->line : 0);
  event_args[4] = SIZET2NUM(em ? (size_t)em->column : 0);
  rb_protect(protected_event_location, (VALUE)event_args, &state);
+ if (state) {
+ fy_parser_event_free(parser->fyp, event);
+ rb_jump_tag(state);
+ }
 
  switch (event->type) {
  case FYET_STREAM_START:
@@ -473,7 +493,10 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  rb_protect(protected_end_mapping, handler, &state);
  break;
  case FYET_NONE:
+ /* An event with no type cannot advance the stream, so stop
+ * rather than loop forever. */
  rb_protect(protected_empty, handler, &state);
+ done = 1;
  break;
  case FYET_STREAM_END:
  rb_protect(protected_end_stream, handler, &state);
@@ -489,6 +512,37 @@ static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
  return self;
 }
 
+static VALUE parse_ensure(VALUE ptr)
+{
+ psych_fy_parser_t *parser = (psych_fy_parser_t *)ptr;
+
+ parser->parsing = 0;
+
+ return Qnil;
+}
+
+static VALUE parse(VALUE self, VALUE handler, VALUE yaml, VALUE path)
+{
+ psych_fy_parser_t *parser;
+ struct parse_args pargs;
+
+ TypedData_Get_Struct(self, psych_fy_parser_t, &psych_parser_type, parser);
+
+ if (parser->parsing) {
+ rb_raise(rb_const_get(mPsych, rb_intern("Exception")),
+ "parser is already parsing, it cannot be reused from a handler callback");
+ }
+ parser->parsing = 1;
+
+ pargs.parser = parser;
+ pargs.self = self;
+ pargs.handler = handler;
+ pargs.yaml = yaml;
+ pargs.path = path;
+
+ return rb_ensure(parse_body, (VALUE)&pargs, parse_ensure, (VALUE)parser);
+}
+
 /*
  * call-seq:
  * parser.mark # => #<Psych::Parser::Mark>

test/psych/test_parser.rb

@@ -26,6 +26,39 @@ def #{m} *args
  end
  end
 
+ # Calls Parser#parse again, once, from inside a callback of the parse it is
+ # already handling.
+ class ReentrantHandler < Handler
+ attr_accessor :parser, :inner_yaml
+ attr_reader :inner_error, :scalars, :empty_calls
+
+ def initialize
+ @parser = nil
+ @inner_yaml = nil
+ @inner_error = nil
+ @scalars = []
+ @empty_calls = 0
+ end
+
+ def empty
+ @empty_calls += 1
+ raise "handler#empty keeps being called, the parse loop is not terminating" if @empty_calls > 1000
+ end
+
+ def scalar value, anchor, tag, plain, quoted, style
+ @scalars << value
+
+ inner, @inner_yaml = @inner_yaml, nil
+ return unless inner
+
+ begin
+ @parser.parse inner
+ rescue => e
+ @inner_error = e
+ end
+ end
+ end
+
  def setup
  super
  @handler = EventCatcher.new
@@ -70,6 +103,56 @@ def test_exception_memory_leak
  end
  end
 
+ def test_event_location_exception_is_propagated
+ klass = Class.new(Psych::Handler) do
+ def event_location start_line, start_column, end_line, end_column
+ raise "from event_location"
+ end
+ end
+
+ parser = Psych::Parser.new klass.new
+ 2.times do
+ ex = assert_raise(RuntimeError) { parser.parse "--- hello\n" }
+ assert_equal "from event_location", ex.message
+ end
+ end
+
+ def test_parse_is_not_reentrant
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- inner\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+
+ # The in-use flag is cleared when the parse finishes, so the same parser
+ # can be used again afterwards.
+ handler.scalars.clear
+ parser.parse "--- second\n"
+ assert_equal ['second'], handler.scalars
+ end
+
+ def test_parse_is_not_reentrant_with_invalid_inner_document
+ pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+
+ handler = ReentrantHandler.new
+ handler.inner_yaml = "--- \x00bad\n"
+ parser = Psych::Parser.new handler
+ handler.parser = parser
+
+ parser.parse "--- outer\n"
+
+ assert_kind_of Psych::Exception, handler.inner_error
+ assert_equal ['outer'], handler.scalars
+ assert_equal 0, handler.empty_calls
+ end
+
  def test_multiparse
  3.times do
  @parser.parse '--- foo'