Skip to content

Commit

Permalink
Handle missing elements in a returned array.
Browse files Browse the repository at this point in the history
Issue gonzus#30: Missing JS array items cause Duktape’s duk_get_prop_index()
to return false. In that case pl_duk_to_perl_impl() was neglecting to
duk_pop(ctx), which caused the next read on the array to be an attempt
to read index 1 from undefined, which triggered an abort().

This fixes that by ensuring that we always duk_pop(ctx) in the relevant
piece of code and return Perl undef to represent missing array items.
  • Loading branch information
FGasper committed Dec 3, 2021
1 parent a318f23 commit 76bd2e3
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 9 deletions.
30 changes: 21 additions & 9 deletions pl_duk.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,28 @@ static SV* pl_duk_to_perl_impl(pTHX_ duk_context* ctx, int pos, HV* seen)
array_top = duk_get_length(ctx, pos);
for (j = 0; j < array_top; ++j) {
SV* nested = 0;
if (!duk_get_prop_index(ctx, pos, j)) {
continue; /* index doesn't exist => end of array */
}
nested = sv_2mortal(pl_duk_to_perl_impl(aTHX_ ctx, -1, seen));
duk_pop(ctx); /* value in current pos */
if (!nested) {
croak("Could not create Perl SV for array\n");

bool exists = duk_get_prop_index(ctx, pos, j);

/* NB: A nonexistent array value does NOT mean that */
/* we are at the end of the array; for example, you */
/* can do `foo[1] = 1` without defining `foo[0]`. */

if (exists) {
nested = sv_2mortal(pl_duk_to_perl_impl(aTHX_ ctx, -1, seen));
}
if (av_store(values_array, j, nested)) {
SvREFCNT_inc(nested);

/* Even if the array value is nonexistent we still */
/* have to remove it from the stack. */
duk_pop(ctx);

if (exists) {
if (!nested) {
croak("Could not create Perl SV for array\n");
}
if (av_store(values_array, j, nested)) {
SvREFCNT_inc(nested);
}
}
}
}
Expand Down
36 changes: 36 additions & 0 deletions t/26_eval_returns_undef_in_array.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use strict;
use warnings;

use Test::More;

use blib;

use JavaScript::Duktape::XS;

my $js = JavaScript::Duktape::XS->new();

my $got = $js->eval('var foo = []; foo[1] = 123; foo');

is_deeply(
$got,
[undef, 123],
'[(empty), 123]',
) or diag explain $got;

$got = $js->eval('var foo = []; foo[1] = undefined; foo');

is_deeply(
$got,
[undef, undef],
'[(empty), undefined]',
) or diag explain $got;

$got = $js->eval('[undefined, undefined]');

is_deeply(
$got,
[undef, undef],
'[undefined, undefined]',
) or diag explain $got;

done_testing;

0 comments on commit 76bd2e3

Please sign in to comment.