Re: CDI and injection issues

[Rémy Maucherat]

On Wed, Nov 23, 2022 at 11:10 AM Romain Manni-Bucau
 wrote:
>
> Well, it is not that simple.
>
> Two notes on that:
>
> 1. One point is the API, injector and instance manager are the exact same
> API if you want a generic API so not sure it should be duplicated with
> different names (or said otherwise the Injector API is not generic enough
> to be worth a tomcat "core" change IMHO)

This is indeed the same API.
Tomcat standalone handles Jakarta Annotations here:
https://github.com/apache/tomcat/blob/main/java/org/apache/catalina/core/DefaultInstanceManager.java#L174
The owb integration is supposed to do the CDI annotations on the same
spot. AFAIK, it works. Except that the PostConstruct will be done
later.
I don't want to duplicate DefaultInstanceManager since there are a lot
of privileged actions and code in there. So adding the callback on
line 174 does seem fine to me.

> 2. the post construct is a conflict between tomcat world and cdi world
> there but you have the same "conflict" (= concurrent world/handling) for
> injections, i.e. tomcat handles persistence context, persistence unit,
> webservice ref etc but CDI can also handle it so the injector is either
> incomplete in one case (cdi without these handling) and should be combined
> between tomcat and injector OR it is done twice, potentially differently.
>
> So at the end it looks like you don't have the choice to just own the
> instance manager to ensure it is done properly so making it easier to
> instantiate is likely the way to go but adding an abstraction which is not
> generic is maybe not helping as much as it can look like upfront.
>
> Side note: meecrowave does a more cdi instantiation:
> https://github.com/apache/openwebbeans-meecrowave/blob/master/meecrowave-core/src/main/java/org/apache/meecrowave/tomcat/CDIInstanceManager.java
> and it is very few lines so not sure the injector would help much more for
> a tomcat-cdi integration.

Sure, but the problem is that I have to keep DefaultInstanceManager
around, and I won't replace everything with a CDI implementation.

Rémy

>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github  |
> LinkedIn  | Book
> 
>
>
> Le mer. 23 nov. 2022 à 10:56, Rémy Maucherat  a écrit :
>
> > On Tue, Nov 22, 2022 at 6:08 PM Romain Manni-Bucau
> >  wrote:
> > >
> > > Hmm, how is your injector different from an InstanceManager? (
> > >
> > https://github.com/apache/tomcat/blob/main/java/org/apache/tomcat/InstanceManager.java
> > > )
> > > Only by not having all `newInstance` flavors?
> >
> > The plan is to keep on using DefaultInstanceManager since OWB only
> > needs a callback for injection (before PostConstruct) and destroy (to
> > remove the instance from the map). Otherwise the whole InstanceManager
> > has to be replaced (and done better than the current one since the
> > PostConstruct calls cannot be in the right order otherwise).
> >
> > > Will also need the backgroundProcess (cache cleanup, same as instance
> > > manager) and similarly the calling context (new instance params), in
> > > particular the classloader.
> > > This is why I said it sounds more like a single API and more hooking the
> > > default instance to enable what you want or just redo it is likely
> > simpler
> > > _on an API standpoint_.
> > > Guess postConstruct() method as in TomEE impl - but using tomcat in place
> > > method, just exposing it - can be sufficient and keep the API clean if
> > you
> > > don't want to reimpl anything.
> >
> > This is only a callback for injection with DefaultInstanceManager.
> > Implementing your own InstanceManager in TomEE is still needed and
> > since it is not using DefaultInstanceManager this callback API is not
> > available at all. Also it will not break existing InstanceManager
> > implementations (even ones that extend DefaultInstanceManager), so
> > that's another thing I had to consider.
> >
> > The user who got me into this is here:
> > https://lists.apache.org/thread/1g4bpj3jnmmmxkh37qvwzoplqo65ldot
> > This is about Weld, but after having a look the
> > OpenWebBeansInstanceManager does the same thing. Then I realized
> > unless the InstanceManager was fully replaced there would be no way to
> > implement the proper ordering.
> >
> > > About JSP hack, it is more general but hits mainly JSP: it is about
> > tomcat
> > > specific JNDI injections, the workaround and wiring used elsewhere for
> > > beans didn't work properly for JSP. Guess you don't have this issue but
> > > something making it easier to handle can also probably be welcomed by
> > > consumers.
> >
> > Ok.
> >
> > Rémy
> >
> > >
> > >
> > > Le mar. 22 nov. 2022 à 17:26, Rémy Maucherat  a écrit :
> > >
> > > > On Tue, Nov 22, 2022 at 4:30 PM Romain Manni-Bucau
> > > >

Handling reports from oss-fuzz

[Mark Thomas]

Hi all,

We currently receive reports from oss-fuzz to the Tomcat security list. 
There is a relatively high volume of reports with a very high false 
positive rate. To date, we haven't had any valid security issues reported.


Concern has been expressed that oss-fuzz is generating excessive noise 
on the security list.


I'd like to propose the following solution, recently adopted by Apache 
Commons.


1. Create a new, private mailing list: fuzz-testing@tomcat.a.o

2. This new list becomes the primary contact for oss-fuzz issues.

3. security@tomact.a.o remains on the CC but we disable notifications
   unless the issue is explicitly starred

The new process would then be:

- issues reported to fuzz-testing@tomact.a.o
- interested PMC members subscribe to that list
- we triage issues (depending on volume this could become an issue)
  - false positives are rejected
  - bugs are fixed
  - security issues are starred
this triggers notification of issue updates to the security list
- security issues are handled as per the usual process

Thoughts?

Mark

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: CDI and injection issues

[Romain Manni-Bucau]

Le jeu. 24 nov. 2022 à 10:13, Rémy Maucherat  a écrit :

> On Wed, Nov 23, 2022 at 11:10 AM Romain Manni-Bucau
>  wrote:
> >
> > Well, it is not that simple.
> >
> > Two notes on that:
> >
> > 1. One point is the API, injector and instance manager are the exact same
> > API if you want a generic API so not sure it should be duplicated with
> > different names (or said otherwise the Injector API is not generic enough
> > to be worth a tomcat "core" change IMHO)
>
> This is indeed the same API.
> Tomcat standalone handles Jakarta Annotations here:
>
> https://github.com/apache/tomcat/blob/main/java/org/apache/catalina/core/DefaultInstanceManager.java#L174
> The owb integration is supposed to do the CDI annotations on the same
> spot. AFAIK, it works. Except that the PostConstruct will be done
> later.
> I don't want to duplicate DefaultInstanceManager since there are a lot
> of privileged actions and code in there. So adding the callback on
> line 174 does seem fine to me.
>
> > 2. the post construct is a conflict between tomcat world and cdi world
> > there but you have the same "conflict" (= concurrent world/handling) for
> > injections, i.e. tomcat handles persistence context, persistence unit,
> > webservice ref etc but CDI can also handle it so the injector is either
> > incomplete in one case (cdi without these handling) and should be
> combined
> > between tomcat and injector OR it is done twice, potentially differently.
> >
> > So at the end it looks like you don't have the choice to just own the
> > instance manager to ensure it is done properly so making it easier to
> > instantiate is likely the way to go but adding an abstraction which is
> not
> > generic is maybe not helping as much as it can look like upfront.
> >
> > Side note: meecrowave does a more cdi instantiation:
> >
> https://github.com/apache/openwebbeans-meecrowave/blob/master/meecrowave-core/src/main/java/org/apache/meecrowave/tomcat/CDIInstanceManager.java
> > and it is very few lines so not sure the injector would help much more
> for
> > a tomcat-cdi integration.
>
> Sure, but the problem is that I have to keep DefaultInstanceManager
> around, and I won't replace everything with a CDI implementation.
>

Well, in the owb module you will have to - previous impl is more about
letting cdi handling the postconstruct but lack the JNDI support to be
complete - to solve the double/conflict injection point anyway. The
*Injector* API does not solve it so not sure there is much space for this
invalid by design API :s. Did you find a way to solve it?


>
> Rémy
>
> >
> > Romain Manni-Bucau
> > @rmannibucau  |  Blog
> >  | Old Blog
> >  | Github <
> https://github.com/rmannibucau> |
> > LinkedIn  | Book
> > <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >
> >
> > Le mer. 23 nov. 2022 à 10:56, Rémy Maucherat  a écrit :
> >
> > > On Tue, Nov 22, 2022 at 6:08 PM Romain Manni-Bucau
> > >  wrote:
> > > >
> > > > Hmm, how is your injector different from an InstanceManager? (
> > > >
> > >
> https://github.com/apache/tomcat/blob/main/java/org/apache/tomcat/InstanceManager.java
> > > > )
> > > > Only by not having all `newInstance` flavors?
> > >
> > > The plan is to keep on using DefaultInstanceManager since OWB only
> > > needs a callback for injection (before PostConstruct) and destroy (to
> > > remove the instance from the map). Otherwise the whole InstanceManager
> > > has to be replaced (and done better than the current one since the
> > > PostConstruct calls cannot be in the right order otherwise).
> > >
> > > > Will also need the backgroundProcess (cache cleanup, same as instance
> > > > manager) and similarly the calling context (new instance params), in
> > > > particular the classloader.
> > > > This is why I said it sounds more like a single API and more hooking
> the
> > > > default instance to enable what you want or just redo it is likely
> > > simpler
> > > > _on an API standpoint_.
> > > > Guess postConstruct() method as in TomEE impl - but using tomcat in
> place
> > > > method, just exposing it - can be sufficient and keep the API clean
> if
> > > you
> > > > don't want to reimpl anything.
> > >
> > > This is only a callback for injection with DefaultInstanceManager.
> > > Implementing your own InstanceManager in TomEE is still needed and
> > > since it is not using DefaultInstanceManager this callback API is not
> > > available at all. Also it will not break existing InstanceManager
> > > implementations (even ones that extend DefaultInstanceManager), so
> > > that's another thing I had to consider.
> > >
> > > The user who got me into this is here:
> > > https://lists.apache.org/thread/1g4bpj3jnmmmxkh37qvwzoplqo65ldot
> > > This is about Weld, but after having a look the
> > > OpenWebBeansInstanceManager does the same thing. Then I realized
> > > unle

Re: Handling reports from oss-fuzz

[Rémy Maucherat]

On Thu, Nov 24, 2022 at 10:14 AM Mark Thomas  wrote:
>
> Hi all,
>
> We currently receive reports from oss-fuzz to the Tomcat security list.
> There is a relatively high volume of reports with a very high false
> positive rate. To date, we haven't had any valid security issues reported.
>
> Concern has been expressed that oss-fuzz is generating excessive noise
> on the security list.
>
> I'd like to propose the following solution, recently adopted by Apache
> Commons.
>
> 1. Create a new, private mailing list: fuzz-testing@tomcat.a.o
>
> 2. This new list becomes the primary contact for oss-fuzz issues.
>
> 3. security@tomact.a.o remains on the CC but we disable notifications
> unless the issue is explicitly starred
>
> The new process would then be:
>
> - issues reported to fuzz-testing@tomact.a.o
> - interested PMC members subscribe to that list
> - we triage issues (depending on volume this could become an issue)
>- false positives are rejected
>- bugs are fixed
>- security issues are starred
>  this triggers notification of issue updates to the security list
> - security issues are handled as per the usual process
>
> Thoughts?

+1

Rémy

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

svn commit: r58227 - in /release/tomcat: jakartaee-migration/v1.0.4/ tomcat-10/v10.1.1/ tomcat-8/v8.5.83/ tomcat-9/v9.0.68/

[markt]

Author: markt
Date: Thu Nov 24 11:46:33 2022
New Revision: 58227

Log:
Drop old versions from CDN

Removed:
release/tomcat/jakartaee-migration/v1.0.4/
release/tomcat/tomcat-10/v10.1.1/
release/tomcat/tomcat-8/v8.5.83/
release/tomcat/tomcat-9/v9.0.68/


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: Handling reports from oss-fuzz

[jean-frederic clere]

On 11/24/22 10:13, Mark Thomas wrote:

Hi all,

We currently receive reports from oss-fuzz to the Tomcat security list. 
There is a relatively high volume of reports with a very high false 
positive rate. To date, we haven't had any valid security issues reported.


Concern has been expressed that oss-fuzz is generating excessive noise 
on the security list.


I'd like to propose the following solution, recently adopted by Apache 
Commons.


1. Create a new, private mailing list: fuzz-testing@tomcat.a.o

2. This new list becomes the primary contact for oss-fuzz issues.

3. security@tomact.a.o remains on the CC but we disable notifications
    unless the issue is explicitly starred

The new process would then be:

- issues reported to fuzz-testing@tomact.a.o
- interested PMC members subscribe to that list
- we triage issues (depending on volume this could become an issue)
   - false positives are rejected
   - bugs are fixed
   - security issues are starred
     this triggers notification of issue updates to the security list
- security issues are handled as per the usual process

Thoughts?


+1



Mark

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



--
Cheers

Jean-Frederic


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: CDI and injection issues

[Rémy Maucherat]

On Thu, Nov 24, 2022 at 10:19 AM Romain Manni-Bucau
 wrote:
>
> Le jeu. 24 nov. 2022 à 10:13, Rémy Maucherat  a écrit :
>
> > On Wed, Nov 23, 2022 at 11:10 AM Romain Manni-Bucau
> >  wrote:
> > >
> > > Well, it is not that simple.
> > >
> > > Two notes on that:
> > >
> > > 1. One point is the API, injector and instance manager are the exact same
> > > API if you want a generic API so not sure it should be duplicated with
> > > different names (or said otherwise the Injector API is not generic enough
> > > to be worth a tomcat "core" change IMHO)
> >
> > This is indeed the same API.
> > Tomcat standalone handles Jakarta Annotations here:
> >
> > https://github.com/apache/tomcat/blob/main/java/org/apache/catalina/core/DefaultInstanceManager.java#L174
> > The owb integration is supposed to do the CDI annotations on the same
> > spot. AFAIK, it works. Except that the PostConstruct will be done
> > later.
> > I don't want to duplicate DefaultInstanceManager since there are a lot
> > of privileged actions and code in there. So adding the callback on
> > line 174 does seem fine to me.
> >
> > > 2. the post construct is a conflict between tomcat world and cdi world
> > > there but you have the same "conflict" (= concurrent world/handling) for
> > > injections, i.e. tomcat handles persistence context, persistence unit,
> > > webservice ref etc but CDI can also handle it so the injector is either
> > > incomplete in one case (cdi without these handling) and should be
> > combined
> > > between tomcat and injector OR it is done twice, potentially differently.
> > >
> > > So at the end it looks like you don't have the choice to just own the
> > > instance manager to ensure it is done properly so making it easier to
> > > instantiate is likely the way to go but adding an abstraction which is
> > not
> > > generic is maybe not helping as much as it can look like upfront.
> > >
> > > Side note: meecrowave does a more cdi instantiation:
> > >
> > https://github.com/apache/openwebbeans-meecrowave/blob/master/meecrowave-core/src/main/java/org/apache/meecrowave/tomcat/CDIInstanceManager.java
> > > and it is very few lines so not sure the injector would help much more
> > for
> > > a tomcat-cdi integration.
> >
> > Sure, but the problem is that I have to keep DefaultInstanceManager
> > around, and I won't replace everything with a CDI implementation.
> >
>
> Well, in the owb module you will have to - previous impl is more about
> letting cdi handling the postconstruct but lack the JNDI support to be
> complete - to solve the double/conflict injection point anyway. The
> *Injector* API does not solve it so not sure there is much space for this
> invalid by design API :s. Did you find a way to solve it?

I don't understand everything in this area.
I tried a Servlet with a @PostConstruct. Tomcat's
DefaultInstanceManager invokes that method.
If OWB is active through the listener on that webapp, and processes
that Servlet instance for injection (there's nothing to inject of
course), then it does not invoke that @PostConstruct. Why is that ?
Does it only do it on beans where it actually did some injection ?

Rémy

>
>
> >
> > Rémy
> >
> > >
> > > Romain Manni-Bucau
> > > @rmannibucau  |  Blog
> > >  | Old Blog
> > >  | Github <
> > https://github.com/rmannibucau> |
> > > LinkedIn  | Book
> > > <
> > https://www.packtpub.com/application-development/java-ee-8-high-performance
> > >
> > >
> > >
> > > Le mer. 23 nov. 2022 à 10:56, Rémy Maucherat  a écrit :
> > >
> > > > On Tue, Nov 22, 2022 at 6:08 PM Romain Manni-Bucau
> > > >  wrote:
> > > > >
> > > > > Hmm, how is your injector different from an InstanceManager? (
> > > > >
> > > >
> > https://github.com/apache/tomcat/blob/main/java/org/apache/tomcat/InstanceManager.java
> > > > > )
> > > > > Only by not having all `newInstance` flavors?
> > > >
> > > > The plan is to keep on using DefaultInstanceManager since OWB only
> > > > needs a callback for injection (before PostConstruct) and destroy (to
> > > > remove the instance from the map). Otherwise the whole InstanceManager
> > > > has to be replaced (and done better than the current one since the
> > > > PostConstruct calls cannot be in the right order otherwise).
> > > >
> > > > > Will also need the backgroundProcess (cache cleanup, same as instance
> > > > > manager) and similarly the calling context (new instance params), in
> > > > > particular the classloader.
> > > > > This is why I said it sounds more like a single API and more hooking
> > the
> > > > > default instance to enable what you want or just redo it is likely
> > > > simpler
> > > > > _on an API standpoint_.
> > > > > Guess postConstruct() method as in TomEE impl - but using tomcat in
> > place
> > > > > method, just exposing it - can be sufficient and keep the API clean
> > if
> > > > you
> > > > > don't want to reimpl anyth

Re: CDI and injection issues

[Romain Manni-Bucau]

Le jeu. 24 nov. 2022 à 16:58, Rémy Maucherat  a écrit :

> On Thu, Nov 24, 2022 at 10:19 AM Romain Manni-Bucau
>  wrote:
> >
> > Le jeu. 24 nov. 2022 à 10:13, Rémy Maucherat  a écrit :
> >
> > > On Wed, Nov 23, 2022 at 11:10 AM Romain Manni-Bucau
> > >  wrote:
> > > >
> > > > Well, it is not that simple.
> > > >
> > > > Two notes on that:
> > > >
> > > > 1. One point is the API, injector and instance manager are the exact
> same
> > > > API if you want a generic API so not sure it should be duplicated
> with
> > > > different names (or said otherwise the Injector API is not generic
> enough
> > > > to be worth a tomcat "core" change IMHO)
> > >
> > > This is indeed the same API.
> > > Tomcat standalone handles Jakarta Annotations here:
> > >
> > >
> https://github.com/apache/tomcat/blob/main/java/org/apache/catalina/core/DefaultInstanceManager.java#L174
> > > The owb integration is supposed to do the CDI annotations on the same
> > > spot. AFAIK, it works. Except that the PostConstruct will be done
> > > later.
> > > I don't want to duplicate DefaultInstanceManager since there are a lot
> > > of privileged actions and code in there. So adding the callback on
> > > line 174 does seem fine to me.
> > >
> > > > 2. the post construct is a conflict between tomcat world and cdi
> world
> > > > there but you have the same "conflict" (= concurrent world/handling)
> for
> > > > injections, i.e. tomcat handles persistence context, persistence
> unit,
> > > > webservice ref etc but CDI can also handle it so the injector is
> either
> > > > incomplete in one case (cdi without these handling) and should be
> > > combined
> > > > between tomcat and injector OR it is done twice, potentially
> differently.
> > > >
> > > > So at the end it looks like you don't have the choice to just own the
> > > > instance manager to ensure it is done properly so making it easier to
> > > > instantiate is likely the way to go but adding an abstraction which
> is
> > > not
> > > > generic is maybe not helping as much as it can look like upfront.
> > > >
> > > > Side note: meecrowave does a more cdi instantiation:
> > > >
> > >
> https://github.com/apache/openwebbeans-meecrowave/blob/master/meecrowave-core/src/main/java/org/apache/meecrowave/tomcat/CDIInstanceManager.java
> > > > and it is very few lines so not sure the injector would help much
> more
> > > for
> > > > a tomcat-cdi integration.
> > >
> > > Sure, but the problem is that I have to keep DefaultInstanceManager
> > > around, and I won't replace everything with a CDI implementation.
> > >
> >
> > Well, in the owb module you will have to - previous impl is more about
> > letting cdi handling the postconstruct but lack the JNDI support to be
> > complete - to solve the double/conflict injection point anyway. The
> > *Injector* API does not solve it so not sure there is much space for this
> > invalid by design API :s. Did you find a way to solve it?
>
> I don't understand everything in this area.
> I tried a Servlet with a @PostConstruct. Tomcat's
> DefaultInstanceManager invokes that method.
> If OWB is active through the listener on that webapp, and processes
> that Servlet instance for injection (there's nothing to inject of
> course), then it does not invoke that @PostConstruct. Why is that ?
> Does it only do it on beans where it actually did some injection ?
>

If you speak of the tomcat/module impl it is because it uses owb.inject
only and not the unmanaged API (or injectiontarget which is equivalent).
The issues start when you have such a bean (whatever component it is, a
servlet, filter, listener):

public class FooComponent ... {
@Inject MyBean service;
@Resource DataSource dataSource; // from web.xml for ex
@Resource(lookup = "entries/conf)" String conf; // from context.xml for
ex
@PostConstruct void onInit() {}
@PreDestroy void onDestroy() {}
}

Here you want:

1. newInstance somehow
2.a. inject cdi instances
2.b. inject tomcat instances
3. postconstruct

4. predestroy
5. release creation context if relevant (not normal scoped bean mainly)

2.a and b only work if you have 2 injectors (or chain the injections in
instance manager since it is equivalent) but the trick is that OWB can
handle tomcat injections - it has API/plugins for part of that - or tomcat
can handle OWB injections.
You also have the hybrid case: @Inject @Resource Foo bar; is not forbidden
by the spec and is a valid CDI case - where an extension can make @Resource
a qualifier.
So at the end, the only proper way to make this working is to make the
tomcat injections handled properly either by making tomcat aware of the CDI
model (using annotated type for ex and using it to filter out injections to
not do) or to just let CDI handle all injections importing the tomcat JNDI
support in CDI - that's what TomEE does.

But in all cases, Injector API does not solve the injection issue and it is
likely than solving the injection issue you will end up with a
postconstruct handling which would