Bug 65256

Summary: ServletContainerInitializer.onStartup() should receive only classes, not interfaces
Product: Tomcat 9 Reporter: Grzegorz Grzybek <gr.grzybek>
Component: ServletAssignee: Tomcat Developers Mailing List <dev>
Severity: normal CC: gr.grzybek
Priority: P2    
Version: 9.0.45   
Target Milestone: -----   
Hardware: PC   
OS: Linux   

Description Grzegorz Grzybek 2021-04-20 12:52:19 UTC
Extracting this issue from https://bz.apache.org/bugzilla/show_bug.cgi?id=65244

I've added java.util.EventListener.class to @HandlesTypes and I got ... a lot of stuff, including:

1 = {@3229} "interface javax.faces.validator.Validator"
3 = {@3230} "interface javax.faces.event.SystemEventListener"
5 = {@3232} "interface javax.faces.event.BehaviorListener"

I also got package-info inside the set.

So I got back to the spec:

> The HandlesTypes annotation on the implementation of the
> ServletContainerInitializer is used to express interest in classes that may
> have annotations (type, method or field level annotations) specified in the value of
> the HandlesTypes or if it extends / implements one those classes anywhere in
> the class’ super types.

I don't think I should get `interface javax.faces.validator.Validator` which exten ds `java.util.EventListener`, because it's not a class - it's an interface.

This may be more difficult to interpret, but if both classes and interfaces were needed, the spec would say "types" or "java.lang.Class" instances...

Javadoc says about:

> the Set of application classes that extend, implement, or have been annotated [...]

This time I checked that Jetty (9.4.40) also passes interfaces.
Comment 1 Remy Maucherat 2021-04-20 14:52:10 UTC
The fix will be in 10.0.6, 9.0.46 and 8.5.66, since the specification language is explicit enough (and there's likely no use case for interfaces).
Comment 2 Grzegorz Grzybek 2021-04-20 14:57:11 UTC
I'll remember to create Jetty issue tomorrow.
Comment 3 romain.manni-bucau 2021-04-20 17:16:21 UTC

I think it is a bug to not list interfaces.
Spec quote considers interfaces as "class" in its wording:

> extends / implements one those classes

Not returning interfaces breaks some serious number of use cases, typically all cases where annotations are used for scanning and are put on interfaces (often it is then implemented thanks a proxy, even for Servlet case ;)) and cases where annotations host the "spec" of the API and are read from the scanned set to deduce it (meta annotation API alike).

So overall I see that interface scanning is:

1. used
2. compaitble with current spec wording (even allowed IMHO)
3. supported by tomcat (before the last commit), jetty and undertow

So it looks legitimate to rather ask the spec to phrase it explicitly rather than breaking user with next release and then get the feature back later (I assume spec will explicit interfaces are enabled).
Comment 4 Grzegorz Grzybek 2021-04-20 17:23:49 UTC
(In reply to romain.manni-bucau from comment #3)
> Hi,
> I think it is a bug to not list interfaces.
> Spec quote considers interfaces as "class" in its wording:
> > extends / implements one those classes

I think you're right. It wasn't my intention to claim that current behavior should be changed. Following the research started in https://bz.apache.org/bugzilla/show_bug.cgi?id=65244 I found that specification may be confusing. But while 65244 was good catch, this one rather isn't.

Thinking more about it, even `package-info` should be valid among the classes passed to `SCI.onStartup()` (when there's annotation used on a package in `@HandlesTypes`).

So please excuse me for the confusion.
Comment 5 Remy Maucherat 2021-04-20 17:43:26 UTC
Ok, I will reread and revert it. This is confusing ...
Comment 6 Remy Maucherat 2021-04-20 18:51:33 UTC
So likely invalid due to the specification being rather unclear and the behavior being there for a long time.