Skip to content

Commit 6a6a378

Browse files
sergedehburg
sergedeh
authored and
burg
committed
[Web Inspector] Add validation for optional enum parameters
https://bugs.webkit.org/show_bug.cgi?id=304062 Reviewed by Devin Rousso and BJ Burg. The Inspector protocol code generator was not validating optional enum parameters, allowing invalid values to pass through to the backend implementation. This is a followup to #58033, handling enum validation at the JSON-RPC error code level. Per the JSON-RPC 2.0 specification (https://www.jsonrpc.org/specification), invalid parameter values should return error code -32602 (Invalid params). The W3C WebDriver BiDi specification (https://w3c.github.io/webdriver-bidi/) explicitly follows JSON-RPC 2.0 and requires this error code for invalid enum values. This fix uses the framework settings system to configure error codes for enum validation. The WebDriverBidi and Test frameworks use InvalidParams (-32602) for both required and optional enum parameters, while other frameworks preserve the existing ServerError (-32000) behavior for compatibility. The per-framework settings can be unified once downstream issues (such as Connection.js test breakages) are resolved. Tests: * imported/w3c/webdriver/tests/bidi/script/get_realms/invalid.py::test_params_type_invalid_value (protocol test) * Source/JavaScriptCore/inspector/scripts/tests/webdriver-bidi-enum-validation.json (generator test) * Source/JavaScriptCore/inspector/scripts/codegen/models.py: Add enum_invalid_value_error_code setting to WebDriverBidi and Test framework configurations. * Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_backend_dispatcher_implementation.py: Read error code from framework setting. Apply InvalidParams or ServerError for both required and optional enum parameters based on framework configuration. * Source/JavaScriptCore/inspector/scripts/tests/webdriver-bidi-enum-validation.json: Add generator test fixture covering required and optional enum parameters. * Source/JavaScriptCore/inspector/scripts/tests/expected/*.json-result: Update expected outputs for Test framework to use InvalidParams for enum validation. * WebDriverTests/TestExpectations.json: Remove expected failure for test_params_type_invalid_value test. Canonical link: https://commits.webkit.org/310738@main
1 parent 77db692 commit 6a6a378

7 files changed

Lines changed: 1222 additions & 10 deletions

‎Source/JavaScriptCore/inspector/scripts/codegen/generate_cpp_backend_dispatcher_implementation.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ def _generate_dispatcher_implementation_for_command(self, command, domain):
223223
parameter_enum_resolutions = []
224224
alternate_dispatcher_method_parameters = ['protocol_requestId']
225225
method_parameters = []
226+
enum_error_code = self.model().framework.setting('enum_invalid_value_error_code', 'ServerError')
226227

227228
# Step 1: build up arguments for the call.
228229
for parameter in command.call_parameters:
@@ -255,11 +256,16 @@ def _generate_dispatcher_implementation_for_command(self, command, domain):
255256
if len(parameter_enum_resolutions):
256257
parameter_enum_resolutions.append('')
257258
parameter_enum_resolutions.append(' auto %(enumVariableName)s = Protocol::%(helpersNamespace)s::parseEnumValueFromString<%(enumType)s>(%(stringVariableName)s);' % enum_args)
259+
enum_args['enumErrorCode'] = enum_error_code
258260
if parameter.is_optional:
261+
parameter_enum_resolutions.append(' if (!%(stringVariableName)s.isEmpty() && !%(enumVariableName)s) {' % enum_args)
262+
parameter_enum_resolutions.append(' m_backendDispatcher->reportProtocolError(BackendDispatcher::%(enumErrorCode)s, makeString("Unknown %(parameterKey)s: "_s, %(stringVariableName)s));' % enum_args)
263+
parameter_enum_resolutions.append(' return;')
264+
parameter_enum_resolutions.append(' }')
259265
parameter_expression = 'WTF::move(%s)' % variable_name
260266
else:
261267
parameter_enum_resolutions.append(' if (!%(enumVariableName)s) {' % enum_args)
262-
parameter_enum_resolutions.append(' m_backendDispatcher->reportProtocolError(BackendDispatcher::ServerError, makeString("Unknown %(parameterKey)s: "_s, %(stringVariableName)s));' % enum_args)
268+
parameter_enum_resolutions.append(' m_backendDispatcher->reportProtocolError(BackendDispatcher::%(enumErrorCode)s, makeString("Unknown %(parameterKey)s: "_s, %(stringVariableName)s));' % enum_args)
263269
parameter_enum_resolutions.append(' return;')
264270
parameter_enum_resolutions.append(' }')
265271
parameter_expression = '*' + variable_name

‎Source/JavaScriptCore/inspector/scripts/codegen/models.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ def validate_target_types(debuggable_types, target_types):
7878
"cpp_protocol_group": "WebDriverBidi",
7979
"objc_protocol_group": "WD",
8080
"objc_prefix": "WD",
81+
"enum_invalid_value_error_code": "InvalidParams",
8182
},
8283
"WebInspector": {
8384
"objc_protocol_group": "RWI",
@@ -91,6 +92,7 @@ def validate_target_types(debuggable_types, target_types):
9192
"cpp_protocol_group": "Test",
9293
"objc_protocol_group": "Test",
9394
"objc_prefix": "Test",
95+
"enum_invalid_value_error_code": "InvalidParams",
9496
}
9597
}
9698

‎Source/JavaScriptCore/inspector/scripts/tests/expected/commands-with-optional-call-return-parameters.json-result‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,10 @@ void DatabaseBackendDispatcher::executeAllOptionalParameters(long protocol_reque
314314
#endif // ENABLE(INSPECTOR_ALTERNATE_DISPATCHERS)
315315

316316
auto in_opt_screenColor = Protocol::TestHelpers::parseEnumValueFromString<Protocol::Database::PrimaryColors>(in_opt_screenColor_json);
317+
if (!in_opt_screenColor_json.isEmpty() && !in_opt_screenColor) {
318+
m_backendDispatcher->reportProtocolError(BackendDispatcher::InvalidParams, makeString("Unknown screenColor: "_s, in_opt_screenColor_json));
319+
return;
320+
}
317321

318322
auto result = m_agent->executeAllOptionalParameters(WTF::move(in_opt_columnNames), in_opt_notes, WTF::move(in_opt_timestamp), WTF::move(in_opt_values), WTF::move(in_opt_payload), WTF::move(in_opt_databaseId), WTF::move(in_opt_sqlError), WTF::move(in_opt_screenColor), WTF::move(in_opt_alternateColors), in_opt_printColor);
319323
if (!result) {
@@ -374,7 +378,7 @@ void DatabaseBackendDispatcher::executeNoOptionalParameters(long protocol_reques
374378

375379
auto in_screenColor = Protocol::TestHelpers::parseEnumValueFromString<Protocol::Database::PrimaryColors>(in_screenColor_json);
376380
if (!in_screenColor) {
377-
m_backendDispatcher->reportProtocolError(BackendDispatcher::ServerError, makeString("Unknown screenColor: "_s, in_screenColor_json));
381+
m_backendDispatcher->reportProtocolError(BackendDispatcher::InvalidParams, makeString("Unknown screenColor: "_s, in_screenColor_json));
378382
return;
379383
}
380384

‎Source/JavaScriptCore/inspector/scripts/tests/expected/enum-values.json-result‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,11 +324,15 @@ void CommandDomainBackendDispatcher::command(long protocol_requestId, RefPtr<JSO
324324

325325
auto in_enumRequired = Protocol::TestHelpers::parseEnumValueFromString<Protocol::TypeDomain::Enum>(in_enumRequired_json);
326326
if (!in_enumRequired) {
327-
m_backendDispatcher->reportProtocolError(BackendDispatcher::ServerError, makeString("Unknown enumRequired: "_s, in_enumRequired_json));
327+
m_backendDispatcher->reportProtocolError(BackendDispatcher::InvalidParams, makeString("Unknown enumRequired: "_s, in_enumRequired_json));
328328
return;
329329
}
330330

331331
auto in_opt_enumOptional = Protocol::TestHelpers::parseEnumValueFromString<Protocol::TypeDomain::Enum>(in_opt_enumOptional_json);
332+
if (!in_opt_enumOptional_json.isEmpty() && !in_opt_enumOptional) {
333+
m_backendDispatcher->reportProtocolError(BackendDispatcher::InvalidParams, makeString("Unknown enumOptional: "_s, in_opt_enumOptional_json));
334+
return;
335+
}
332336

333337
auto result = m_agent->command(*in_enumRequired, WTF::move(in_opt_enumOptional), in_parameterRequired, in_opt_parameterOptional);
334338
if (!result) {

0 commit comments

Comments
 (0)