Skip to content

Commit b237211

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 b237211

2 files changed

Lines changed: 32 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: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,14 +384,24 @@ def test_parsed_bytes
384384
assert_equal 0, @parser.parsed_bytes
385385
end
386386

387+
def test_parse_error_message
388+
error = assert_parse_error("\n\n[plop\nfoo", "unexpected character: 'plop'")
389+
assert_equal 0, error.line
390+
assert_equal 0, error.column
391+
end
392+
387393
private
388394

389-
def assert_parse_error(json)
395+
def assert_parse_error(json, expected_error_message = nil)
390396
parser = new_parser
391397
parser << json
392-
assert_raise(JSON::ParserError, "expected a parse error for #{json.inspect}") do
398+
error = assert_raise(JSON::ParserError, "expected a parse error for #{json.inspect}") do
393399
parser.parse
394400
end
401+
if expected_error_message
402+
assert_equal expected_error_message, error.message
403+
end
404+
error
395405
end
396406

397407
def assert_incomplete(json)

0 commit comments

Comments
 (0)