if metadata-complete = true servlet security annotations are still parsed

[olamy]

Specs says If “metadata-complete” is set to "true", the deployment tool MUST ignore any servlet annotations present in the class files of the application and web fragments .
Even ServletSecurityAnnotationHandler javadoc says

 * The ServletSecurity annotation for a servlet should only be processed
 * if metadata-complete == false.

[janbartel]

@olamy see the Servlet Specification section 15.5.1:

The web application deployment descriptor contains a metadata-complete attribute
on the web-app element. The metadata-complete attribute defines whether the
web.xml descriptor is complete, or whether other sources of metadata used by the
deployment process should be considered. Metadata may come from the web.xml
file, web-fragment.xml files, annotations on class files in WEB-INF/classes , and
annotations on classes in jar files in the WEB-INF/lib directory. If metadata-
complete is set to " true ", the deployment tool only examines the web.xml file and
must ignore annotations such as @WebServlet , @webfilter , and @weblistener
present in the class files of the application, and must also ignore any web-
fragment.xml descriptor packaged in a jar file in WEB-INF/lib . If the metadata-
complete attribute is not specified or is set to " false ", the deployment tool must
examine the class files and web-fragment.xml files for metadata,as previously
specified.
The web-fragment.xml also contains the metadata-complete attribute on the web-
fragment element. The attribute defines whether the web-fragment.xml descriptor
is complete for the given fragment, or whether it should scan for annotations in the
classes in the associated jar file. If metadata-complete is set to “ true ” the
deployment tool only examines the web-fragment.xml and must ignore annotations
such as @WebServlet , @webfilter and @weblistener present in the class files of the
fragment. If metadata-complete is not specified or is set to “ false ” the deployment
tool must examine the class files for metadata.

The servlet spec defines "discoverable" annotations and "introspectable" annotations. Discoverable ones are those that are actively scanned-for by the container iff metadata-complete is false. The introspectable ones are those that must be looked at on a class when the class is going in to service, whether or not metadata-complete is true. I recall very long email threads about this in the servlet spec group back in servlet 3.0. AFAIK the agreed conclusion is that if metadata-complete == true, WebServlet, WebFilter and WebListener will not be discovered. But all other annotations that can be put on a class must be honoured: both for servlets/filters/listeners defined in web.xml/web-fragment.xml or added programmatically.

@gregw is that your understanding?

[olamy]

reference in TCK https://github.com/eclipse-ee4j/jakartaee-tck/blob/7ed17e520644b68f567009cdf9cffccbd2c92b14/src/com/sun/ts/tests/servlet/spec/security/metadatacomplete/UnProtectedTestServlet.java#L31

even our javadoc says to not parse in this case :) https://github.com/eclipse/jetty.project/blob/add8ffca5b8e7ebcb81ffff35b573f42ec52e217/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/ServletSecurityAnnotationHandler.java#L53

[olamy]

Servlet specs says (so I'm lost :) )

8.1 Annotations and pluggability
....
The web application deployment descriptor contains a new “metadata-complete”
attribute on the web-app element. The “metadata-complete” attribute defines
whether the web descriptor is complete, or whether the class files of the jar file
should be examined for annotations and web fragments at deployment time. If
“metadata-complete” is set to "true", the deployment tool MUST ignore any
servlet annotations present in the class files of the application and web fragments. If
the metadata-complete attribute is not specified or is set to "false", the
deployment tool must examine the class files of the application for annotations, and
scan for web fragments.
....

[janbartel]

@olamy yes, it is an extremely poorly defined and sometimes contradictory specification.

When considering how to handle annotations that are not @WebListener, @WebServlet,@WebFilter. bear in mind that programmatically added servlets, filters, listeners etc must have their annotations obeyed, regardless of metadata-complete. This gets very tricky in the case where eg the servlet is "preliminarily" defined in web.xml as just a name but with no class. A "preliminary" servlet declaration can always be completed by calling some of the programmatic configuration methods on the ServletContext - essentially defining the class for the servlet. Now, what should happen when a servlet is preliminarily defined in web.xml (ie it has a name only), but metadata-complete is true? Jetty remembers the source of the servlet declaration because different logic obviously applies - however, if the original source is web.xml, but then completed programmatically, does that make it a web.xml source or a programmatic source? If a web.xml source, and metadata-complete is true, shouldn't the whole servlet be declared in web.xml?

As for the comment in ServletSecurityAnnotationHandler - the implementation was trying to keep pace with the on-the-fly decisions of the servlet spec group, so I think that one is out-of-date, before the spec group decided that security annotations would have to be introspected and not discovered.

All of which is not to say that we shouldn't change our implementation, but the question is - to what??!

[olamy]

I will open a case in the TCK because our implementation currently break a TCK test.
But with 15.5.1 spec part I understand if metadata-complete is true all annotations must be ignored (but it's my non native english reading of it :) )

[olamy]

but reading https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf
section 8.1

If the value of the “metadata-complete” attribute is specified as true, the
deployment tool must ignore any annotations that specify such deployment
information in the class files packaged in the web application.

[janbartel]

@olamy more interesting is this part of the spec, about Security annotations:

When metadata-complete=true is defined for a portable deployment descriptor,
the @ServletSecurity annotation does not apply to any of the url-patterns
mapped to (any servlet mapped to) the annotated class in the deployment
descriptor.
The @ServletSecurity annotation is not applied to the url-patterns of a
ServletRegistration created using the addServlet(String, Servlet)
method of the ServletContext interface, unless the Servlet was constructed by
the createServlet method of the ServletContext interface.
With the exceptions listed above, when a Servlet class is annotated with
@ServletSecurity, the annotation defines the security constraints that apply to all
the url-patterns mapped to all the Servlets mapped to the class.
When a class has not been annotated with the @ServletSecurity annotation, the
access policy that is applied to a servlet mapped from that class is established by the
applicable security-constraint elements, if any, in the corresponding portable
deployment descriptor, or barring any such elements, by the constraints, if any,
established programmatically for the target servlet via the setServletSecurity
method of the ServletRegistration interface.

Let me digest that prose and get back to you.

[janbartel]

Well ... having read and re-read the relevant section (13.4.1) I'm not sure it's any clearer :(

So here's my understandings, but check with the servlet group experts/tck maintainers etc:

In all cases, metadata-complete==true:

• Servlet fully defined in web.xml -> ignore @ServletSecurity annotation.
• Servlet partially defined in web.xml and completed programmatically using ServletContext.createServlet -> introspect @ServletSecurity, but it only applies to programmatically defined url patterns, NOT those from web.xml
• Servlet partially defined in web.xml and completed programmatically NOT using ServletContext.createServlet -> ignore @ServletSecurity annotation.
• Servlet defined by @WebServlet annotation -> ignore @ServletSecurity annotation

[olamy]

done