fix(artifacts): a non-identifier view-dispatch target is not a dataflow variable, not a NullPointerException - #271
Merged
Conversation
…ow variable, not a NullPointerException ViewDispatches.resolve() ran the intra-procedural dataflow tier on s.varName() unguarded, but Site.varName() returns null for any dispatch target that is not a bare identifier -- a call expression like `return String.valueOf(count);` inside a @RestController, as seen in Robot Shop's shipping service (Controller.count()). Spring entrypoint detection does not yet distinguish @RestController from @controller (SpringEntrypointFinder.isEntrypointClass matches on annotation.getNameAsString().contains("Controller")), so a REST endpoint's String return is still gated into the view-name tier. That null then reached DataflowTiers.intra and crashed in IntraTier.reachingLiteral's `var.equals(edge.getVar())` at analysis level >= 3. The very next tier down (interprocAll, L4) already guards the identical null with `s.varName() == null ? null : ...` -- the L3 call was just missing the equivalent check. Guarded `var == null` at the shared DataflowTiers.intra choke point rather than only at the call site, so any future caller is protected too. Added ViewNameDispatchTest.aNonIdentifierReturnExpressionStaysNonLiteralInsteadOfCrashing, reproducing the exact Robot Shop shape, and ViewDispatchDataflowTierTest.aCallExpressionTargetStaysNonLiteralInsteadOfCrashing for the analogous dispatcher-call-site shape; both reproduce the NPE on the old code and pass with the guard. Full suite: 606 tests, only the Docker-only integration test unrun (no local Docker daemon). Separately, @RestController return values are HTTP response bodies, not Spring view names -- conflating them with @controller in SpringEntrypointFinder is a real semantic gap, left untouched here since it changes detection scope rather than just fixing the crash.
Collaborator
Author
|
Follow-up filed: #272 (the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ViewDispatches.resolve()ran the intra-procedural dataflow tier ons.varName()unguarded, butSite.varName()returns null for any dispatch target that is not a bare identifier — a call expression likereturn String.valueOf(count);inside a@RestController, as seen in Robot Shop'sshippingservice (Controller.count()):Spring entrypoint detection doesn't yet distinguish
@RestControllerfrom@Controller(SpringEntrypointFinder.isEntrypointClassmatches onannotation.getNameAsString().contains("Controller")), so a REST endpoint'sStringreturn is still gated into the view-name tier. The resulting nullvarreachedDataflowTiers.intraand crashed inIntraTier.reachingLiteral'svar.equals(edge.getVar())at analysis level >= 3:The very next tier down (
interprocAll, L4) already guards the identical null withs.varName() == null ? null : ...— the L3 call was just missing the equivalent check. Reproduced directly against the public Robot Shop source at-a 4before fixing.Fix
Guarded
var == nullat the sharedDataflowTiers.intrachoke point rather than only at the call site, so any future caller is protected too:Tests
ViewNameDispatchTest.aNonIdentifierReturnExpressionStaysNonLiteralInsteadOfCrashingreproduces the exact Robot Shop shape (a@RestController'sreturn String.valueOf(n);).ViewDispatchDataflowTierTest.aCallExpressionTargetStaysNonLiteralInsteadOfCrashingcovers the analogous dispatcher-call-site shape (req.getRequestDispatcher(computePage()).forward(...)).Both reproduce the NPE on the pre-fix code and pass with the guard. Full suite: 606 tests, only the Docker-only integration test unrun (no local Docker daemon in this environment).
Out of scope
@RestControllerreturn values are HTTP response bodies, not Spring view names — conflating them with@ControllerinSpringEntrypointFinderis a real semantic gap (it also affects@Controllermethods annotated@ResponseBody). Left untouched here since it changes detection scope rather than just fixing the crash; happy to follow up in a separate PR if wanted.