fix: Scan for WebServlet annotation #125
Conversation
| { | ||
| HttpClient client = HttpClient.newBuilder().version(HttpClient.Version.HTTP_1_1).build(); | ||
| HttpRequest request = HttpRequest.newBuilder() | ||
| var client = HttpClient.newBuilder().version(HttpClient.Version.HTTP_1_1).build(); |
There was a problem hiding this comment.
No, keep the variable, don't fall back to var.
The use of var makes for bad and hard to follow examples. (the whole point of this repo)
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee | ||
| http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd" | ||
| version="3.1"> |
There was a problem hiding this comment.
Don't delete this, upgrade it to ee6 instead.
| @@ -51,6 +49,7 @@ public static Server newServer(int port) | |||
| context.setBaseResource(baseResource); | |||
| context.setContextPath("/"); | |||
| context.setWelcomeFiles(new String[]{"index.html", "welcome.html"}); | |||
| context.setAttribute("org.eclipse.jetty.server.webapp.ContainerIncludeJarPattern", ".*/"); | |||
There was a problem hiding this comment.
Set the pattern properly, don't include 100% of the container.
As on Maven, this even includes all of the maven deps, and the maven plugins too.
In fact, the default pattern in ee10 should work out of the box.
There was a problem hiding this comment.
What should be the pattern? When I removed this line (using default?), test was failing. I changed to .*/classes/* now.
There was a problem hiding this comment.
There are two patterns that impact how the webapp scanning operates.
- The Container level (
org.eclipse.jetty.server.webapp.ContainerIncludeJarPattern) which scans for things that are not part of the webapp.
By default, it will not scan the container, unless this pattern is defined. - The WebApp level (
org.eclipse.jetty.server.webapp.WebInfIncludeJarPattern) which scans for things that belong specifically to the webapp.
By default, this will scan all artifacts on the webapp (likeWEB-INF/lib/andclasses/) unless the pattern is defined.
For the container, you might need to support servlet, and jsp, and taglib.
For servlet only ...
webappContext.setAttribute(
"org.eclipse.jetty.server.webapp.ContainerIncludeJarPattern",
".*/jakarta.servlet-api-[^/]*\\.jar$");For servlet, with JSP, and even Taglib support ...
webappContext.setAttribute(
"org.eclipse.jetty.server.webapp.ContainerIncludeJarPattern",
".*/jakarta.servlet-api-[^/]*\\.jar$|.*/jakarta.servlet.jsp.jstl-.*\\.jar$|.*/[^/]*taglibs.*\\.jar$");Add more jar patterns for other jars that you might need from the container (which shouldn't be much).
Point is, don't open everything on the container for scanning, that's just wrong way to use the pattern, wrong for performance as you scan everything, wrong for security as you never want to open everything to the webapp, etc ...
There was a problem hiding this comment.
The overwhelming majority of embedded use cases never scan, as that's an anti-pattern for embedded use cases (which focus on absolute control, and startup performance).
They instead define the webapp configuration entirely in code for quick startup.
This is the reason why those patterns are the way they are by default.
|
There are build errors with this latest code push. Run |
|
I applied Spotless, but when it comes to |
Following this example I couldn't make
@WebServletannotation work. I reproduced the issue with test and fixed it following:Also, seems
web.xmlis not used at all (it points to Servlets 3.1)and there were some minor issues, like missing@OverrideI fixed.