Skip to content

Commit 9524d82

Browse files
fix(worker): throw when an environment-data key cannot be converted to a string
setEnvironmentData and getEnvironmentData converted the key with a helper that swallows a throwing toString() and returns an empty string, so such a key silently read, overwrote or deleted the unrelated empty-string entry. The conversion is checked now and the exception reaches the caller.
1 parent 42a8bcf commit 9524d82

2 files changed

Lines changed: 47 additions & 2 deletions

File tree

‎test-app/app/src/main/assets/app/tests/testMessaging.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,34 @@ describe("Messaging runtime edges", function () {
204204
});
205205
});
206206

207+
describe("environment data keys", function () {
208+
function thrownMessage(fn) {
209+
try {
210+
fn();
211+
} catch (e) {
212+
return e && e.message;
213+
}
214+
return "nothing thrown";
215+
}
216+
217+
it("throws what a key's toString throws instead of using the empty-string key", function () {
218+
var wt = require("node:worker_threads");
219+
var key = { toString: function () { throw new Error("key toString failed"); } };
220+
wt.setEnvironmentData("", "empty");
221+
try {
222+
expect(thrownMessage(function () { wt.setEnvironmentData(key, "other"); }))
223+
.toBe("key toString failed");
224+
expect(thrownMessage(function () { wt.getEnvironmentData(key); }))
225+
.toBe("key toString failed");
226+
expect(thrownMessage(function () { wt.setEnvironmentData(key); }))
227+
.toBe("key toString failed");
228+
expect(wt.getEnvironmentData("")).toBe("empty");
229+
} finally {
230+
wt.setEnvironmentData("");
231+
}
232+
});
233+
});
234+
207235
describe("worker error reporting", function () {
208236
// A worker boots on its own thread, so the first error arrives whenever
209237
// the runner gets to it; specs wait for it and only then settle for

‎test-app/runtime/src/main/cpp/Messaging.cpp‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -935,13 +935,27 @@ void SetEmitMessageCallback(const FunctionCallbackInfo<Value>& info) {
935935
state->emitMessage.Reset(isolate, info[0].As<v8::Function>());
936936
}
937937

938+
// The key as the string the store files it under. A key whose conversion throws
939+
// leaves that exception pending for the caller rather than standing in for "".
940+
static bool EnvironmentDataKey(Isolate* isolate, Local<Value> value, std::string& key) {
941+
Local<v8::String> str;
942+
if (!value->ToString(isolate->GetCurrentContext()).ToLocal(&str)) {
943+
return false;
944+
}
945+
key = ArgConverter::ConvertToString(str);
946+
return true;
947+
}
948+
938949
void SetEnvironmentDataCallback(const FunctionCallbackInfo<Value>& info) {
939950
Isolate* isolate = info.GetIsolate();
940951
if (info.Length() < 1) {
941952
return;
942953
}
943954
Local<Context> context = isolate->GetCurrentContext();
944-
std::string key = ArgConverter::ToString(isolate, info[0]);
955+
std::string key;
956+
if (!EnvironmentDataKey(isolate, info[0], key)) {
957+
return;
958+
}
945959
if (info.Length() < 2 || info[1]->IsUndefined()) {
946960
std::lock_guard<std::mutex> lock(g_environmentDataMutex);
947961
g_environmentData.erase(key);
@@ -964,7 +978,10 @@ void GetEnvironmentDataCallback(const FunctionCallbackInfo<Value>& info) {
964978
if (info.Length() < 1) {
965979
return;
966980
}
967-
std::string key = ArgConverter::ToString(isolate, info[0]);
981+
std::string key;
982+
if (!EnvironmentDataKey(isolate, info[0], key)) {
983+
return;
984+
}
968985
std::shared_ptr<serialization::SerializedValue> stored;
969986
{
970987
std::lock_guard<std::mutex> lock(g_environmentDataMutex);

0 commit comments

Comments
 (0)