jakartaee / servlet

Jakarta Servlet
https://eclipse.org/ee4j/servlet
Other
250 stars 79 forks source link

ServletSecTestServlet imports org.slf4j.Logger but test war doesn't include sl4j #638

Closed arjantijms closed 1 month ago

arjantijms commented 1 month ago

Source:

...
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@DeclareRoles({"Administrator"})
@ServletSecurity(value = @HttpConstraint(ServletSecurity.EmptyRoleSemantic.PERMIT), httpMethodConstraints = {@HttpMethodConstraint(value = "POST", rolesAllowed = {"Administrator"}, transportGuarantee = ServletSecurity.TransportGuarantee.CONFIDENTIAL), @HttpMethodConstraint(value = "GET", rolesAllowed = {"Administrator"}, transportGuarantee = ServletSecurity.TransportGuarantee.CONFIDENTIAL)})
@WebServlet({"/ServletSecTest"})
public class ServletSecTestServlet extends HttpServlet {
  private boolean fail = false;

  ...

War build by "ClientCertAnnoTests":

Screenshot 2024-05-22 at 17 51 33
arjantijms commented 1 month ago

Same problem for "servlet.tck.spec.servletresponse.servletResponseTests"

[INFO] Running servlet.tck.spec.servletresponse.servletResponseTests
...
servlet_spec_servletresponse_web.war:
/WEB-INF/
/WEB-INF/classes/
/WEB-INF/classes/servlet/
/WEB-INF/classes/servlet/tck/
/WEB-INF/classes/servlet/tck/spec/
/WEB-INF/classes/servlet/tck/spec/servletresponse/
/WEB-INF/classes/servlet/tck/spec/servletresponse/HttpTestServlet.class
/WEB-INF/classes/servlet/tck/spec/servletresponse/TestServlet.class
/WEB-INF/web.xml

Throws: java.lang.NoClassDefFoundError: org/slf4j/LoggerFactory on the server.

markt-asf commented 1 month ago

Looks like we are going to need a 6.1.1 TCK.

For servlet.tck.spec.security.clientcertanno it looks like we can just remove the logger since it isn't used.

For servlet.tck.spec.servletresponse I think we need to add the JAR.

I have changes for the above that I'm testing locally. If that works I can stage an updated TCK for you to test.

markt-asf commented 1 month ago

Hmm. Don't think the 6.1.0 TCK has been released yet. We might still have a chance to fix this for 6.1.0.

markt-asf commented 1 month ago

Fixed by #640

arjantijms commented 1 month ago

Hmm. Don't think the 6.1.0 TCK has been released yet. We might still have a chance to fix this for 6.1.0.

The ballot has already started, so unfortunately we can't change it for 6.1.0. It would have to be a 6.1.1 (service release). That's however relatively easy to do.

markt-asf commented 1 month ago

The change made it before the ballot started. It is in 6.1.0