Skip to content

Commit 8adf08f

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 8adf08f

2 files changed

Lines changed: 65 additions & 2 deletions

File tree

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

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

207+
describe("environment data keys", function () {
208+
it("keeps keys containing NUL distinct from their prefixes", function () {
209+
var wt = require("node:worker_threads");
210+
var prefix = "environment-key";
211+
var key = prefix + "\0suffix";
212+
try {
213+
wt.setEnvironmentData(prefix, "prefix");
214+
wt.setEnvironmentData(key, "full key");
215+
expect(wt.getEnvironmentData(prefix)).toBe("prefix");
216+
expect(wt.getEnvironmentData(key)).toBe("full key");
217+
wt.setEnvironmentData(key);
218+
expect(wt.getEnvironmentData(key)).toBeUndefined();
219+
expect(wt.getEnvironmentData(prefix)).toBe("prefix");
220+
} finally {
221+
wt.setEnvironmentData(key);
222+
wt.setEnvironmentData(prefix);
223+
}
224+
});
225+
226+
function thrownMessage(fn) {
227+
try {
228+
fn();
229+
} catch (e) {
230+
return e && e.message;
231+
}
232+
return "nothing thrown";
233+
}
234+
235+
it("throws what a key's toString throws instead of using the empty-string key", function () {
236+
var wt = require("node:worker_threads");
237+
var key = { toString: function () { throw new Error("key toString failed"); } };
238+
wt.setEnvironmentData("", "empty");
239+
try {
240+
expect(thrownMessage(function () { wt.setEnvironmentData(key, "other"); }))
241+
.toBe("key toString failed");
242+
expect(thrownMessage(function () { wt.getEnvironmentData(key); }))
243+
.toBe("key toString failed");
244+
expect(thrownMessage(function () { wt.setEnvironmentData(key); }))
245+
.toBe("key toString failed");
246+
expect(wt.getEnvironmentData("")).toBe("empty");
247+
} finally {
248+
wt.setEnvironmentData("");
249+
}
250+
});
251+
});
252+
207253
describe("worker error reporting", function () {
208254
// A worker boots on its own thread, so the first error arrives whenever
209255
// 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::ToString(isolate, 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)