Skip to content

Commit c6ff0e2

Browse files
committed
ResumableParser: Don't compute lines and columns on parse error
Fix: #1022 They can't always be accurate because we don't always keep the full document in the buffer. As such it's better never to compute them than to sometimes provide wrong coordinates. In theory we could keep the number of lines since the start of the parse, but that's more book keeping for little utility. Anyway, these are useful to find a syntax error in a file, not so much in a stream of documents.
1 parent 21c2bbe commit c6ff0e2

2 files changed

Lines changed: 29 additions & 14 deletions

File tree

ext/json/ext/parser/parser.c

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,7 @@ static void emit_parse_warning(const char *message, JSON_ParserState *state)
627627

628628
#define PARSE_ERROR_FRAGMENT_LEN 32
629629

630-
static VALUE build_parse_error_message(const char *format, JSON_ParserState *state, long line, long column)
630+
static VALUE build_parse_error_message(const char *format, JSON_ParserState *state)
631631
{
632632
unsigned char buffer[PARSE_ERROR_FRAGMENT_LEN + 3];
633633

@@ -661,9 +661,7 @@ static VALUE build_parse_error_message(const char *format, JSON_ParserState *sta
661661
}
662662
}
663663

664-
VALUE message = rb_enc_sprintf(enc_utf8, format, ptr);
665-
rb_str_catf(message, " at line %ld column %ld", line, column);
666-
return message;
664+
return rb_enc_sprintf(enc_utf8, format, ptr);
667665
}
668666

669667
static VALUE parse_error_new(JSON_ParserState *state, VALUE message, long line, long column, bool eos)
@@ -679,10 +677,15 @@ static VALUE parse_error_new(JSON_ParserState *state, VALUE message, long line,
679677

680678
NORETURN(static) void raise_parse_error(const char *format, JSON_ParserState *state, bool eos)
681679
{
682-
long line, column;
683-
cursor_position(state, &line, &column);
684-
VALUE message = build_parse_error_message(format, state, line, column);
685-
rb_exc_raise(parse_error_new(state, message, line, column, eos));
680+
VALUE message = build_parse_error_message(format, state);
681+
if (state->parser) { // line and columns can't be accurate in resumable
682+
rb_exc_raise(parse_error_new(state, message, 0, 0, eos));
683+
} else {
684+
long line, column;
685+
cursor_position(state, &line, &column);
686+
rb_str_catf(message, " at line %ld column %ld", line, column);
687+
rb_exc_raise(parse_error_new(state, message, line, column, eos));
688+
}
686689
}
687690

688691
NORETURN(static) void raise_eos_error(const char *format, JSON_ParserState *state)
@@ -1172,10 +1175,15 @@ NORETURN(static) void raise_duplicate_key_error(JSON_ParserState *state, VALUE d
11721175
rb_inspect(duplicate_key)
11731176
);
11741177

1175-
long line, column;
1176-
cursor_position(state, &line, &column);
1177-
rb_str_concat(message, build_parse_error_message("", state, line, column)) ;
1178-
rb_exc_raise(parse_error_new(state, message, line, column, false));
1178+
rb_str_concat(message, build_parse_error_message("", state));
1179+
if (state->parser) { // line and columns can't be accurate in resumable
1180+
rb_exc_raise(parse_error_new(state, message, 0, 0, false));
1181+
} else {
1182+
long line, column;
1183+
cursor_position(state, &line, &column);
1184+
rb_str_catf(message, " at line %ld column %ld", line, column);
1185+
rb_exc_raise(parse_error_new(state, message, line, column, false));
1186+
}
11791187
}
11801188

11811189
NOINLINE(static) void json_on_duplicate_key(JSON_ParserState *state, JSON_ParserConfig *config, size_t count, const VALUE *pairs)

test/json/resumable_parser_test.rb

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,14 +384,21 @@ def test_parsed_bytes
384384
assert_equal 0, @parser.parsed_bytes
385385
end
386386

387+
def test_parse_error_message
388+
assert_parse_error("\n\n[plop\nfoo", "unexpected character: 'plop'")
389+
end
390+
387391
private
388392

389-
def assert_parse_error(json)
393+
def assert_parse_error(json, expected_error_message = nil)
390394
parser = new_parser
391395
parser << json
392-
assert_raise(JSON::ParserError, "expected a parse error for #{json.inspect}") do
396+
error = assert_raise(JSON::ParserError, "expected a parse error for #{json.inspect}") do
393397
parser.parse
394398
end
399+
if expected_error_message
400+
assert_equal expected_error_message, error.message
401+
end
395402
end
396403

397404
def assert_incomplete(json)

0 commit comments

Comments
 (0)