Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions Source/JavaScriptCore/inspector/InjectedScriptBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ static RefPtr<JSON::Value> jsToInspectorValue(JSC::JSGlobalObject* globalObject,
if (value.isString())
return JSON::Value::create(asString(value)->value(globalObject).data);

// BigInt and Symbol have no JSON representation. They can reach this point
// when Runtime.evaluate / callFunctionOn is invoked with returnByValue on a
// result that holds one (e.g. `[1n]` or `Symbol()`), so report the value as
// not representable rather than hitting ASSERT_NOT_REACHED below.
if (value.isBigInt() || value.isSymbol())
return nullptr;
Comment on lines +69 to +74

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add regression coverage for non-JSON values.

Add or verify tests for direct 1n, direct Symbol(), and nested values such as [1n] with returnByValue: true. Cover the synchronous and asynchronous paths, including Runtime.evaluate, Runtime.callFunctionOn, and Debugger.evaluateOnCallFrame where applicable. Assert "Object couldn't be returned by value" and verify that execution does not reach ASSERT_NOT_REACHED.

🧰 Tools
🪛 Cppcheck (2.21.0)

[style] 69-69: The function 'inspectorEnvironment' is never used.

(unusedFunction)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/inspector/InjectedScriptBase.cpp` around lines 69 - 74,
Add regression tests for the non-JSON handling in InjectedScriptBase, covering
direct 1n, direct Symbol(), and nested [1n] values with returnByValue enabled.
Exercise synchronous and asynchronous Runtime.evaluate and
Runtime.callFunctionOn flows, plus Debugger.evaluateOnCallFrame where supported,
and assert the "Object couldn't be returned by value" error while confirming
execution completes without reaching ASSERT_NOT_REACHED.


if (value.isObject()) {
if (isJSArray(value)) {
auto inspectorArray = JSON::Array::create();
Expand Down Expand Up @@ -163,7 +170,7 @@ Ref<JSON::Value> InjectedScriptBase::makeCall(ScriptFunctionCall& function)

auto resultJSONValue = toInspectorValue(globalObject, value);
if (!resultJSONValue)
return JSON::Value::create(makeString("Object has too long reference chain (must not be longer than "_s, JSON::Value::maxDepth, ')'));
return JSON::Value::create(String("Object couldn't be returned by value"_s));

return resultJSONValue.releaseNonNull();
}
Expand Down Expand Up @@ -193,7 +200,7 @@ void InjectedScriptBase::makeAsyncCall(ScriptFunctionCall& function, AsyncCallCa
else if (auto resultJSONValue = toInspectorValue(globalObject, callFrame->argument(0)))
checkAsyncCallResult(resultJSONValue, callback);
else
checkAsyncCallResult(JSON::Value::create(makeString("Object has too long reference chain (must not be longer than "_s, JSON::Value::maxDepth, ')')), callback);
checkAsyncCallResult(JSON::Value::create(String("Object couldn't be returned by value"_s)), callback);
return JSC::JSValue::encode(JSC::jsUndefined());
});
}
Expand Down
Loading