Skip to content

Commit c1a8a70

Browse files
authored
webdriver: Fix JS serialization for object with special key (servo#39804)
Previously, the JS deserialization is wrong for object like `{"-": 3}`. This was because we never generate a valid JSON string literal from the request. Now I share @Loirooriol's laughter: "We just fixed something very basic, but there is no existing test covering this", after fixing a bunch of things in past few days. Testing: Added two new subtests to [wdspec](https://web-platform-tests.org/writing-tests/wdspec.html). Fixes: servo#38083 --------- Signed-off-by: Euclid Ye <[email protected]>
1 parent 1c24269 commit c1a8a70

File tree

4 files changed

+16
-17
lines changed

4 files changed

+16
-17
lines changed

components/webdriver_server/elements.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use embedder_traits::WebDriverScriptCommand;
66
use ipc_channel::ipc;
77
use serde_json::Value;
88
use webdriver::command::JavascriptCommandParameters;
9-
use webdriver::error::{ErrorStatus, WebDriverError, WebDriverResult};
9+
use webdriver::error::{WebDriverError, WebDriverResult};
1010

1111
use crate::{Handler, VerifyBrowsingContextIsOpen, wait_for_ipc_response};
1212

@@ -26,11 +26,12 @@ impl Handler {
2626
parameters: JavascriptCommandParameters,
2727
) -> WebDriverResult<(String, Vec<String>)> {
2828
// Step 1. Let script be the result of getting a property named "script" from parameters
29-
// Step 2. (Skip) If script is not a String, return error with error code invalid argument.
29+
// Step 2. (Done) If script is not a String, return error with error code invalid argument.
3030
let script = parameters.script;
3131

3232
// Step 3. Let args be the result of getting a property named "args" from parameters.
33-
// Step 4. (Skip) If args is not an Array return error with error code invalid argument.
33+
// Step 4. (Done) If args is not an Array return error with error code invalid argument.
34+
// Step 5. Let `arguments` be JSON deserialize with session and args.
3435
let args: Vec<String> = parameters
3536
.args
3637
.as_deref()
@@ -45,9 +46,9 @@ impl Handler {
4546
/// <https://w3c.github.io/webdriver/#dfn-deserialize-a-web-element>
4647
pub(crate) fn deserialize_web_element(&self, element: &Value) -> WebDriverResult<String> {
4748
// Step 2. Let reference be the result of getting the web element identifier property from object.
48-
let element_ref: String = match element {
49+
let element_ref = match element {
4950
Value::String(s) => s.clone(),
50-
_ => element.to_string(),
51+
_ => unreachable!(),
5152
};
5253

5354
// Step 3. Let element be the result of trying to get a known element with session and reference.
@@ -69,7 +70,7 @@ impl Handler {
6970
// Step 2. Let reference be the result of getting the shadow root identifier property from object.
7071
let shadow_root_ref = match shadow_root {
7172
Value::String(s) => s.clone(),
72-
_ => shadow_root.to_string(),
73+
_ => unreachable!(),
7374
};
7475

7576
// Step 3. Let element be the result of trying to get a known element with session and reference.
@@ -123,18 +124,14 @@ impl Handler {
123124
let elems = map
124125
.iter()
125126
.map(|(k, v)| {
127+
let key = serde_json::to_string(k)?;
126128
let arg = self.json_deserialize(v)?;
127-
Ok(format!("{}: {}", k, arg))
129+
Ok(format!("{key}: {arg}"))
128130
})
129131
.collect::<WebDriverResult<Vec<String>>>()?;
130132
format!("{{{}}}", elems.join(", "))
131133
},
132-
_ => serde_json::to_string(v).map_err(|_| {
133-
WebDriverError::new(
134-
ErrorStatus::InvalidArgument,
135-
"Failed to serialize script argument",
136-
)
137-
})?,
134+
_ => serde_json::to_string(v)?,
138135
};
139136

140137
Ok(res)

tests/wpt/meta/MANIFEST.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -956339,7 +956339,7 @@
956339956339
},
956340956340
"execute_async_script": {
956341956341
"arguments.py": [
956342-
"93d323c02e5dee5bcd0a322de4fc9d5bf70fc815",
956342+
"d678d569be0e2425c9b669caf11e664bb4be204e",
956343956343
[
956344956344
null,
956345956345
{}
@@ -956413,7 +956413,7 @@
956413956413
},
956414956414
"execute_script": {
956415956415
"arguments.py": [
956416-
"aa3b3d6e9d951b61319e3fdcda43c6438174b015",
956416+
"caf1790955370b5bd15c9f06cc1040d969334ac8",
956417956417
[
956418956418
null,
956419956419
{}

tests/wpt/tests/webdriver/tests/classic/execute_async_script/arguments.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ def test_null(session):
2323
("foo", "string"),
2424
("foo\"bar", 'string'),
2525
('"); alert(1); //', "string"),
26-
], ids=["boolean", "number", "string", "string_quote", "string_injection"])
26+
({"foo-bar":"bar-foo"}, "object")
27+
], ids=["boolean", "number", "string", "string_quote", "string_injection", "special_key_object"])
2728
def test_primitives(session, value, expected_type):
2829
result = execute_async_script(session, """
2930
arguments[1]([typeof arguments[0], arguments[0]])

tests/wpt/tests/webdriver/tests/classic/execute_script/arguments.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ def test_null(session):
2121
("foo", "string"),
2222
("foo\"bar", 'string'),
2323
('"); alert(1); //', "string"),
24-
], ids=["boolean", "number", "string", "string_quote", "string_injection"])
24+
({"foo-bar":"bar-foo"}, "object")
25+
], ids=["boolean", "number", "string", "string_quote", "string_injection", "special_key_object"])
2526
def test_primitives(session, value, expected_type):
2627
result = execute_script(session, "return [typeof arguments[0], arguments[0]]", args=[value])
2728
actual = assert_success(result)

0 commit comments

Comments
 (0)