Skip to content

Commit 1afea41

Browse files
authored
fix(artifacts): a non-identifier view-dispatch target is not a dataflow variable, not a NullPointerException (#271)
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.
1 parent 2c8deb1 commit 1afea41

3 files changed

Lines changed: 43 additions & 1 deletion

File tree

‎src/main/java/com/ibm/cldk/artifacts/DataflowTiers.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ private static void collect(Map<String, JType> types, String source, Map<String,
6565

6666
/** L3: the one literal every reaching definition of {@code var} closes on at {@code useLocalId}. */
6767
static String intra(Owner owner, String useLocalId, String var) {
68-
if (owner == null || owner.callable.getDdg() == null || owner.source == null) {
68+
if (owner == null || var == null || owner.callable.getDdg() == null
69+
|| owner.source == null) {
6970
return null;
7071
}
7172
return IntraTier.reachingLiteral(owner.callable, owner.source, useLocalId, var);

‎src/test/java/com/ibm/cldk/artifacts/ViewDispatchDataflowTierTest.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,21 @@ void aLocalClosesOverTheDdgAtLevelThree() throws Exception {
8383
assertTrue(r.unresolved.isEmpty());
8484
}
8585

86+
// A dispatcher target that is neither a literal nor a bare identifier (a call expression) used
87+
// to NPE in the dataflow tier: `Site.varName()` returns null for this shape too, and the tier
88+
// dereferenced it unguarded.
89+
@Test
90+
void aCallExpressionTargetStaysNonLiteralInsteadOfCrashing() throws Exception {
91+
ViewDispatches.Result r = run(HEAD
92+
+ " void doGet(HttpServletRequest req, HttpServletResponse res) {\n"
93+
+ " req.getRequestDispatcher(computePage()).forward(req, res);\n"
94+
+ " }\n"
95+
+ " String computePage() { return \"/pages/x.jsp\"; }\n"
96+
+ "}\n", 3);
97+
assertTrue(r.dispatches.isEmpty());
98+
assertEquals("non-literal", only(r).getReason());
99+
}
100+
86101
@Test
87102
void twoDisagreeingDefinitionsStayNonLiteral() throws Exception {
88103
ViewDispatches.Result r = run(HEAD

‎src/test/java/com/ibm/cldk/artifacts/ViewNameDispatchTest.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ private ViewDispatches.Result run(String controller, String properties, String..
3434
throws Exception {
3535
ServletApiStubs.write(root, "src/main/java/org/springframework/stereotype/Controller.java",
3636
"package org.springframework.stereotype;\npublic @interface Controller {}\n");
37+
ServletApiStubs.write(root,
38+
"src/main/java/org/springframework/web/bind/annotation/RestController.java",
39+
"package org.springframework.web.bind.annotation;\npublic @interface RestController {}\n");
3740
ServletApiStubs.write(root,
3841
"src/main/java/org/springframework/web/bind/annotation/GetMapping.java",
3942
"package org.springframework.web.bind.annotation;\n"
@@ -136,4 +139,27 @@ void aViewNameWithNoTemplateIsNoSuchArtifact() throws Exception {
136139
assertEquals("missing", r.unresolved.get(0).getTarget());
137140
assertEquals("view-name", r.unresolved.get(0).getVia());
138141
}
142+
143+
// Robot Shop's shipping service: a `@RestController`'s `return String.valueOf(x)` is
144+
// still gated into the view-name tier (the entrypoint check does not yet distinguish
145+
// `@RestController` from `@Controller`) and the target is neither a literal nor a bare
146+
// identifier, so `Site.varName()` is null. That null used to reach `DataflowTiers.intra`
147+
// unguarded and NPE in `IntraTier.reachingLiteral`; it must instead just stay unresolved.
148+
@Test
149+
void aNonIdentifierReturnExpressionStaysNonLiteralInsteadOfCrashing() throws Exception {
150+
ViewDispatches.Result r = run(
151+
"package demo;\n"
152+
+ "import org.springframework.web.bind.annotation.RestController;\n"
153+
+ "import org.springframework.web.bind.annotation.GetMapping;\n"
154+
+ "@RestController\npublic class Home {\n"
155+
+ " @GetMapping(\"/count\") public String count() {\n"
156+
+ " long n = 10;\n"
157+
+ " return String.valueOf(n);\n"
158+
+ " }\n}\n",
159+
null);
160+
assertTrue(r.dispatches.isEmpty());
161+
assertEquals(1, r.unresolved.size());
162+
assertEquals("non-literal", r.unresolved.get(0).getReason());
163+
assertEquals("view-name", r.unresolved.get(0).getVia());
164+
}
139165
}

0 commit comments

Comments
 (0)