Home Original page

[py][bidi]: add BiDi script module commands by navin772 · Pull Request #15880 · SeleniumHQ/selenium

Possible issue
✅ Validate required JSON fields
Suggestion Impact:The suggestion was implemented with improvements. The commit added validation for required fields across multiple classes (RealmInfo, Source, EvaluateResult, ScriptMessage, RealmDestroyed) using individual field checks with specific error messages, and changed from json.get() to direct dictionary access for required fields.

code diff:

+        if "realm" not in json:
+            raise ValueError("Missing required field 'realm' in RealmInfo")
+        if "origin" not in json:
+            raise ValueError("Missing required field 'origin' in RealmInfo")
+        if "type" not in json:
+            raise ValueError("Missing required field 'type' in RealmInfo")
+
         return cls(
-            realm=json.get("realm"),
-            origin=json.get("origin"),
-            type=json.get("type"),
+            realm=json["realm"],
+            origin=json["origin"],
+            type=json["type"],
             context=json.get("context"),
             sandbox=json.get("sandbox"),
         )
@@ -83,7 +90,7 @@
     context: Optional[str] = None
 
     @classmethod
-    def from_json(cls, json: dict) -> "Source":
+    def from_json(cls, json: Dict[str, Any]) -> "Source":
         """Creates a Source instance from a dictionary.
 
         Parameters:
@@ -94,8 +101,11 @@
         -------
             Source: A new instance of Source.
         """
+        if "realm" not in json:
+            raise ValueError("Missing required field 'realm' in Source")
+
         return cls(
-            realm=json.get("realm"),
+            realm=json["realm"],
             context=json.get("context"),
         )
 
@@ -110,7 +120,7 @@
     exception_details: Optional[dict] = None
 
     @classmethod
-    def from_json(cls, json: dict) -> "EvaluateResult":
+    def from_json(cls, json: Dict[str, Any]) -> "EvaluateResult":
         """Creates an EvaluateResult instance from a dictionary.
 
         Parameters:
@@ -121,9 +131,14 @@
         -------
             EvaluateResult: A new instance of EvaluateResult.
         """
+        if "realm" not in json:
+            raise ValueError("Missing required field 'realm' in EvaluateResult")
+        if "type" not in json:
+            raise ValueError("Missing required field 'type' in EvaluateResult")
+
         return cls(
-            type=json.get("type"),
-            realm=json.get("realm"),
+            type=json["type"],
+            realm=json["realm"],
             result=json.get("result"),
             exception_details=json.get("exceptionDetails"),
         )
@@ -140,7 +155,7 @@
         self.source = source
 
     @classmethod
-    def from_json(cls, json: dict) -> "ScriptMessage":
+    def from_json(cls, json: Dict[str, Any]) -> "ScriptMessage":
         """Creates a ScriptMessage instance from a dictionary.
 
         Parameters:
@@ -151,10 +166,17 @@
         -------
             ScriptMessage: A new instance of ScriptMessage.
         """
+        if "channel" not in json:
+            raise ValueError("Missing required field 'channel' in ScriptMessage")
+        if "data" not in json:
+            raise ValueError("Missing required field 'data' in ScriptMessage")
+        if "source" not in json:
+            raise ValueError("Missing required field 'source' in ScriptMessage")
+
         return cls(
-            channel=json.get("channel"),
-            data=json.get("data"),
-            source=Source.from_json(json.get("source", {})),
+            channel=json["channel"],
+            data=json["data"],
+            source=Source.from_json(json["source"]),
         )
 
 
@@ -167,7 +189,7 @@
         self.realm_info = realm_info
 
     @classmethod
-    def from_json(cls, json: dict) -> "RealmCreated":
+    def from_json(cls, json: Dict[str, Any]) -> "RealmCreated":
         """Creates a RealmCreated instance from a dictionary.
 
         Parameters:
@@ -190,7 +212,7 @@
         self.realm = realm
 
     @classmethod
-    def from_json(cls, json: dict) -> "RealmDestroyed":
+    def from_json(cls, json: Dict[str, Any]) -> "RealmDestroyed":
         """Creates a RealmDestroyed instance from a dictionary.
 
         Parameters:
@@ -201,7 +223,10 @@
         -------
             RealmDestroyed: A new instance of RealmDestroyed.
         """
-        return cls(realm=json.get("realm"))
+        if "realm" not in json:
+            raise ValueError("Missing required field 'realm' in RealmDestroyed")
+
+        return cls(realm=json["realm"])

The from_json method should validate that required fields (realm, origin, type)
are present in the JSON data. Currently, it allows None values for required
fields which could cause issues downstream.

py/selenium/webdriver/common/bidi/script.py [57-75]

 @classmethod
 def from_json(cls, json: dict) -> "RealmInfo":
     """Creates a RealmInfo instance from a dictionary.
 
     Parameters:
     -----------
         json: A dictionary containing the realm information.
 
     Returns:
     -------
         RealmInfo: A new instance of RealmInfo.
     """
+    if not json.get("realm") or not json.get("origin") or not json.get("type"):
+        raise ValueError("Required fields 'realm', 'origin', and 'type' must be present")
+    
     return cls(
         realm=json.get("realm"),
         origin=json.get("origin"),
         type=json.get("type"),
         context=json.get("context"),
         sandbox=json.get("sandbox"),
     )

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that the from_json method for RealmInfo does not validate the presence of required fields (realm, origin, type). The RealmInfo dataclass defines these as non-optional strings, so allowing None values from json.get() could lead to downstream errors. Adding validation improves robustness by ensuring the data conforms to the expected structure.

Medium