Skip to content

Conversation

@nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Nov 30, 2025

User description

Avoid metadata generation in JsonSerializerContexts.

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Bug fix


Description

  • Add [JsonIgnore] attributes to extension properties in BrowsingContext

  • Prevent non-serializable properties from being included in JSON serialization

  • Import System.Text.Json.Serialization namespace for attribute support


Diagram Walkthrough

flowchart LR
  A["BrowsingContext class"] -->|Add JsonIgnore| B["BiDi property"]
  A -->|Add JsonIgnore| C["Log property"]
  A -->|Add JsonIgnore| D["Network property"]
  A -->|Add JsonIgnore| E["Script property"]
  A -->|Add JsonIgnore| F["Storage property"]
  A -->|Add JsonIgnore| G["Input property"]
  B -->|Prevents serialization| H["JSON output"]
  C -->|Prevents serialization| H
  D -->|Prevents serialization| H
  E -->|Prevents serialization| H
  F -->|Prevents serialization| H
  G -->|Prevents serialization| H
Loading

File Walkthrough

Relevant files
Bug fix
BrowsingContext.cs
Add JsonIgnore attributes to extension properties               

dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContext.cs

  • Added using System.Text.Json.Serialization; import statement
  • Applied [JsonIgnore] attribute to six extension properties: BiDi, Log,
    Network, Script, Storage, and Input
  • Prevents these non-serializable properties from being included during
    JSON serialization
+7/-0     

@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No auditing: The new changes only add JsonIgnore attributes and do not implement or modify any logging
of critical actions, so there is no evidence of audit trails for sensitive operations in
the added code.

Referred Code
[JsonIgnore]
public BiDi BiDi { get; }

[JsonIgnore]
public BrowsingContextLogModule Log => _logModule.Value;

[JsonIgnore]
public BrowsingContextNetworkModule Network => _networkModule.Value;

[JsonIgnore]
public BrowsingContextScriptModule Script => _scriptModule.Value;

[JsonIgnore]
public BrowsingContextStorageModule Storage => _storageModule.Value;

[JsonIgnore]
public BrowsingContextInputModule Input => _inputModule.Value;

public Task<NavigateResult> NavigateAsync(string url, NavigateOptions? options = null)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
No error handling: The newly added attributes and property annotations introduce no error handling or
edge-case management for serialization-related failures, leaving potential null or lazy
initialization issues unaddressed in this diff.

Referred Code
[JsonIgnore]
public BiDi BiDi { get; }

[JsonIgnore]
public BrowsingContextLogModule Log => _logModule.Value;

[JsonIgnore]
public BrowsingContextNetworkModule Network => _networkModule.Value;

[JsonIgnore]
public BrowsingContextScriptModule Script => _scriptModule.Value;

[JsonIgnore]
public BrowsingContextStorageModule Storage => _storageModule.Value;

[JsonIgnore]
public BrowsingContextInputModule Input => _inputModule.Value;

public Task<NavigateResult> NavigateAsync(string url, NavigateOptions? options = null)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
No input validation: The changes only mark properties with JsonIgnore and add a using directive, providing no
evidence of input validation or security checks for data handled by these properties in
the added code.

Referred Code
[JsonIgnore]
public BiDi BiDi { get; }

[JsonIgnore]
public BrowsingContextLogModule Log => _logModule.Value;

[JsonIgnore]
public BrowsingContextNetworkModule Network => _networkModule.Value;

[JsonIgnore]
public BrowsingContextScriptModule Script => _scriptModule.Value;

[JsonIgnore]
public BrowsingContextStorageModule Storage => _storageModule.Value;

[JsonIgnore]
public BrowsingContextInputModule Input => _inputModule.Value;

public Task<NavigateResult> NavigateAsync(string url, NavigateOptions? options = null)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Validate URL input upfront

Validate that url is non-empty and a well-formed absolute URI before navigating;
throw a meaningful exception if invalid.

dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContext.cs [66-67]

 public Task<NavigateResult> NavigateAsync(string url, NavigateOptions? options = null)
 {
+    if (string.IsNullOrWhiteSpace(url) || !Uri.TryCreate(url.Trim(), UriKind.Absolute, out var _))
+    {
+        throw new ArgumentException("A valid absolute URL must be provided.", nameof(url));
+    }
     ...
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Validate and sanitize external inputs (e.g., URLs) before use to avoid invalid requests and injection issues.

Low
  • More

@nvborisenko nvborisenko merged commit 4c6474b into SeleniumHQ:trunk Nov 30, 2025
14 of 15 checks passed
@nvborisenko nvborisenko deleted the bidi-ignore-non-serializable branch November 30, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants