Re: Bootstrap and modules

2020-06-12 Thread Raymond Auge
Hey Mark,

I'll have to get back to this convo in a day or so.

I'll test your theory but at first glance it sounds like going in the right
direction.

- Ray

On Thu, Jun 11, 2020 at 5:08 PM Mark Thomas  wrote:

> On 11/06/2020 21:59, Mark Thomas wrote:
> > On 11/06/2020 21:24, Raymond Auge wrote:
> >> This can totally be fixed in configuration. There is no problem. I just
> >> wanted to make sure that in doing so we wouldn't just be pushing some
> >> dirt under the rug so to speak.
> >
> > I'm concerned we might be doing exactly that now we are heading into a
> > JPMS world and this seems like an opportunity to pause and see if there
> > is a better way.
> >
> > I've yet to get my head around JPMS and I might be mis-remembering some
> > of the things I have read.
> >
> > bootstrap.jar has everything it needs to start, create the class loader
> > hierarchy, switch to the catalinaLoader (which can see all the JARs
> > rather than just bootstrap.jar and tomcat-juli.jar) and then continue
> > with start-up.
> >
> > It does things this way because the class loader hierarchy and the
> > configuration of the class loaders is configurable. So we can't just put
> > everything on the class path before starting the JVM.
> >
> > Any static analysis of bootstrap.jar is always going to show it having
> > dependencies that won't be visible until Tomcat starts and the
> > catalinaLoader is up and running. To what extent does this cause
> > complications for JPMS and/or OSGi?
> >
> > Is this completely manageable with configuration? If it is, I think I'd
> > lean towards a configuration based solution primarily for backwards
> > compatibility reasons. What are the arguments against this approach?
> >
> > If this completely manageable with configuration are there any
> > particular classes that are causing more than their fair share of pain
> > where a small packaging change would provide a relatively large benefit?
>
> Sorry. More thoughts occurred to me as I looked at the PRs again.
>
> If we created o.a.c.startup.Constants, defined the constants required by
> the bootstrap classes there and then referenced those from o.a.c.Globals
> would that help?
>
> Is it Tool's use of ExceptionUtils that is causing that class to be
> needed? If so would making Bootstrap.handleThrowable() package private
> and using that instead help?
>
> Mark
>
> >
> > Mark
> >
> >
> >>
> >> :)
> >>
> >> - Ray
> >>
> >> On Thu, Jun 11, 2020 at 3:25 PM Raymond Auge  >> > wrote:
> >>
> >> To be clear, it's not necessarily having _all of a package_. It's
> >> more about all the reachable class references. For instance jdeps
> >> looks at classes and finds any reachable references. So does bnd for
> >> calculating OSGi metadata.
> >>
> >> So the issue is not really about packages, it's about having missing
> >> class references and whether those should be elided in
> >> configuration, or simple fixed in packaging (which again does not
> >> necessarily mean full packages :)
> >>
> >> - Ray
> >>
> >> On Thu, Jun 11, 2020 at 3:20 PM Rémy Maucherat  >> > wrote:
> >>
> >> On Thu, Jun 11, 2020 at 9:01 PM Mark Thomas  >> > wrote:
> >>
> >> Hi,
> >>
> >> As discussed in PR#298 [1], properly/fully/correctly
> >> supporting JPMS /
> >> OSGi gets trickier than necessary with the bootstrap JAR
> >> because of the
> >> way we currently package it with the minimum that it needs
> >> and duplicate
> >> some classes.
> >>
> >> My simplistic understanding is that having all of a Java
> >> package in a
> >> single JAR makes JPMS and OSGi a lot simpler. Further, our
> >> current
> >> approach may not be 100% compatible with one or both of
> them.
> >>
> >> The trade-offs involved here are:
> >> - having all of a package in a JAR simplifies JPMS and OSGi
> >> - We want to keep the bootstrap JAR small (is this much of a
> >> concern?)
> >> - We don't want duplicate code (maintenance overhead)
> >> - Bootstrap uses various utility functions from the Tomcat
> >> code base
> >>
> >> I'm wondering if there is a better approach we could adopt
> >> that makes
> >> JPMS / OSGi simpler without compromising too much on the
> >> other trade-offs.
> >>
> >> The sort of thing I have in mind is:
> >> - move everything out of o.a.c.startup that doesn't need to
> >> be there to
> >>   either some new package (name TBD) or an existing package
> >>
> >>
> >> That means way too many risky changes IMO, the listeners in the
> >> startup package are often extended to add features so they need
> >> to 

Re: Bootstrap and modules

2020-06-12 Thread Mark Thomas
On 12/06/2020 14:15, Raymond Auge wrote:
> Hey Mark,
> 
> I'll have to get back to this convo in a day or so.
> 
> I'll test your theory but at first glance it sounds like going in the
> right direction.

No rush. I'd rather take time and get this right.

Thanks,

Mark


> 
> - Ray
> 
> On Thu, Jun 11, 2020 at 5:08 PM Mark Thomas  > wrote:
> 
> On 11/06/2020 21:59, Mark Thomas wrote:
> > On 11/06/2020 21:24, Raymond Auge wrote:
> >> This can totally be fixed in configuration. There is no problem.
> I just
> >> wanted to make sure that in doing so we wouldn't just be pushing some
> >> dirt under the rug so to speak.
> >
> > I'm concerned we might be doing exactly that now we are heading into a
> > JPMS world and this seems like an opportunity to pause and see if
> there
> > is a better way.
> >
> > I've yet to get my head around JPMS and I might be mis-remembering
> some
> > of the things I have read.
> >
> > bootstrap.jar has everything it needs to start, create the class
> loader
> > hierarchy, switch to the catalinaLoader (which can see all the JARs
> > rather than just bootstrap.jar and tomcat-juli.jar) and then continue
> > with start-up.
> >
> > It does things this way because the class loader hierarchy and the
> > configuration of the class loaders is configurable. So we can't
> just put
> > everything on the class path before starting the JVM.
> >
> > Any static analysis of bootstrap.jar is always going to show it having
> > dependencies that won't be visible until Tomcat starts and the
> > catalinaLoader is up and running. To what extent does this cause
> > complications for JPMS and/or OSGi?
> >
> > Is this completely manageable with configuration? If it is, I
> think I'd
> > lean towards a configuration based solution primarily for backwards
> > compatibility reasons. What are the arguments against this approach?
> >
> > If this completely manageable with configuration are there any
> > particular classes that are causing more than their fair share of pain
> > where a small packaging change would provide a relatively large
> benefit?
> 
> Sorry. More thoughts occurred to me as I looked at the PRs again.
> 
> If we created o.a.c.startup.Constants, defined the constants required by
> the bootstrap classes there and then referenced those from o.a.c.Globals
> would that help?
> 
> Is it Tool's use of ExceptionUtils that is causing that class to be
> needed? If so would making Bootstrap.handleThrowable() package private
> and using that instead help?
> 
> Mark
> 
> >
> > Mark
> >
> >
> >>
> >> :)
> >>
> >> - Ray
> >>
> >> On Thu, Jun 11, 2020 at 3:25 PM Raymond Auge
> mailto:raymond.a...@liferay.com>
> >>  >> wrote:
> >>
> >>     To be clear, it's not necessarily having _all of a package_. It's
> >>     more about all the reachable class references. For instance jdeps
> >>     looks at classes and finds any reachable references. So does
> bnd for
> >>     calculating OSGi metadata.
> >>
> >>     So the issue is not really about packages, it's about having
> missing
> >>     class references and whether those should be elided in
> >>     configuration, or simple fixed in packaging (which again does not
> >>     necessarily mean full packages :)
> >>
> >>     - Ray
> >>
> >>     On Thu, Jun 11, 2020 at 3:20 PM Rémy Maucherat
> mailto:r...@apache.org>
> >>     >> wrote:
> >>
> >>         On Thu, Jun 11, 2020 at 9:01 PM Mark Thomas
> mailto:ma...@apache.org>
> >>         >> wrote:
> >>
> >>             Hi,
> >>
> >>             As discussed in PR#298 [1], properly/fully/correctly
> >>             supporting JPMS /
> >>             OSGi gets trickier than necessary with the bootstrap JAR
> >>             because of the
> >>             way we currently package it with the minimum that it
> needs
> >>             and duplicate
> >>             some classes.
> >>
> >>             My simplistic understanding is that having all of a Java
> >>             package in a
> >>             single JAR makes JPMS and OSGi a lot simpler.
> Further, our
> >>             current
> >>             approach may not be 100% compatible with one or both
> of them.
> >>
> >>             The trade-offs involved here are:
> >>             - having all of a package in a JAR simplifies JPMS
> and OSGi
> >>             - We want to keep the bootstrap JAR small (is this
> much of a
> >>             concern?)
> >

Java library bug in JCEKS keystore loader

2020-06-12 Thread Christopher Schultz
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

All,

I've been writing a Java-based certification-expiration checking
utility that can handle all kinds of file formats like PEM and the
various keystore formats supported by the JVM.

Since it's not possible to tell what type of keystore is being loaded
without writing a bunch of magic-checking code or implementing an
ASN.1 parser (no thank you), I simply try all keystore types until I
find one that works. I'm using a rewindable InputStream which works well
.

Until you try to use the JCEKS keystore loader, specifically in Java
versions 8 - 13. I haven't checked that later versions, and it appears
that it's been fixed in Java 14.

The API call is KeyStore.load(InputStream, char[]) and usually it
behaves well. Unfortunately, in these affected versions, the JCEKS
provider (which happens to be com.sun.crypto.provider.JceKeyStore, in
the public engineLoad method) calls InputStream.close on the stream
passed-into it. Yuck.

JCEKS is a little-used keystore format, but it is required to store
secret keys (different from private keys) because JKS can't support
those. So it's possible that some users have been forced to use JCEKS
keystore format.

I haven't looked-through the Tomcat code yet to see if there could be
places where this could cause a problem. Sane code usually looks like
this:

try(FileInputStream fin = new FileInputStream("keystore.jceks")) {
  KeyStore ks = KeyStore.getInstance("JCEKS");
  ks.load(fin);
}

This code will fail with "java.io.IOException: Stream closed", which
is of course incorrect resource management.

My solution was to simply wrap the InputStream in one that will simply
ignore any attempts to close it:

try(FileInputStream fin = new UnclosableInputStream(new
FileInputStream("keystore.jceks"))) {
  KeyStore ks = KeyStore.getInstance("JCEKS");
  ks.load(fin);
}

public class UnclosableInputStream
 extends java.io.FilterInputStream
{
protected UnclosableInputStream(InputStream in) {
super(in);
}

@Override
public void close() throws IOException
{
// System.out.println("DEBUG: Ignoring attempt to close
InputStream by provider of keystore type " + keystoreType);
// new Throwable("trace").printStackTrace();
}
}

I'll be looking at Tomcat's keystore-loading code to see if this would
be helpful, as not many people are running Tomcat on Java 14 yet, I
would guess.

- -chris

P.S. I'll probably be releasing this tool on GitHub, soon, if anyone
wants to have a look.
-BEGIN PGP SIGNATURE-
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl7j+XgACgkQHPApP6U8
pFgUPQ/+L7DQhP19ELBKYUbXQgdKN+aieiJoQap5wID+QZSXBJmbEgnUvrRcLdte
oBaTgOJc3pKc6lV2K6PYIzT2HTpB4420h9cbn6vtFAuTdbN08RTxO5e6IGBUovtT
o/Y0iVSy4m3aQfOg7nISqGoaVggDffTrr/ZQDPgjxbTbAeTHEhk7eCN0nNMWmzFD
V2jn7b7IOowZoXRSWlMZ4FqmjPIugckmIs1qDfzooPQhdplLpbJn1ucLikG/j5jj
kmeCRaiPtNCC3FmhlBgkU5Ac50LksoY4uR0dROzpxyZbClyUnq2zAIY/9paygG8s
ZK/FZIaTdzJU3VeemaE3mMuCIDtTwzmRJH29kMTz1X4GOvsL3PbHFqeav9KGmUWO
H3ASkTe27jhXSzxLrbxIsaQDRTAjscMMBAjy11Nsp5Qism7JQ7OgKiX0nOnR2Vne
hP7cgk9snAkI9QhnTnyqnSd7dYKjMqEmmIa58YuRc+OaUeNf3wwmljSpKYvI2D31
ldBCEVF4PE+GNT0DXGfX2osTLMkglkDkgElHWennTDkHko72aexNX7c3Ut3+ldRu
ho6CG2Fwaugw/QS3uk9E/ZfXwl5ZU2Rn4ysWac17FHgOA7A/yHrPkJxDxKGZHSHI
rzMgeyquIpizAJEj6NKNQJdwujcZeVAhvUae+mQyoaJYplEpKRs=
=AakA
-END PGP SIGNATURE-

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



Re: Bootstrap and modules

2020-06-12 Thread Raymond Auge
There is one difference between

org.apache.tomcat.util.ExceptionUtils.handleThrowable(Throwable)

and

org.apache.catalina.startup.Bootstrap.handleThrowable(Throwable)

that is that ExceptionUtils also swallows StackOverflowError while
Bootstrap does not.
Should that be ported over or is it a deal breaker? An option would be to
add an additional method to Bootstrap that behaves like ExceptionUtils.

- Ray


On Fri, Jun 12, 2020 at 9:27 AM Mark Thomas  wrote:

> On 12/06/2020 14:15, Raymond Auge wrote:
> > Hey Mark,
> >
> > I'll have to get back to this convo in a day or so.
> >
> > I'll test your theory but at first glance it sounds like going in the
> > right direction.
>
> No rush. I'd rather take time and get this right.
>
> Thanks,
>
> Mark
>
>
> >
> > - Ray
> >
> > On Thu, Jun 11, 2020 at 5:08 PM Mark Thomas  > > wrote:
> >
> > On 11/06/2020 21:59, Mark Thomas wrote:
> > > On 11/06/2020 21:24, Raymond Auge wrote:
> > >> This can totally be fixed in configuration. There is no problem.
> > I just
> > >> wanted to make sure that in doing so we wouldn't just be pushing
> some
> > >> dirt under the rug so to speak.
> > >
> > > I'm concerned we might be doing exactly that now we are heading
> into a
> > > JPMS world and this seems like an opportunity to pause and see if
> > there
> > > is a better way.
> > >
> > > I've yet to get my head around JPMS and I might be mis-remembering
> > some
> > > of the things I have read.
> > >
> > > bootstrap.jar has everything it needs to start, create the class
> > loader
> > > hierarchy, switch to the catalinaLoader (which can see all the JARs
> > > rather than just bootstrap.jar and tomcat-juli.jar) and then
> continue
> > > with start-up.
> > >
> > > It does things this way because the class loader hierarchy and the
> > > configuration of the class loaders is configurable. So we can't
> > just put
> > > everything on the class path before starting the JVM.
> > >
> > > Any static analysis of bootstrap.jar is always going to show it
> having
> > > dependencies that won't be visible until Tomcat starts and the
> > > catalinaLoader is up and running. To what extent does this cause
> > > complications for JPMS and/or OSGi?
> > >
> > > Is this completely manageable with configuration? If it is, I
> > think I'd
> > > lean towards a configuration based solution primarily for backwards
> > > compatibility reasons. What are the arguments against this
> approach?
> > >
> > > If this completely manageable with configuration are there any
> > > particular classes that are causing more than their fair share of
> pain
> > > where a small packaging change would provide a relatively large
> > benefit?
> >
> > Sorry. More thoughts occurred to me as I looked at the PRs again.
> >
> > If we created o.a.c.startup.Constants, defined the constants
> required by
> > the bootstrap classes there and then referenced those from
> o.a.c.Globals
> > would that help?
> >
> > Is it Tool's use of ExceptionUtils that is causing that class to be
> > needed? If so would making Bootstrap.handleThrowable() package
> private
> > and using that instead help?
> >
> > Mark
> >
> > >
> > > Mark
> > >
> > >
> > >>
> > >> :)
> > >>
> > >> - Ray
> > >>
> > >> On Thu, Jun 11, 2020 at 3:25 PM Raymond Auge
> > mailto:raymond.a...@liferay.com>
> > >>  > >> wrote:
> > >>
> > >> To be clear, it's not necessarily having _all of a package_.
> It's
> > >> more about all the reachable class references. For instance
> jdeps
> > >> looks at classes and finds any reachable references. So does
> > bnd for
> > >> calculating OSGi metadata.
> > >>
> > >> So the issue is not really about packages, it's about having
> > missing
> > >> class references and whether those should be elided in
> > >> configuration, or simple fixed in packaging (which again does
> not
> > >> necessarily mean full packages :)
> > >>
> > >> - Ray
> > >>
> > >> On Thu, Jun 11, 2020 at 3:20 PM Rémy Maucherat
> > mailto:r...@apache.org>
> > >> >> wrote:
> > >>
> > >> On Thu, Jun 11, 2020 at 9:01 PM Mark Thomas
> > mailto:ma...@apache.org>
> > >> >>
> wrote:
> > >>
> > >> Hi,
> > >>
> > >> As discussed in PR#298 [1], properly/fully/correctly
> > >> supporting JPMS /
> > >> OSGi gets trickier than necessary with the bootstrap
> JAR
> > >> because of the
> > >> way we currently packag

Re: Bootstrap and modules

2020-06-12 Thread Raymond Auge
Actually Bootstrap has a comment

// Copied from ExceptionUtils since that class is not visible during start

So it seems like perhaps the change should be ported.

 - Ray

On Fri, Jun 12, 2020 at 10:45 PM Raymond Auge 
wrote:

> There is one difference between
>
> org.apache.tomcat.util.ExceptionUtils.handleThrowable(Throwable)
>
> and
>
> org.apache.catalina.startup.Bootstrap.handleThrowable(Throwable)
>
> that is that ExceptionUtils also swallows StackOverflowError while
> Bootstrap does not.
> Should that be ported over or is it a deal breaker? An option would be to
> add an additional method to Bootstrap that behaves like ExceptionUtils.
>
> - Ray
>
>
> On Fri, Jun 12, 2020 at 9:27 AM Mark Thomas  wrote:
>
>> On 12/06/2020 14:15, Raymond Auge wrote:
>> > Hey Mark,
>> >
>> > I'll have to get back to this convo in a day or so.
>> >
>> > I'll test your theory but at first glance it sounds like going in the
>> > right direction.
>>
>> No rush. I'd rather take time and get this right.
>>
>> Thanks,
>>
>> Mark
>>
>>
>> >
>> > - Ray
>> >
>> > On Thu, Jun 11, 2020 at 5:08 PM Mark Thomas > > > wrote:
>> >
>> > On 11/06/2020 21:59, Mark Thomas wrote:
>> > > On 11/06/2020 21:24, Raymond Auge wrote:
>> > >> This can totally be fixed in configuration. There is no problem.
>> > I just
>> > >> wanted to make sure that in doing so we wouldn't just be pushing
>> some
>> > >> dirt under the rug so to speak.
>> > >
>> > > I'm concerned we might be doing exactly that now we are heading
>> into a
>> > > JPMS world and this seems like an opportunity to pause and see if
>> > there
>> > > is a better way.
>> > >
>> > > I've yet to get my head around JPMS and I might be mis-remembering
>> > some
>> > > of the things I have read.
>> > >
>> > > bootstrap.jar has everything it needs to start, create the class
>> > loader
>> > > hierarchy, switch to the catalinaLoader (which can see all the
>> JARs
>> > > rather than just bootstrap.jar and tomcat-juli.jar) and then
>> continue
>> > > with start-up.
>> > >
>> > > It does things this way because the class loader hierarchy and the
>> > > configuration of the class loaders is configurable. So we can't
>> > just put
>> > > everything on the class path before starting the JVM.
>> > >
>> > > Any static analysis of bootstrap.jar is always going to show it
>> having
>> > > dependencies that won't be visible until Tomcat starts and the
>> > > catalinaLoader is up and running. To what extent does this cause
>> > > complications for JPMS and/or OSGi?
>> > >
>> > > Is this completely manageable with configuration? If it is, I
>> > think I'd
>> > > lean towards a configuration based solution primarily for
>> backwards
>> > > compatibility reasons. What are the arguments against this
>> approach?
>> > >
>> > > If this completely manageable with configuration are there any
>> > > particular classes that are causing more than their fair share of
>> pain
>> > > where a small packaging change would provide a relatively large
>> > benefit?
>> >
>> > Sorry. More thoughts occurred to me as I looked at the PRs again.
>> >
>> > If we created o.a.c.startup.Constants, defined the constants
>> required by
>> > the bootstrap classes there and then referenced those from
>> o.a.c.Globals
>> > would that help?
>> >
>> > Is it Tool's use of ExceptionUtils that is causing that class to be
>> > needed? If so would making Bootstrap.handleThrowable() package
>> private
>> > and using that instead help?
>> >
>> > Mark
>> >
>> > >
>> > > Mark
>> > >
>> > >
>> > >>
>> > >> :)
>> > >>
>> > >> - Ray
>> > >>
>> > >> On Thu, Jun 11, 2020 at 3:25 PM Raymond Auge
>> > mailto:raymond.a...@liferay.com>
>> > >> > > >> wrote:
>> > >>
>> > >> To be clear, it's not necessarily having _all of a package_.
>> It's
>> > >> more about all the reachable class references. For instance
>> jdeps
>> > >> looks at classes and finds any reachable references. So does
>> > bnd for
>> > >> calculating OSGi metadata.
>> > >>
>> > >> So the issue is not really about packages, it's about having
>> > missing
>> > >> class references and whether those should be elided in
>> > >> configuration, or simple fixed in packaging (which again
>> does not
>> > >> necessarily mean full packages :)
>> > >>
>> > >> - Ray
>> > >>
>> > >> On Thu, Jun 11, 2020 at 3:20 PM Rémy Maucherat
>> > mailto:r...@apache.org>
>> > >> >> wrote:
>> > >>
>> > >> On Thu, Jun 11, 2020 at 9:01 PM Mark Thomas
>> > mailto:ma...@apache.org>
>> > >>