Skip to content

Commit

Permalink
Improve handling of non-integer numeric indices (fix #2815)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nicowilliams
nicowilliams committed Aug 17, 2023
1 parent 0733fd3 commit abdda81
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 32 deletions.
98 changes: 66 additions & 32 deletions src/jv_aux.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <assert.h>
#include <limits.h>
#include <string.h>
#include <stdlib.h>
#include <assert.h>
#include "jv_alloc.h"
#include "jv_private.h"

Expand All @@ -13,7 +14,9 @@ static double jv_number_get_value_and_consume(jv number) {
return value;
}

static int parse_slice(jv j, jv slice, int* pstart, int* pend) {
enum slice_for { FOR_READ, FOR_WRITE };

static jv parse_slice(jv j, jv slice, int* pstart, int* pend, enum slice_for slice_for) {
// Array slices
jv start_jv = jv_object_get(jv_copy(slice), jv_string("start"));
jv end_jv = jv_object_get(slice, jv_string("end"));
Expand All @@ -27,8 +30,14 @@ static int parse_slice(jv j, jv slice, int* pstart, int* pend) {
} else if (jv_get_kind(j) == JV_KIND_STRING) {
len = jv_string_length_codepoints(j);
} else {
/*
* XXX This should be dead code because callers shouldn't call this
* function if `j' is neither an array nor a string.
*/
jv_free(j);
return 0;
jv_free(start_jv);
jv_free(end_jv);
return jv_invalid_with_msg(jv_string("Only arrays and strings can be sliced"));
}
if (jv_get_kind(end_jv) == JV_KIND_NULL) {
jv_free(end_jv);
Expand All @@ -38,29 +47,39 @@ static int parse_slice(jv j, jv slice, int* pstart, int* pend) {
jv_get_kind(end_jv) != JV_KIND_NUMBER) {
jv_free(start_jv);
jv_free(end_jv);
return 0;
} else {
double dstart = jv_number_value(start_jv);
double dend = jv_number_value(end_jv);
return jv_invalid_with_msg(jv_string("Array/string slice indices must be integers"));
}
if (!jv_is_integer(start_jv) || !jv_is_integer(end_jv)) {
jv_free(start_jv);
jv_free(end_jv);
if (dstart < 0) dstart += len;
if (dend < 0) dend += len;
if (dstart < 0) dstart = 0;
if (dstart > len) dstart = len;

int start = (int)dstart;
int end = (dend > len) ? len : (int)dend;
// Ends are exclusive but e.g. 1 < 1.5 so :1.5 should be :2 not :1
if(end < dend) end += 1;

if (end > len) end = len;
if (end < start) end = start;
assert(0 <= start && start <= end && end <= len);
*pstart = start;
*pend = end;
return 1;
}
if (slice_for == FOR_READ)
return jv_null();
return jv_invalid_with_msg(jv_string("Array/string slice indices must be integers"));
}
double dstart = jv_number_value(start_jv);
double dend = jv_number_value(end_jv);
jv_free(start_jv);
jv_free(end_jv);
if (dstart < 0) dstart += len;
if (dend < 0) dend += len;
if (dstart < 0) dstart = 0;
if (dstart > len) dstart = len;
if (dstart < INT_MIN) dstart = INT_MIN;
if (dstart > INT_MAX) dstart = INT_MAX;
if (dend < INT_MIN) dend = INT_MIN;
if (dend > INT_MAX) dend = INT_MAX;

int start = (int)dstart;
int end = (dend > len) ? len : (int)dend;
// Ends are exclusive but e.g. 1 < 1.5 so :1.5 should be :2 not :1
if(end < dend) end += 1;

if (end > len) end = len;
if (end < start) end = start;
assert(0 <= start && start <= end && end <= len);
*pstart = start;
*pend = end;
return jv_true();
}

jv jv_get(jv t, jv k) {
Expand Down Expand Up @@ -89,19 +108,21 @@ jv jv_get(jv t, jv k) {
}
} else if (jv_get_kind(t) == JV_KIND_ARRAY && jv_get_kind(k) == JV_KIND_OBJECT) {
int start, end;
if (parse_slice(jv_copy(t), k, &start, &end)) {
jv e = parse_slice(jv_copy(t), k, &start, &end, FOR_READ);
if (jv_get_kind(e) == JV_KIND_TRUE) {
v = jv_array_slice(t, start, end);
} else {
jv_free(t);
v = jv_invalid_with_msg(jv_string_fmt("Start and end indices of an array slice must be numbers"));
v = e;
}
} else if (jv_get_kind(t) == JV_KIND_STRING && jv_get_kind(k) == JV_KIND_OBJECT) {
int start, end;
if (parse_slice(jv_copy(t), k, &start, &end)) {
jv e = parse_slice(jv_copy(t), k, &start, &end, FOR_READ);
if (jv_get_kind(e) == JV_KIND_TRUE) {
v = jv_string_slice(t, start, end);
} else {
v = jv_invalid_with_msg(jv_string_fmt("Start and end indices of an string slice must be numbers"));
jv_free(t);
v = e;
}
} else if (jv_get_kind(t) == JV_KIND_ARRAY && jv_get_kind(k) == JV_KIND_ARRAY) {
v = jv_array_indexes(t, k);
Expand Down Expand Up @@ -146,14 +167,20 @@ jv jv_set(jv t, jv k, jv v) {
t = jv_object_set(t, k, v);
} else if (jv_get_kind(k) == JV_KIND_NUMBER &&
(jv_get_kind(t) == JV_KIND_ARRAY || isnull)) {
if (!jv_is_integer(k)) {
jv_free(t);
jv_free(k);
return jv_invalid_with_msg(jv_string("Cannot index array with non-integer number"));
}
if (isnull) t = jv_array();
t = jv_array_set(t, (int)jv_number_value(k), v);
jv_free(k);
} else if (jv_get_kind(k) == JV_KIND_OBJECT &&
(jv_get_kind(t) == JV_KIND_ARRAY || isnull)) {
if (isnull) t = jv_array();
int start, end;
if (parse_slice(jv_copy(t), k, &start, &end)) {
jv e = parse_slice(jv_copy(t), k, &start, &end, FOR_WRITE);
if (jv_get_kind(e) == JV_KIND_TRUE) {
if (jv_get_kind(v) == JV_KIND_ARRAY) {
int array_len = jv_array_length(jv_copy(t));
assert(0 <= start && start <= end && end <= array_len);
Expand Down Expand Up @@ -185,8 +212,14 @@ jv jv_set(jv t, jv k, jv v) {
} else {
jv_free(t);
jv_free(v);
t = jv_invalid_with_msg(jv_string_fmt("Start and end indices of an array slice must be numbers"));
t = e;
}
} else if (jv_get_kind(k) == JV_KIND_OBJECT && jv_get_kind(t) == JV_KIND_STRING) {
jv_free(t);
jv_free(k);
jv_free(v);
/* Well, why not? We should implement this... */
t = jv_invalid_with_msg(jv_string_fmt("Cannot update string slices"));
} else {
jv err = jv_invalid_with_msg(jv_string_fmt("Cannot update field at %s index of %s",
jv_kind_name(jv_get_kind(k)),
Expand Down Expand Up @@ -255,13 +288,14 @@ static jv jv_dels(jv t, jv keys) {
}
} else if (jv_get_kind(key) == JV_KIND_OBJECT) {
int start, end;
if (parse_slice(jv_copy(t), key, &start, &end)) {
jv e = parse_slice(jv_copy(t), key, &start, &end, FOR_WRITE);
if (jv_get_kind(e) == JV_KIND_TRUE) {
starts = jv_array_append(starts, jv_number(start));
ends = jv_array_append(ends, jv_number(end));
} else {
jv_free(new_array);
jv_free(key);
new_array = jv_invalid_with_msg(jv_string_fmt("Start and end indices of an array slice must be numbers"));
new_array = e;
goto arr_out;
}
} else {
Expand Down
18 changes: 18 additions & 0 deletions tests/jq.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2024,3 +2024,21 @@ walk(1)
walk(select(IN({}, []) | not))
{"a":1,"b":[]}
{"a":1}

# #2815 -- probably subject to change
[range(.)]|.[1.5:3.5]
10
null

try ("foobar"|.[1.5:3.5] = "xyz") catch .
null
"Cannot update string slices"

try ([range(10)]|.[1.5:3.5] = ["xyz"]) catch .
null
"Array/string slice indices must be integers"

try ("foobar"|.[1.5]) catch .
null
"Cannot index string with number"

0 comments on commit abdda81

Please sign in to comment.