Skip to content

Commit

Permalink
wasm: Fix JSON parser to copy memory for strings and numbers
Browse files Browse the repository at this point in the history
Previously the parser was constructing strings and numbers with
references to the input buffer memory. If that memory was freed after
the opa_json_parse() call, it would corrupt the strings and numbers
returned by the parse.

This commit just updates the parser to create a copy of the
string/number values. If this becomes a performance issue in the
future, we can introduce an optional API for callers that promise NOT
to free the input buffer.

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
  • Loading branch information
tsandall committed Oct 9, 2020
1 parent 80bb853 commit 4851c4b
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 4 deletions.
2 changes: 1 addition & 1 deletion internal/compiler/wasm/opa/opa.go

Large diffs are not rendered by default.

Binary file modified internal/compiler/wasm/opa/opa.wasm
Binary file not shown.
18 changes: 16 additions & 2 deletions wasm/src/json.c
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,14 @@ opa_value *opa_json_parse_string(int token, const char *buf, int len)
{
if (token == OPA_JSON_TOKEN_STRING)
{
return opa_string(buf, len);
char *cpy = (char *)opa_malloc(len);

for (int i = 0; i < len; i++)
{
cpy[i] = buf[i];
}

return opa_string_allocated(cpy, len);
}

int max_len = opa_json_max_string_len(buf, len);
Expand Down Expand Up @@ -435,7 +442,14 @@ opa_value *opa_json_parse_string(int token, const char *buf, int len)

opa_value *opa_json_parse_number(const char *buf, int len)
{
return opa_number_ref(buf, len);
char *cpy = (char *)opa_malloc(len);

for (int i = 0; i < len; i++)
{
cpy[i] = buf[i];
}

return opa_number_ref_allocated(cpy, len);
}

opa_value *opa_json_parse_array(opa_json_lex *ctx)
Expand Down
27 changes: 26 additions & 1 deletion wasm/src/value.c
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ void opa_value_free(opa_value *node)
opa_free(opa_cast_boolean(node));
return;
case OPA_NUMBER:
opa_free(opa_cast_number(node));
opa_number_free(opa_cast_number(node));
return;
case OPA_STRING:
opa_string_free(opa_cast_string(node));
Expand Down Expand Up @@ -881,9 +881,34 @@ opa_value *opa_number_ref(const char *s, size_t len)
ret->repr = OPA_NUMBER_REPR_REF;
ret->v.ref.s = s;
ret->v.ref.len = len;
ret->v.ref.free = 0;
return &ret->hdr;
}

opa_value *opa_number_ref_allocated(const char *s, size_t len)
{
opa_number_t *ret = (opa_number_t *)opa_malloc(sizeof(opa_number_t));
ret->hdr.type = OPA_NUMBER;
ret->repr = OPA_NUMBER_REPR_REF;
ret->v.ref.s = s;
ret->v.ref.len = len;
ret->v.ref.free = 1;
return &ret->hdr;
}

void opa_number_free(opa_number_t *n)
{
if (n->repr == OPA_NUMBER_REPR_REF)
{
if (n->v.ref.free)
{
opa_free((void *)n->v.ref.s);
}
}

opa_free(n);
}

int opa_number_try_int(opa_number_t *n, long long *i)
{
switch (n->repr)
Expand Down
3 changes: 3 additions & 0 deletions wasm/src/value.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ typedef struct
{
const char *s;
size_t len;
unsigned char free; // if set 's' is not a reference and should be freed
} opa_number_ref_t;

typedef struct
Expand Down Expand Up @@ -127,6 +128,7 @@ opa_value *opa_number_size(size_t v);
opa_value *opa_number_int(long long v);
opa_value *opa_number_float(double v);
opa_value *opa_number_ref(const char *s, size_t len);
opa_value *opa_number_ref_allocated(const char *s, size_t len);
opa_value *opa_string(const char *v, size_t len);
opa_value *opa_string_terminated(const char *v);
opa_value *opa_string_allocated(const char *v, size_t len);
Expand All @@ -142,6 +144,7 @@ void opa_value_number_set_int(opa_value *v, long long i);

int opa_number_try_int(opa_number_t *n, long long *i);
double opa_number_as_float(opa_number_t *n);
void opa_number_free(opa_number_t *n);

void opa_string_free(opa_string_t *s);

Expand Down
21 changes: 21 additions & 0 deletions wasm/tests/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,27 @@ void test_opa_json_parse_composites()
test("object nested", parse_crunch("{\"a\": {\"c\": 1, \"d\": 2}, \"b\": {\"e\": 3, \"f\": 4}}", &fixture_object2()->hdr));
}

void test_opa_json_parse_memory_ownership()
{
char s[] = "[1,\"a\"]";

opa_value *result = opa_json_parse(s, sizeof(s));

opa_value *exp = opa_array();
opa_array_t *arr = opa_cast_array(exp);
opa_array_append(arr, opa_number_int(1));
opa_array_append(arr, opa_string("a", 1));

test("expected value", opa_value_compare(result, exp) == 0);

for (int i = 0; i < sizeof(s); i++)
{
s[i] = 0;
}

test("expected value after overwriting buffer", opa_value_compare(result, exp) == 0);
}

void test_opa_object_insert()
{

Expand Down

0 comments on commit 4851c4b

Please sign in to comment.