Skip to content

fix: handle invalid JSON fields in cryptor plugin - #6960

Merged
Aias00 merged 2 commits into
apache:masterfrom
SouthwestAsiaFloat:issue-6878
Aug 23, 2026
Merged

fix: handle invalid JSON fields in cryptor plugin#6960
Aias00 merged 2 commits into
apache:masterfrom
SouthwestAsiaFloat:issue-6878

Conversation

@SouthwestAsiaFloat

Copy link
Copy Markdown
Contributor

Fixes #6878

  • Return null when the request or response body is not a JSON object.

  • Validate every node in a nested field path before accessing it.

  • Prevent NullPointerException, ClassCastException, and IllegalStateException when a configured field is missing or has an unexpected JSON type.

  • Add unit tests covering non-object bodies and invalid nested paths.

  • You have read the contribution guidelines.

  • You submit test cases (unit or integration tests) that back your changes.

  • Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #6960 Review — fix: handle invalid JSON fields in cryptor plugin

Verdict: APPROVE

Scope

JsonUtil.parser(String json, String fieldName) in shenyu-plugin-cryptor. Adds null/type
safety when extracting a (possibly nested) field from a JSON body.

Verification

  • import java.util.Objects is already present (head file L18) — compile is fine.
  • Top-level guard: GsonUtils.getInstance().toObjectMap(json) returns null for non-object
    JSON (array "[...]", bare string, malformed). Now returns null instead of propagating.
  • Nested branch rewrite:
    • First segment cast to JsonObject is now guarded by instanceof (was an unchecked cast
      that could throw ClassCastException).
    • Every intermediate node is checked for null and isJsonObject() before descending.
    • Final node is checked for isJsonPrimitive() before getAsString() (was
      getAsJsonPrimitive() which threw on objects/arrays).
    • All early-exit paths return null consistently.
  • Non-nested else branch is unchanged: map.get(fieldName) == null ? null : .toString().
    Callers (e.g. the Set<String> overload in parser(json, fieldNames)) already filter out
    blank/null results, so returning null here is safe and matches prior missing-field behavior.
  • Logic traced against every new test case; all 8 negative cases resolve to null and both
    positive cases (name and data.nested.name) return the expected value.

Tests

JsonUtilTest added (49 lines): positive + non-object body + 6 invalid-nested-path cases.
Covers the exact exception paths this PR eliminates (NPE / CCE / IllegalStateException).

Notes

  • Behavioral change: a nested path whose final node is a JSON object/array now returns null
    instead of throwing. In the cryptor context null already means "no field to process", so
    this is the intended safe fallback, not a silent data-loss.
  • No public API/signature change; no new dependency.

Clean, minimal, well-tested null-safety fix. Approving.

@Aias00
Aias00 merged commit 2cd5799 into apache:master Aug 23, 2026
41 of 51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] RateLimiterPluginDataHandler leaks the old LettuceConnectionFactory when redis config is updated

2 participants