Skip to content

fix: avoid NPE when mock rule omits httpStatusCode (#6881) - #6953

Open
juicewcode wants to merge 1 commit into
apache:masterfrom
juicewcode:fix/avoid-NPE-when-mock-rule-omits-httpStatusCode
Open

fix: avoid NPE when mock rule omits httpStatusCode (#6881)#6953
juicewcode wants to merge 1 commit into
apache:masterfrom
juicewcode:fix/avoid-NPE-when-mock-rule-omits-httpStatusCode

Conversation

@juicewcode

Copy link
Copy Markdown
Contributor

Fixes #6881

Changes

  • MockPlugin.java: guard httpStatusCode with
    Optional.ofNullable(...).orElse(200), so a missing/null status code
    defaults to 200 OK and the mock response is returned.
  • MockPluginTest.java: new unit test covering
    • null httpStatusCode → 200 with mock body (issue regression),
    • an explicitly specified status code (e.g. 404) being honored,
    • a missing cached handle falling through to the plugin chain.

Make sure that:

  • 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.

  MockHandle.httpStatusCode has no default, so when a mock rule's handle
  JSON omits or nulls it, Gson leaves the field null and MockPlugin passes
  it to HttpStatus.valueOf(int), whose auto-unboxing throws NPE and makes
  every matching request return 500.

  Fall back to 200 (OK) via Optional.ofNullable(...).orElse(200) in
  MockPlugin.doExecute, so the mock response is returned instead.

  Add MockPluginTest covering: null httpStatusCode -> 200 mock response,
  an explicitly specified status code being honored, and a missing cached
  handle falling through to the plugin chain.

@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.

Review: fix: avoid NPE when mock rule omits httpStatusCode (#6881)

Approved.

Verification

  • MockPlugin.doExecute now wraps mockHandle.getHttpStatusCode() with Optional.ofNullable(...).orElse(200), so a missing/null status code defaults to 200 OK instead of throwing HttpStatus.valueOf(null) NPE.
  • Confirmed MockHandle.getHttpStatusCode() returns Integer (nullable) — so the original HttpStatus.valueOf(...) would indeed NPE on an omitted field; the guard is correct and necessary.
  • Confirmed the new test references valid symbols: MockPluginHandler.CACHED_HANDLE (a Supplier<CommonHandleCache<...>>) with cachedHandle / removeHandle exist on the head branch.
  • New MockPluginTest covers three scenarios: null httpStatusCode → 200 with body, explicit 404 honored, and missing cached handle falls through to the chain.

Notes

  • Clean, well-tested defensive fix.

Thanks!

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] MockPlugin SpEL generators share a static EvaluationContext — cross-request data race on the req variable

2 participants