Skip to content

Commit

Permalink
Fix setting array elements through JSI
Browse files Browse the repository at this point in the history
Summary:
X-link: facebook/react-native#38521

`HermesRuntimeImpl::setValueAtIndexImpl` directly called
`setElementAt`, which meant that it would not handle index-like
properties properly. Use `putComputed_RJS` instead, which will check
for such properties.

Reviewed By: avp

Differential Revision: D47617445

fbshipit-source-id: c3505670960bd6223bd014f138cee191267a5315
  • Loading branch information
neildhar authored and facebook-github-bot committed Jul 20, 2023
1 parent e6afd14 commit 3a794e6
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
8 changes: 6 additions & 2 deletions API/hermes/hermes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2045,8 +2045,12 @@ void HermesRuntimeImpl::setValueAtIndexImpl(
")");
}

auto h = arrayHandle(arr);
h->setElementAt(h, runtime_, i, vmHandleFromValue(value));
auto res = vm::JSObject::putComputed_RJS(
arrayHandle(arr),
runtime_,
runtime_.makeHandle(vm::HermesValue::encodeTrustedNumberValue(i)),
vmHandleFromValue(value));
checkStatus(res.getStatus());
}

jsi::Function HermesRuntimeImpl::createFunctionFromHostFunction(
Expand Down
14 changes: 14 additions & 0 deletions API/jsi/jsi/test/testlib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,20 @@ TEST_P(JSITest, ArrayTest) {
Array alpha2 = Array(rt, 1);
alpha2 = std::move(alpha);
EXPECT_EQ(alpha2.size(rt), 4);

// Test getting/setting an element that is an accessor.
auto arrWithAccessor =
eval(
"Object.defineProperty([], '0', {set(){ throw 72 }, get(){ return 45 }});")
.getObject(rt)
.getArray(rt);
try {
arrWithAccessor.setValueAtIndex(rt, 0, 1);
FAIL() << "Expected exception";
} catch (const JSError& err) {
EXPECT_EQ(err.value().getNumber(), 72);
}
EXPECT_EQ(arrWithAccessor.getValueAtIndex(rt, 0).getNumber(), 45);
}

TEST_P(JSITest, FunctionTest) {
Expand Down

0 comments on commit 3a794e6

Please sign in to comment.