From 76bd2e33d2c1d64cb06b4ba3281ed7c9eb9f0732 Mon Sep 17 00:00:00 2001
From: Felipe Gasper <felipe@felipegasper.com>
Date: Wed, 17 Nov 2021 00:32:17 -0500
Subject: [PATCH] Handle missing elements in a returned array.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Issue #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.
---
 pl_duk.c                           | 30 +++++++++++++++++--------
 t/26_eval_returns_undef_in_array.t | 36 ++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 9 deletions(-)
 create mode 100644 t/26_eval_returns_undef_in_array.t

diff --git a/pl_duk.c b/pl_duk.c
index 2a7e50b..1e1717f 100644
--- a/pl_duk.c
+++ b/pl_duk.c
@@ -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);
+                            }
                         }
                     }
                 }
diff --git a/t/26_eval_returns_undef_in_array.t b/t/26_eval_returns_undef_in_array.t
new file mode 100644
index 0000000..604bfc7
--- /dev/null
+++ b/t/26_eval_returns_undef_in_array.t
@@ -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;