Re: [tomcat] branch master updated: Avoid reflection for default instantiation
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256
Filip,
On 7/22/20 12:41, Filip Hanik wrote:
> Hi Christopher,
>>> environments. -Class clazz =
>>> Class.forName(className); -return
>>> (AuthConfigFactory) clazz.getConstructor().newInstance(); + if
>>> (className.equals("org.apache.catalina.authenticator.jaspic.AuthConf
ig
>>
>>>
FactoryImpl"))
>>> {
>>
>> Why not use AuthConfigFactoryImpl.class.getName()? It may help in
>> the future with refactoring.
>
> [Filip Hanik] Trying to avoid a circular dependency. You see the
> javax/jakarta package should not import org.apache.catalina code. I
> should be able to execute the AuthConfigFactory code without
> needing to load
> org.apache.catalina.authenticator.jaspic.AuthConfigFactoryImpl
> class. The JVM is smart enough that if the execution doesn't enter
> the if statement block, it won't attempt the classloading of the
> AuthConfigFactoryImpl class. However, if the AuthConfigFactoryImpl
> Class itself is part of the evaluation statement, it will be
> loaded.
>
> The previous implementation also had it as a string, instead of
> AuthConfigFactoryImpl.class.getName() for the same reason.
> https://github.com/apache/tomcat/blob/35dc7b9288aad4a7d70750c157543d4f
f1593c98/java/jakarta/security/auth/message/config/AuthConfigFactory.jav
a#L48-L49
>
> This way, I can build a jakarta.security.auth.message library,
> that can be used without the org.apache.catalina library.
That's a very good reason. Thanks for explaining.
> I need to change my commit to use the constant, instead of the
> duplicated string in the IF statement.
>
> if (className.equals(DEFAULT_JASPI_AUTHCONFIGFACTORYIMPL)) { return
> new
> org.apache.catalina.authenticator.jaspic.AuthConfigFactoryImpl(); }
> else { Class clazz = Class.forName(className); return
> (AuthConfigFactory) clazz.getConstructor().newInstance(); }
:)
- -chris
-BEGIN PGP SIGNATURE-
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl8ZyrUACgkQHPApP6U8
pFghOg//UxcPA7Xm+SKXtyCWEIcabjFkbdHv9o8kOHTiWVSUOrhXLBvTznDKYmYk
+5zFjxsFbBjj1amQnGER/3zJSSJmvdcEUohpHpYHUHFGLh59YyXTVb0ou8PSiX+B
iFEZCqKrkylZPCn21tPXN4wgmnvQxcD9S3++vZBWyCCiQw/BoUYwYGLtg9/sgCjj
eqk2r/yqGRlVtHsMEu04wMadcqpum6f14LO1b8J1C5jP0N9fwiGTEsD4HXskcUfQ
PeDjHtG6tJPXwfYjwRjzZolQIFmQwJ1B6WLufsyw0ZrUVBUENkU4xQwBy3pcVewn
0xc9+vgg+VSXblrDMnoswUBf2hLmfmpw49evcjeKSY7q0G0Fdoe1lUmt+OM74ppK
EZ6qmvCphWtXakyCU5uXx82nQMsNXdwmgLG3Dni4ya79dVoGvn8j3Guh/7g45Jet
0bc7x7KsouqJIbqQAPqdt8E2xKRARsdUE4BH9S0ENkvTqnhhbCWmlxJk3CiPpwoy
3zD5xt9CoXuKX/CjK92hP+nw4b1j+Mdhmwj4whM4FbcvLC6Rq8JXt0obysArweT8
FbfZD45OsWqbgVrVpwcCt19z25+6Ar02DvUz0CkI1sbxzS+hd1yhp2BRbiqTd6HY
EkUodl2R7H95ZmIRZ0QhcnEyq5tPJUcspXLvmzQ5Li25oJEWLrc=
=XYLJ
-END PGP SIGNATURE-
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
RE: [tomcat] branch master updated: Avoid reflection for default instantiation
Hi Christopher,
> > environments. -Class clazz =
> > Class.forName(className); -return
> > (AuthConfigFactory) clazz.getConstructor().newInstance(); + if
> > (className.equals("org.apache.catalina.authenticator.jaspic.AuthConfig
> FactoryImpl"))
> > {
>
> Why not use AuthConfigFactoryImpl.class.getName()? It may help in the
> future with refactoring.
[Filip Hanik]
Trying to avoid a circular dependency. You see the javax/jakarta package should
not import org.apache.catalina code. I should be able to execute the
AuthConfigFactory code without needing to load
org.apache.catalina.authenticator.jaspic.AuthConfigFactoryImpl class. The JVM
is smart enough that if the execution doesn't enter the if statement block, it
won't attempt the classloading of the AuthConfigFactoryImpl class. However, if
the AuthConfigFactoryImpl Class itself is part of the evaluation statement, it
will be loaded.
The previous implementation also had it as a string, instead of
AuthConfigFactoryImpl.class.getName() for the same reason.
https://github.com/apache/tomcat/blob/35dc7b9288aad4a7d70750c157543d4ff1593c98/java/jakarta/security/auth/message/config/AuthConfigFactory.java#L48-L49
This way, I can build a jakarta.security.auth.message library, that can be used
without the org.apache.catalina library.
I need to change my commit to use the constant, instead of the duplicated
string in the IF statement.
if
(className.equals(DEFAULT_JASPI_AUTHCONFIGFACTORYIMPL)) {
return new
org.apache.catalina.authenticator.jaspic.AuthConfigFactoryImpl();
} else {
Class clazz = Class.forName(className);
return (AuthConfigFactory)
clazz.getConstructor().newInstance();
}
>
> - -chris
Re: [tomcat] branch master updated: Avoid reflection for default instantiation
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256
Filip,
On 7/21/20 11:22, fha...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git
> repository.
>
> fhanik pushed a commit to branch master in repository
> https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this
> push: new c08bf81 Avoid reflection for default instantiation
> c08bf81 is described below
>
> commit c08bf81f0db7742779ab0c5da45818dde8465d34 Author: Filip Hanik
> AuthorDate: Mon Jul 13 12:43:55 2020 -0700
>
> Avoid reflection for default instantiation
>
> (Most commonly used codepath)
>
> Avoid having to load APR classes in the Connector
>
> Ensure that IntrospectionUtils can call setProperty on
> PersistentProviderRegistrations ---
> .../auth/message/config/AuthConfigFactory.java | 8 ++-
> .../jaspic/PersistentProviderRegistrations.java| 12 -
> java/org/apache/catalina/connector/Connector.java | 8 +--
> .../apache/catalina/core/AprLifecycleListener.java | 32
> +--- java/org/apache/catalina/core/AprStatus.java |
> 60 ++
> java/org/apache/catalina/core/StandardHost.java| 4 +-
> java/org/apache/catalina/loader/WebappLoader.java | 4 ++
> java/org/apache/catalina/startup/Tomcat.java | 8 ++- 8 files
> changed, 109 insertions(+), 27 deletions(-)
>
> diff --git
> a/java/jakarta/security/auth/message/config/AuthConfigFactory.java
> b/java/jakarta/security/auth/message/config/AuthConfigFactory.java
> index d0e1cbd..6f02fde 100644 ---
> a/java/jakarta/security/auth/message/config/AuthConfigFactory.java
> +++
> b/java/jakarta/security/auth/message/config/AuthConfigFactory.java
> @@ -72,8 +72,12 @@ public abstract class AuthConfigFactory { //
> this class. Note that the Thread context class loader // should not
> be used since that would trigger a memory leak // in container
> environments. -Class clazz =
> Class.forName(className); -return
> (AuthConfigFactory) clazz.getConstructor().newInstance(); +
> if
> (className.equals("org.apache.catalina.authenticator.jaspic.AuthConfig
FactoryImpl"))
> {
Why not use AuthConfigFactoryImpl.class.getName()? It may help in the
future with refactoring.
- -chris
-BEGIN PGP SIGNATURE-
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl8YXgIACgkQHPApP6U8
pFgHmA/8CweFBvWtEn14ZzUZHkA/7HVaLPG6r7Y4qzkcrzJfQImYzV7E0x3NL59m
fDagGxJrxugESEKf3HLmq5VIzAlemkbPxYQ7S4KaUJVHbRW/MH9zPVzbvAVXy4Gm
CIpVF/QoXftJ9WYsMkwFFu8ZeRsUQSJ5Z4eFXmHgOzDSj42vUR7VDsFXmpoqdIpC
jp0CV9p+XyfAcvtsJXnTKKmDGFV7liH4d38mz8wNLFw1yFk8jswHeyzzy+6u9QVu
fFno1AZ67UWjeOlMz+kQ4S9n3X+irT03Qpc8+kvWDibnEDYuHivvhvROWOn4ja92
dyF+6YZxOGIf4QHwM0BHL+8IzrcodB15j7Iv0Fw9VKrJvcj55qZTerHvOkiwUDQF
1vsiqPtOEWrE4q87Y7aev3WBpRWfxQFu50IQNIAvPwiBmT9mj+3iztq46m5CTtnt
zjfdzxEMF5n74L+2u+CPIekngJ8i0RJOkq4UGrJmXpbX/82q+eN+TDws6GchRquG
3Hr2EgC3oocITMTbu+5ZjvbBVAh30VhqlOF1GwO8YSBIgvNUyPvIqZXK4Re9gjYm
BSRHl2juGFP3d1OSkdimWBkBc8MHx3QOkcCOzZYgPg3mdAq7beqKgh/DTvqQd5D8
MXGe4cgfHoOEY0X53K/qG1KXcIntRXab9Nue8GriGC9M7oMWgIM=
=48+3
-END PGP SIGNATURE-
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch master updated: Avoid reflection for default instantiation
This is an automated email from the ASF dual-hosted git repository.
fhanik pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new c08bf81 Avoid reflection for default instantiation
c08bf81 is described below
commit c08bf81f0db7742779ab0c5da45818dde8465d34
Author: Filip Hanik
AuthorDate: Mon Jul 13 12:43:55 2020 -0700
Avoid reflection for default instantiation
(Most commonly used codepath)
Avoid having to load APR classes in the Connector
Ensure that IntrospectionUtils can call setProperty on
PersistentProviderRegistrations
---
.../auth/message/config/AuthConfigFactory.java | 8 ++-
.../jaspic/PersistentProviderRegistrations.java| 12 -
java/org/apache/catalina/connector/Connector.java | 8 +--
.../apache/catalina/core/AprLifecycleListener.java | 32 +---
java/org/apache/catalina/core/AprStatus.java | 60 ++
java/org/apache/catalina/core/StandardHost.java| 4 +-
java/org/apache/catalina/loader/WebappLoader.java | 4 ++
java/org/apache/catalina/startup/Tomcat.java | 8 ++-
8 files changed, 109 insertions(+), 27 deletions(-)
diff --git a/java/jakarta/security/auth/message/config/AuthConfigFactory.java
b/java/jakarta/security/auth/message/config/AuthConfigFactory.java
index d0e1cbd..6f02fde 100644
--- a/java/jakarta/security/auth/message/config/AuthConfigFactory.java
+++ b/java/jakarta/security/auth/message/config/AuthConfigFactory.java
@@ -72,8 +72,12 @@ public abstract class AuthConfigFactory {
// this class. Note that the Thread context class
loader
// should not be used since that would trigger a
memory leak
// in container environments.
-Class clazz = Class.forName(className);
-return (AuthConfigFactory)
clazz.getConstructor().newInstance();
+if
(className.equals("org.apache.catalina.authenticator.jaspic.AuthConfigFactoryImpl"))
{
+return new
org.apache.catalina.authenticator.jaspic.AuthConfigFactoryImpl();
+} else {
+Class clazz = Class.forName(className);
+return (AuthConfigFactory)
clazz.getConstructor().newInstance();
+}
}
});
} catch (PrivilegedActionException e) {
diff --git
a/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java
b/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java
index a1ba60c..8ffe8ec 100644
---
a/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java
+++
b/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java
@@ -41,7 +41,7 @@ import org.xml.sax.SAXException;
* Utility class for the loading and saving of JASPIC persistent provider
* registrations.
*/
-final class PersistentProviderRegistrations {
+public final class PersistentProviderRegistrations {
private static final StringManager sm =
StringManager.getManager(PersistentProviderRegistrations.class);
@@ -233,6 +233,16 @@ final class PersistentProviderRegistrations {
public void addProperty(Property property) {
properties.put(property.getName(), property.getValue());
}
+
+/**
+ * Used by IntrospectionUtils via reflection.
+ * @param name - the name of of the property to set on this object
+ * @param value - the value to set
+ * @see #addProperty(String, String)
+ */
+public void setProperty(String name, String value) {
+addProperty(name, value);
+}
void addProperty(String name, String value) {
properties.put(name, value);
}
diff --git a/java/org/apache/catalina/connector/Connector.java
b/java/org/apache/catalina/connector/Connector.java
index c35c4cf..c2e7e25 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -29,7 +29,7 @@ import org.apache.catalina.Globals;
import org.apache.catalina.LifecycleException;
import org.apache.catalina.LifecycleState;
import org.apache.catalina.Service;
-import org.apache.catalina.core.AprLifecycleListener;
+import org.apache.catalina.core.AprStatus;
import org.apache.catalina.util.LifecycleMBeanBase;
import org.apache.coyote.AbstractProtocol;
import org.apache.coyote.Adapter;
@@ -1022,15 +1022,15 @@ public class Connector extends LifecycleMBeanBase {
setParseBodyMethods(getParseBodyMethods());
}
-if (protocolHandler.isAprRequired() &&
!AprLifecycleListener.isInstanceCreated()) {
+if
