diff --git a/src/main/java/com/ibm/cldk/javaee/jakarta/JakartaEntrypointFinder.java b/src/main/java/com/ibm/cldk/javaee/jakarta/JakartaEntrypointFinder.java index f4faa44d..f29a69ca 100644 --- a/src/main/java/com/ibm/cldk/javaee/jakarta/JakartaEntrypointFinder.java +++ b/src/main/java/com/ibm/cldk/javaee/jakarta/JakartaEntrypointFinder.java @@ -5,8 +5,10 @@ import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; import com.github.javaparser.ast.body.Parameter; import com.github.javaparser.ast.body.TypeDeclaration; +import com.github.javaparser.ast.expr.AnnotationExpr; import com.github.javaparser.ast.type.ClassOrInterfaceType; import com.ibm.cldk.javaee.utils.interfaces.AbstractEntrypointFinder; +import java.util.Set; @SuppressWarnings({"unchecked", "rawtypes"}) public class JakartaEntrypointFinder extends AbstractEntrypointFinder { @@ -38,16 +40,42 @@ public boolean isEntrypointClass(TypeDeclaration typeDecl) { .anyMatch(n -> n.contains("HttpServlet") || n.contains("GenericServlet")) || classDecl.getImplementedTypes().stream().map( ClassOrInterfaceType::asString).anyMatch( - n -> n.contains("ServletContextListener") + n -> n.equals("Filter") || n.endsWith(".Filter") + || n.contains("ServletContextListener") || n.contains("HttpSessionListener") || n.contains("ServletRequestListener") || n.contains("MessageListener")); } + /** + * The servlet, filter, listener and WebSocket lifecycle methods the container itself invokes. + * Anything else on an entrypoint type is ordinary code, however servlet-typed its parameters. + */ + private static final Set LIFECYCLE = Set.of( + "service", "doGet", "doPost", "doPut", "doDelete", "doHead", "doOptions", "doTrace", + "doFilter", "init", "destroy", + "contextInitialized", "contextDestroyed", "sessionCreated", "sessionDestroyed", + "requestInitialized", "requestDestroyed", "onMessage"); + + private static final Set CALLBACK_ANNOTATIONS = + Set.of("OnMessage", "OnOpen", "OnClose", "OnError"); + + /** + * A container-invoked method of an entrypoint type: a lifecycle method by name, or a WebSocket + * callback by annotation (#263). Before this, any method with an {@code HttpServletRequest} / + * {@code HttpServletResponse} parameter was marked — which made a plain helper such as + * DayTrader's {@code requestDispatch(ctx, req, resp, page)} an entrypoint, and every + * interprocedural tier then refused to bind its parameters from the call sites that do bind + * them. The container binds a lifecycle method's parameters; it never calls a helper. + */ @Override public boolean isEntrypointMethod(CallableDeclaration callableDecl) { - return ((NodeList) callableDecl.getParameters()).stream() - .anyMatch(parameter -> parameter.getType().asString().contains("HttpServletRequest") || - parameter.getType().asString().contains("HttpServletResponse")); + boolean callback = ((NodeList) callableDecl.getAnnotations()).stream() + .anyMatch(a -> CALLBACK_ANNOTATIONS.contains(a.getNameAsString())); + if (!callback && !LIFECYCLE.contains(callableDecl.getNameAsString())) { + return false; + } + java.util.Optional owner = callableDecl.findAncestor(TypeDeclaration.class); + return owner.isPresent() && isEntrypointClass(owner.get()); } } diff --git a/src/test/java/com/ibm/cldk/artifacts/ViewDispatchDataflowTierTest.java b/src/test/java/com/ibm/cldk/artifacts/ViewDispatchDataflowTierTest.java index 0e4de2f5..c6660e94 100644 --- a/src/test/java/com/ibm/cldk/artifacts/ViewDispatchDataflowTierTest.java +++ b/src/test/java/com/ibm/cldk/artifacts/ViewDispatchDataflowTierTest.java @@ -110,13 +110,13 @@ void aTracedLiteralThatNamesNoFileIsNoSuchArtifact() throws Exception { assertEquals(List.of("literal", "dataflow"), u.getProv()); } - // The helper takes no servlet-typed parameter on purpose: the Jakarta finder marks any method - // with an HttpServletRequest parameter as an entrypoint, and the interprocedural tier rightly - // refuses to bind an entrypoint's parameter from a call site (the container binds it). + // The helper takes servlet-typed parameters like DayTrader's requestDispatch does: since #263 + // the Jakarta finder marks only lifecycle methods, so the interprocedural tier binds `page` + // from the call site instead of refusing it as container-bound. private static final String PARAMETER = HEAD - + " void doGet(HttpServletRequest req, HttpServletResponse res) { show(\"/y.jsp\"); }\n" - + " void show(String page) {\n" - + " getServletContext().getRequestDispatcher(page).forward(null, null);\n" + + " void doGet(HttpServletRequest req, HttpServletResponse res) { show(req, res, \"/y.jsp\"); }\n" + + " void show(HttpServletRequest req, HttpServletResponse res, String page) {\n" + + " req.getRequestDispatcher(page).forward(req, res);\n" + " }\n}\n"; @Test @@ -136,10 +136,10 @@ void aParameterClosesOverTheCallGraphAtLevelFour() throws Exception { @Test void twoCallersWithDifferentPagesStayNonLiteral() throws Exception { ViewDispatches.Result r = run(HEAD - + " void a() { show(\"/y.jsp\"); }\n" - + " void b() { show(\"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/pages/x.jsp\"); }\n" - + " void show(String page) {\n" - + " getServletContext().getRequestDispatcher(page).forward(null, null);\n" + + " void a(HttpServletRequest req, HttpServletResponse res) { show(req, res, \"/y.jsp\"); }\n" + + " void b(HttpServletRequest req, HttpServletResponse res) { show(req, res, \"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/pages/x.jsp\"); }\n" + + " void show(HttpServletRequest req, HttpServletResponse res, String page) {\n" + + " req.getRequestDispatcher(page).forward(req, res);\n" + " }\n}\n", 4); assertTrue(r.dispatches.isEmpty()); assertEquals("non-literal", only(r).getReason()); diff --git a/src/test/java/com/ibm/cldk/javaee/JakartaEntrypointMethodsTest.java b/src/test/java/com/ibm/cldk/javaee/JakartaEntrypointMethodsTest.java new file mode 100644 index 00000000..6dfeb3c9 --- /dev/null +++ b/src/test/java/com/ibm/cldk/javaee/JakartaEntrypointMethodsTest.java @@ -0,0 +1,104 @@ +package com.ibm.cldk.javaee; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.ibm.cldk.schema.JCallable; +import com.ibm.cldk.schema.JEntrypointReport; +import com.ibm.cldk.schema.JModule; +import com.ibm.cldk.schema.JType; +import com.ibm.cldk.syntactic_analysis.L1Extractor; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Which methods the Jakarta finder marks (#263): the servlet / filter / listener lifecycle methods + * of an entrypoint type, not any method that happens to take a servlet-typed parameter. A helper + * such as DayTrader's {@code requestDispatch(ctx, req, resp, page)} is ordinary code whose + * parameters are bound by its callers — marking it an entrypoint made every interprocedural tier + * refuse to bind them. + */ +class JakartaEntrypointMethodsTest { + + @TempDir + Path root; + + private Map> marks(String source) throws Exception { + Path src = root.resolve("src/main/java/demo"); + Files.createDirectories(src); + Files.writeString(src.resolve("Web.java"), source, StandardCharsets.UTF_8); + Map modules = L1Extractor.extractAll( + root, "app", null, new LinkedHashMap<>(), 1, 3, "ast", new JEntrypointReport()); + Map> out = new TreeMap<>(); + for (JModule m : modules.values()) { + for (Map.Entry e : m.getTypes().entrySet()) { + for (JCallable c : e.getValue().getCallables().values()) { + out.put(e.getKey() + "." + c.getSignature().substring(0, c.getSignature().indexOf('(')), + c.getEntrypointFrameworks()); + } + } + } + return out; + } + + @Test + void lifecycleMethodsOfAServletAreMarkedAndItsHelpersAreNot() throws Exception { + Map> m = marks( + "package demo;\n" + + "import javax.servlet.http.*;\n" + + "public class Front extends HttpServlet {\n" + + " protected void doGet(HttpServletRequest req, HttpServletResponse res) { show(req, res, \"/x.jsp\"); }\n" + + " public void init() {}\n" + + " void show(HttpServletRequest req, HttpServletResponse res, String page) {}\n" + + " static String render(HttpServletRequest req) { return \"\"; }\n" + + "}\n"); + assertEquals(List.of("jakarta"), m.get("Front.doGet")); + assertEquals(List.of("jakarta"), m.get("Front.init")); + assertEquals(List.of(), m.get("Front.show"), "a helper taking req/res is not an entrypoint"); + assertEquals(List.of(), m.get("Front.render")); + } + + @Test + void aLifecycleNameOutsideAnEntrypointTypeIsNotMarked() throws Exception { + Map> m = marks( + "package demo;\n" + + "import javax.servlet.http.*;\n" + + "public class Plain {\n" + + " void doGet(HttpServletRequest req, HttpServletResponse res) {}\n" + + " public void init() {}\n" + + "}\n"); + assertEquals(List.of(), m.get("Plain.doGet")); + assertEquals(List.of(), m.get("Plain.init")); + } + + @Test + void filterAndListenerAndEndpointCallbacksAreMarked() throws Exception { + Map> m = marks( + "package demo;\n" + + "import javax.servlet.*;\n" + + "@WebFilter(\"/*\")\n" + + "class Gate implements Filter {\n" + + " public void doFilter(ServletRequest q, ServletResponse s, FilterChain c) {}\n" + + " void log(ServletRequest q) {}\n" + + "}\n" + + "class Boot implements ServletContextListener {\n" + + " public void contextInitialized(ServletContextEvent e) {}\n" + + "}\n" + + "@ServerEndpoint(\"/ws\")\n" + + "class Sock {\n" + + " @OnMessage public void onText(String msg) {}\n" + + " void send(String msg) {}\n" + + "}\n"); + assertEquals(List.of("jakarta"), m.get("Gate.doFilter")); + assertEquals(List.of(), m.get("Gate.log")); + assertEquals(List.of("jakarta"), m.get("Boot.contextInitialized")); + assertEquals(List.of("jakarta"), m.get("Sock.onText")); + assertEquals(List.of(), m.get("Sock.send")); + } +}