Skip to content

Commit

Permalink
Add JSON path to parse error messages
Browse files Browse the repository at this point in the history
printf format fix

Change exception type

sparse should handle errors correctly
  • Loading branch information
ohler55 committed Sep 26, 2018
1 parent 528cf60 commit f66ebf6
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 21 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
# CHANGELOG

## 3.6.11 - 2018-09-21

- Added the JSON path to parse error messages.

- BigDecimal parse errors now return Oj::ParseError instead of ArgumentError.

## 3.6.10 - 2018-09-13

- additional occurances of `SYM2ID(sym)` replaced.
- Additional occurances of `SYM2ID(sym)` replaced.

## 3.6.9 - 2018-09-12

Expand Down
55 changes: 46 additions & 9 deletions ext/oj/parse.c
Original file line number Diff line number Diff line change
Expand Up @@ -796,12 +796,42 @@ oj_num_as_value(NumInfo ni) {
void
oj_set_error_at(ParseInfo pi, VALUE err_clas, const char* file, int line, const char *format, ...) {
va_list ap;
char msg[128];
char msg[256];
char *p = msg;
char *end = p + sizeof(msg) - 2;
char *start;
Val vp;

va_start(ap, format);
vsnprintf(msg, sizeof(msg) - 1, format, ap);
p += vsnprintf(msg, sizeof(msg) - 1, format, ap);
va_end(ap);
pi->err.clas = err_clas;
if (p + 3 < end) {
*p++ = ' ';
*p++ = '(';
start = p;
for (vp = pi->stack.head; vp < pi->stack.tail; vp++) {
if (end <= p + 1 + vp->klen) {
break;
}
if (NULL != vp->key) {
if (start < p) {
*p++ = '.';
}
memcpy(p, vp->key, vp->klen);
p += vp->klen;
} else {
if (RUBY_T_ARRAY == rb_type(vp->val)) {
if (end <= p + 12) {
break;
}
p += snprintf(p, end - p, "[%ld]", RARRAY_LEN(vp->val));
}
}
}
*p++ = ')';
}
*p = '\0';
if (0 == pi->json) {
oj_err_set(&pi->err, err_clas, "%s at line %d, column %d [%s:%d]", msg, pi->rd.line, pi->rd.col, file, line);
} else {
Expand Down Expand Up @@ -936,23 +966,31 @@ oj_pi_parse(int argc, VALUE *argv, ParseInfo pi, char *json, size_t len, int yie
if (!err_has(&pi->err)) {
// If the stack is not empty then the JSON terminated early.
Val v;
VALUE err_class = oj_parse_error_class;

if (0 != line) {
VALUE ec = rb_obj_class(rb_errinfo());

if (0 != (v = stack_peek(&pi->stack))) {
if (rb_eArgError != ec) {
err_class = ec;
}
}
if (NULL != (v = stack_peek(&pi->stack))) {
switch (v->next) {
case NEXT_ARRAY_NEW:
case NEXT_ARRAY_ELEMENT:
case NEXT_ARRAY_COMMA:
oj_set_error_at(pi, oj_parse_error_class, __FILE__, __LINE__, "Array not terminated");
oj_set_error_at(pi, err_class, __FILE__, __LINE__, "Array not terminated");
break;
case NEXT_HASH_NEW:
case NEXT_HASH_KEY:
case NEXT_HASH_COLON:
case NEXT_HASH_VALUE:
case NEXT_HASH_COMMA:
oj_set_error_at(pi, oj_parse_error_class, __FILE__, __LINE__, "Hash/Object not terminated");
oj_set_error_at(pi, err_class, __FILE__, __LINE__, "Hash/Object not terminated");
break;
default:
oj_set_error_at(pi, oj_parse_error_class, __FILE__, __LINE__, "not terminated");
oj_set_error_at(pi, err_class, __FILE__, __LINE__, "not terminated");
}
}
}
Expand All @@ -969,9 +1007,6 @@ oj_pi_parse(int argc, VALUE *argv, ParseInfo pi, char *json, size_t len, int yie
if (pi->str_rx.head != oj_default_options.str_rx.head) {
oj_rxclass_cleanup(&pi->str_rx);
}
if (0 != line) {
rb_jump_tag(line);
}
if (err_has(&pi->err)) {
if (Qnil != pi->err_class) {
pi->err.clas = pi->err_class;
Expand All @@ -992,6 +1027,8 @@ oj_pi_parse(int argc, VALUE *argv, ParseInfo pi, char *json, size_t len, int yie
} else {
oj_err_raise(&pi->err);
}
} else if (0 != line) {
rb_jump_tag(line);
}
if (pi->options.quirks_mode == No) {
switch (rb_type(result)) {
Expand Down
20 changes: 13 additions & 7 deletions ext/oj/sparse.c
Original file line number Diff line number Diff line change
Expand Up @@ -839,23 +839,31 @@ oj_pi_sparse(int argc, VALUE *argv, ParseInfo pi, int fd) {
if (!err_has(&pi->err)) {
// If the stack is not empty then the JSON terminated early.
Val v;
VALUE err_class = oj_parse_error_class;

if (0 != line) {
VALUE ec = rb_obj_class(rb_errinfo());

if (rb_eArgError != ec) {
err_class = ec;
}
}
if (0 != (v = stack_peek(&pi->stack))) {
switch (v->next) {
case NEXT_ARRAY_NEW:
case NEXT_ARRAY_ELEMENT:
case NEXT_ARRAY_COMMA:
oj_set_error_at(pi, oj_parse_error_class, __FILE__, __LINE__, "Array not terminated");
oj_set_error_at(pi, err_class, __FILE__, __LINE__, "Array not terminated");
break;
case NEXT_HASH_NEW:
case NEXT_HASH_KEY:
case NEXT_HASH_COLON:
case NEXT_HASH_VALUE:
case NEXT_HASH_COMMA:
oj_set_error_at(pi, oj_parse_error_class, __FILE__, __LINE__, "Hash/Object not terminated");
oj_set_error_at(pi, err_class, __FILE__, __LINE__, "Hash/Object not terminated");
break;
default:
oj_set_error_at(pi, oj_parse_error_class, __FILE__, __LINE__, "not terminated");
oj_set_error_at(pi, err_class, __FILE__, __LINE__, "not terminated");
}
}
}
Expand All @@ -867,9 +875,6 @@ oj_pi_sparse(int argc, VALUE *argv, ParseInfo pi, int fd) {
if (0 != fd) {
close(fd);
}
if (0 != line) {
rb_jump_tag(line);
}
if (err_has(&pi->err)) {
if (Qnil != pi->err_class) {
pi->err.clas = pi->err_class;
Expand All @@ -886,8 +891,9 @@ oj_pi_sparse(int argc, VALUE *argv, ParseInfo pi, int fd) {
} else {
oj_err_raise(&pi->err);
}

oj_err_raise(&pi->err);
} else if (0 != line) {
rb_jump_tag(line);
}
return result;
}
2 changes: 1 addition & 1 deletion lib/oj/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

module Oj
# Current version of the module.
VERSION = '3.6.10'
VERSION = '3.6.11a1'
end
10 changes: 10 additions & 0 deletions test/test_scp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,16 @@ def test_pipe
end
end

def test_bad_bignum
if '2.4.0' < RUBY_VERSION
handler = AllHandler.new()
json = %|{"big":-e123456789}|
assert_raises Oj::ParseError do
Oj.sc_parse(handler, json)
end
end
end

def test_pipe_close
# Windows does not support fork
return if RbConfig::CONFIG['host_os'] =~ /(mingw|mswin)/
Expand Down
29 changes: 26 additions & 3 deletions test/test_various.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def eql?(o)
end
alias == eql?

end# Jam
end # Jam

class Jeez < Jam
def initialize(x, y)
Expand All @@ -43,7 +43,7 @@ def to_json()
def self.json_create(h)
self.new(h['x'], h['y'])
end
end# Jeez
end # Jeez

# contributed by sauliusg to fix as_json
class Orange < Jam
Expand Down Expand Up @@ -86,7 +86,7 @@ def to_hash()
def self.json_create(h)
self.new(h['x'], h['y'])
end
end# Jazz
end # Jazz

def setup
@default_options = Oj.default_options
Expand Down Expand Up @@ -661,6 +661,29 @@ def test_quirks_string_mode
assert_equal('string', Oj.load('"string"', :quirks_mode => true))
end

def test_error_path
msg = ''
assert_raises(Oj::ParseError) {
begin
Oj.load(%|{
"first": [
1, 2, { "third": 123x }
]
}|)
rescue Oj::ParseError => e
msg = e.message
raise e
end
}
assert_equal('first[2].third', msg.split('(')[1].split(')')[0])
end

def test_bad_bignum
if '2.4.0' < RUBY_VERSION
assert_raises(Oj::ParseError) { Oj.load(%|{ "big": -e123456789 }|) }
end
end

def test_quirks_array_mode
assert_equal([], Oj.load("[]", :quirks_mode => false))
assert_equal([], Oj.load("[]", :quirks_mode => true))
Expand Down

0 comments on commit f66ebf6

Please sign in to comment.