[java] Add warn annotation for undeclared field while JSON coercion

[pujagani]

🔗 Related Issues

Will help with BiDi generator implementation. Part of the low-level ADR compliance.

💥 What does this PR do?

This is an annotation that if present will warn in case of any extra or undeclared fields while JSON coercing.

🔧 Implementation Notes

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s):
  • What was generated:
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)

[qodo-code-review]

PR Summary by Qodo

Add @WarnOnUnknownFields to log dropped JSON properties during coercion

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Introduce an opt-in annotation to warn when JSON includes unknown fields
• Log a warning when ConstructorCoercer drops undeclared properties for annotated types
• Add tests to verify warning/no-warning behavior and field population

Diagram

graph TD
  A["Json"] --> B["ConstructorCoercer"] --> C{ "Type annotated?" }
  C -->|"No"| D["Drop unknown fields"]
  C -->|"Yes"| E["java.util.logging.Logger"] --> F["Warn: dropped field"]
  B --> G["Target class"]
  G --> H["@WarnOnUnknownFields"]

  subgraph Legend
    direction LR
    _api["API"] ~~~ _comp["Component"] ~~~ _dec{"Decision"}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Global strict/lenient unknown-field mode

• ➕ Consistent behavior across all JSON coercions without per-type opt-in
• ➕ Easy to enforce correctness in security- or schema-sensitive contexts
• ➖ Risk of noisy logs or behavior change across the codebase
• ➖ Harder to roll out gradually for only selected DTOs

2. Fail-fast on unknown fields (throw exception)

• ➕ Strong schema enforcement; prevents silently ignoring important input
• ➕ Simplifies debugging by making mismatches immediately visible
• ➖ Potentially breaking for existing inputs that include extra fields
• ➖ Less flexible for forward/backward compatibility during protocol evolution

Recommendation: The opt-in annotation approach is a good fit for Selenium’s evolving JSON/BiDi models: it preserves current lenient behavior by default while enabling targeted visibility where schema compliance matters. Consider adding (later) an optional global toggle (or PropertySetting-based switch) for projects that want uniform behavior without annotating every type.

Files changed (3) +122 / -0

Enhancement (2) +49 / -0

ConstructorCoercer.javaLog warnings for unknown JSON fields on annotated types +15/-0

Log warnings for unknown JSON fields on annotated types

• Adds a JUL Logger and, when the declaring class is annotated with @WarnOnUnknownFields, emits a warning for any JSON property that does not map to a constructor parameter. Unknown properties are still ignored (not passed to the constructor); the change only adds observability.

java/src/org/openqa/selenium/json/ConstructorCoercer.java

WarnOnUnknownFields.javaIntroduce @WarnOnUnknownFields opt-in annotation +34/-0

Introduce @WarnOnUnknownFields opt-in annotation

• Adds a runtime-retained, type-level annotation used by ConstructorCoercer to decide whether to warn when encountering unknown JSON properties during constructor-based coercion.

java/src/org/openqa/selenium/json/WarnOnUnknownFields.java

Tests (1) +73 / -0

ConstructorCoercerTest.javaAdd tests for unknown-field warning behavior +73/-0

Add tests for unknown-field warning behavior

• Adds tests asserting that unknown JSON fields produce no logs by default, but do produce a warning when the target type is annotated. Includes a small log-capture helper and verifies known fields are still populated.

java/test/org/openqa/selenium/json/ConstructorCoercerTest.java

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Log-level dependent tests ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New

Description

The new/updated tests assume a WARNING log record is always emitted, but production code only logs
when LOG.isLoggable(Level.WARNING) and the test helper does not force logger/handler levels. This
can make the tests fail under different java.util.logging configurations.

Code

java/test/org/openqa/selenium/json/ConstructorCoercerTest.java[R314-317]

+    List<LogRecord> records = captureLogRecords(() -> new Json().toType(raw, WarnOnUnknown.class));
+
+    assertThat(records).hasSize(1);
+    assertThat(records.get(0).getMessage()).contains("...(truncated)").doesNotContain(longKey);

Evidence

The warning path is explicitly guarded by LOG.isLoggable(Level.WARNING), while the tests rely on
captured records without ensuring WARNING is enabled on the logger they attach a handler to.

java/src/org/openqa/selenium/json/ConstructorCoercer.java[292-304]
java/test/org/openqa/selenium/json/ConstructorCoercerTest.java[299-318]
java/test/org/openqa/selenium/json/ConstructorCoercerTest.java[320-342]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Tests assert exactly one WARNING record is captured, but `ConstructorCoercer` only emits warnings when WARNING is loggable. The helper `captureLogRecords` adds a handler but does not set the logger (or handler) level, so warnings may be suppressed depending on environment config.

### Issue Context
`ConstructorCoercer` gates warning emission using `LOG.isLoggable(Level.WARNING)`.

### Fix Focus Areas
- java/test/org/openqa/selenium/json/ConstructorCoercerTest.java[320-342]
- java/src/org/openqa/selenium/json/ConstructorCoercer.java[292-304]

### Implementation notes
- In `captureLogRecords`:
 - Save `Level prevLevel = logger.getLevel()` and set `logger.setLevel(Level.ALL)` (or at least `Level.WARNING`).
 - Set `handler.setLevel(Level.ALL)`.
 - Optionally set `logger.setUseParentHandlers(false)` during capture to reduce noise, then restore it.
 - Restore prior logger level/parent-handler setting in `finally` along with removing the handler.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unbounded unknown key collection ✓ Resolved 🐞 Bug ➹ Performance ⭐ New

Description

ConstructorCandidate.create() collects *all* unknown JSON keys into an ArrayList before logging a
capped summary, adding avoidable O(n) extra memory/work for large attacker-controlled payloads. This
contradicts the intent of bounding the logging cost and can worsen resource usage for
@WarnOnUnknownFields types.

Code

java/src/org/openqa/selenium/json/ConstructorCoercer.java[R296-299]

+        List<String> unknownKeys = new ArrayList<>();
+        for (String key : properties.keySet()) {
+          if (!parameterIndexes.containsKey(key)) {
+            unknownKeys.add(key);

Evidence

The code comment states the summary should keep the cost bounded, but create() still stores every
unknown key into a list before logging, which grows linearly with the number of unknown keys.

java/src/org/openqa/selenium/json/ConstructorCoercer.java[82-106]
java/src/org/openqa/selenium/json/ConstructorCoercer.java[292-305]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ConstructorCandidate.create()` builds `unknownKeys` by appending every unknown property name into a list, even though the log message only prints the first `MAX_LOGGED_UNKNOWN_KEYS`. This adds unnecessary linear memory overhead and work.

### Issue Context
The warning path is explicitly about keeping logging bounded; currently only the *message size* is capped, not the temporary list size.

### Fix Focus Areas
- java/src/org/openqa/selenium/json/ConstructorCoercer.java[292-305]

### Implementation notes
- Replace `List<String> unknownKeys = new ArrayList<>();` with:
 - `int unknownCount = 0;`
 - `List<String> sampleKeys = new ArrayList<>(MAX_LOGGED_UNKNOWN_KEYS);`
- For each unknown key: increment `unknownCount`; add to `sampleKeys` only if `sampleKeys.size() < MAX_LOGGED_UNKNOWN_KEYS`.
- Update `describeUnknownFields(...)` to accept `(declaringClass, unknownCount, sampleKeys)` (or similar) so you can log the true count while only retaining a bounded sample list.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Log forging via JSON keys ✓ Resolved 🐞 Bug ⛨ Security

Description

ConstructorCoercer logs unknown JSON property names verbatim; JSON keys can contain newline/control
characters, producing confusing multi-line log output and undermining log integrity when parsing
untrusted payloads. This behavior is introduced by the new @WarnOnUnknownFields logging path.

Code

java/src/org/openqa/selenium/json/ConstructorCoercer.java[R229-232]

+                constructor.getDeclaringClass().getSimpleName()
+                    + ": dropping undeclared field \""
+                    + key
+                    + "\"");

Evidence

Unknown field names come from parsed JSON object keys (properties.keySet()), and the code
concatenates key directly into a warning message without escaping/sanitization.

java/src/org/openqa/selenium/json/ConstructorCoercer.java[61-69]
java/src/org/openqa/selenium/json/ConstructorCoercer.java[225-235]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ConstructorCoercer` logs unknown JSON property names directly into a WARNING message. Because JSON keys are attacker-controlled strings, they may contain control characters (e.g., `\n`, `\r`, tabs) that can break line-oriented log ingestion or appear as forged log lines.

### Issue Context
The warning is emitted only when the target type is annotated with `@WarnOnUnknownFields`, but that annotation is explicitly intended for stricter handling, which commonly applies to untrusted inputs.

### Fix Focus Areas
- java/src/org/openqa/selenium/json/ConstructorCoercer.java[225-235]

### Suggested fix
- Escape or strip control characters from `key` before including it in the log message (e.g., replace `\r`/`\n` with `\\r`/`\\n`).
- Optionally truncate very long keys to a safe maximum length before logging.
- Prefer supplier-based logging (`LOG.warning(() -> ...)`) after sanitization to avoid eager message construction.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View medium (1)

4. Per-field warning log flood ✓ Resolved 🐞 Bug ☼ Reliability

Description

For @WarnOnUnknownFields types, ConstructorCoercer emits one WARNING per unknown property, which can
amplify log volume and overhead for payloads with many extra keys. This can degrade
reliability/operability by flooding logs rather than producing a bounded summary.

Code

java/src/org/openqa/selenium/json/ConstructorCoercer.java[R226-229]

+        for (String key : properties.keySet()) {
+          if (!parameterIndexes.containsKey(key)) {
+            LOG.warning(
+                constructor.getDeclaringClass().getSimpleName()

Evidence

The new code iterates over every JSON property key and logs a WARNING for each key not found in
parameterIndexes, creating 1 log record per unknown key.

java/src/org/openqa/selenium/json/ConstructorCoercer.java[225-235]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ConstructorCoercer` currently logs a separate WARNING for every unknown JSON key when `@WarnOnUnknownFields` is present. Large objects with many unknown keys will emit many log records, increasing CPU/I/O and potentially overwhelming log storage/alerting.

### Issue Context
This is in the JSON coercion hot path. Even if request sizes are constrained upstream, the library-level behavior should ideally be bounded to avoid accidental or adversarial log amplification.

### Fix Focus Areas
- java/src/org/openqa/selenium/json/ConstructorCoercer.java[225-235]

### Suggested fix
- Collect unknown keys and log a single summary warning (e.g., `X: dropped N undeclared fields: [a, b, c, ...]`).
- Cap the number of keys included in the message (e.g., first 10) and include the total count.
- Optionally guard message construction with `LOG.isLoggable(Level.WARNING)` and/or use supplier-based logging.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context

✅ Compliance rules (platform): 18 rules

Tip of the day

💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous review results

Review updated until commit 0b246e9

Results up to commit 21a16f3 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Log forging via JSON keys ✓ Resolved 🐞 Bug ⛨ Security

Description

ConstructorCoercer logs unknown JSON property names verbatim; JSON keys can contain newline/control
characters, producing confusing multi-line log output and undermining log integrity when parsing
untrusted payloads. This behavior is introduced by the new @WarnOnUnknownFields logging path.

Code

java/src/org/openqa/selenium/json/ConstructorCoercer.java[R229-232]

+                constructor.getDeclaringClass().getSimpleName()
+                    + ": dropping undeclared field \""
+                    + key
+                    + "\"");

Evidence

Unknown field names come from parsed JSON object keys (properties.keySet()), and the code
concatenates key directly into a warning message without escaping/sanitization.

java/src/org/openqa/selenium/json/ConstructorCoercer.java[61-69]
java/src/org/openqa/selenium/json/ConstructorCoercer.java[225-235]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ConstructorCoercer` logs unknown JSON property names directly into a WARNING message. Because JSON keys are attacker-controlled strings, they may contain control characters (e.g., `\n`, `\r`, tabs) that can break line-oriented log ingestion or appear as forged log lines.

### Issue Context
The warning is emitted only when the target type is annotated with `@WarnOnUnknownFields`, but that annotation is explicitly intended for stricter handling, which commonly applies to untrusted inputs.

### Fix Focus Areas
- java/src/org/openqa/selenium/json/ConstructorCoercer.java[225-235]

### Suggested fix
- Escape or strip control characters from `key` before including it in the log message (e.g., replace `\r`/`\n` with `\\r`/`\\n`).
- Optionally truncate very long keys to a safe maximum length before logging.
- Prefer supplier-based logging (`LOG.warning(() -> ...)`) after sanitization to avoid eager message construction.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Per-field warning log flood ✓ Resolved 🐞 Bug ☼ Reliability

Description

For @WarnOnUnknownFields types, ConstructorCoercer emits one WARNING per unknown property, which can
amplify log volume and overhead for payloads with many extra keys. This can degrade
reliability/operability by flooding logs rather than producing a bounded summary.

Code

java/src/org/openqa/selenium/json/ConstructorCoercer.java[R226-229]

+        for (String key : properties.keySet()) {
+          if (!parameterIndexes.containsKey(key)) {
+            LOG.warning(
+                constructor.getDeclaringClass().getSimpleName()

Evidence

The new code iterates over every JSON property key and logs a WARNING for each key not found in
parameterIndexes, creating 1 log record per unknown key.

java/src/org/openqa/selenium/json/ConstructorCoercer.java[225-235]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ConstructorCoercer` currently logs a separate WARNING for every unknown JSON key when `@WarnOnUnknownFields` is present. Large objects with many unknown keys will emit many log records, increasing CPU/I/O and potentially overwhelming log storage/alerting.

### Issue Context
This is in the JSON coercion hot path. Even if request sizes are constrained upstream, the library-level behavior should ideally be bounded to avoid accidental or adversarial log amplification.

### Fix Focus Areas
- java/src/org/openqa/selenium/json/ConstructorCoercer.java[225-235]

### Suggested fix
- Collect unknown keys and log a single summary warning (e.g., `X: dropped N undeclared fields: [a, b, c, ...]`).
- Cap the number of keys included in the message (e.g., first 10) and include the total count.
- Optionally guard message construction with `LOG.isLoggable(Level.WARNING)` and/or use supplier-based logging.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 0ca8254

[qodo-code-review]

Code review by qodo was updated up to the latest commit 0b246e9