Re: [tomcat] 02/02: Additional changes required to enable EnvironmentPropertySource
Am 15.09.19 um 11:03 schrieb Mark Thomas:
> On 14/09/2019 20:01, Felix Schumacher wrote:
>> Am 12.09.19 um 22:40 schrieb ma...@apache.org:
>>> This is an automated email from the ASF dual-hosted git repository.
>>>
>>> markt pushed a commit to branch master
>>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>>>
>>> commit cae17a52598393680952aa21cee0e27b13a73455
>>> Author: Mark Thomas
>>> AuthorDate: Thu Sep 12 15:31:26 2019 +0100
>>>
>>> Additional changes required to enable EnvironmentPropertySource
>>> ---
>>> .../org/apache/tomcat/util/IntrospectionUtils.java | 49
>>> --
>>> java/org/apache/tomcat/util/digester/Digester.java | 33 ++-
>>> webapps/docs/changelog.xml | 4 +-
>>> 3 files changed, 69 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/java/org/apache/tomcat/util/IntrospectionUtils.java
>>> b/java/org/apache/tomcat/util/IntrospectionUtils.java
>>> index 3ffa702..f6ac737 100644
>>> --- a/java/org/apache/tomcat/util/IntrospectionUtils.java
>>> +++ b/java/org/apache/tomcat/util/IntrospectionUtils.java
>>> @@ -476,9 +499,27 @@ public final class IntrospectionUtils {
>>> // This provides a layer of abstraction
>>>
>>> public static interface PropertySource {
>>> -
>>> public String getProperty(String key);
>>> -
>>> }
>>>
>>> +
>>> +public static interface PropertySourceSecure extends PropertySource {
>> I think a better name would be SecurePropertySource or
>> ClassloaderAwarePropertySource. The thing that it represents should be
>> at the end of the name IMHO.
> Fair enough. I prefer "SecurePropertySource" so I'll go with that before
> I tag.
>
>> At work I prototyped a similar approach and introduced a
>> NamespaceAwarePropertySource. It is basically an interface that has a
>> getNamespace() method that returns a prefix for the keys. I think that
>> it would be nice if these two approaches.
> Sorry, I'm not quite understanding how this works or the use case it is
> trying to address. Could you provide a simple example?
A namespaced PropertySource would look like this
interface NamespacedPropertySource extends PropertySource {
String getNamespace(); // or getPrefix()
}
Those PropertySources would be registered by the service loader approach
into a map with their namespace as a key.
If a property is looked up with a key, for example "env.hostname", that
key would be split into the namespace (or prefix) and the actual key for
the source. The SecureProperty from the above map (found by the
namespace) would then be asked to resolve the property.
In this setup, the multiplexer that is looking up the source could check
with the security manager whether access to the key is allowed and thus
freeing the implementer of the NamespacedProperty of doing this work.
class MultiPropertySource implements SecurePropertySource { // not a
good name
Map sources = findThemByServiceLoader();
String getProperty(String key, ClassLoader classLoader) {
String[] nameComponents = key.split(":", 2); // uses a colon for
separation as split uses a regex and a dot is a special char in that context
String namespace = nameComponents[0];
String realKey = nameComponents[1];
if (!checkSecurity(namespace, realKey, classLoader)) { // this would
do the security check
return null;
}
SecurePropertySource source = sources.get(namespace);
return source.getProperty(realKey);
}
}
Does this make sense to you?
Felix
>
>> My prototype didn't try to
>> call a security manager, but with this commit it would be easy to add.
>>
>> On the other hand it uses a ServiceLoader approach to automatically find
>> all NamespaceAwarePropertySources. Do you think this would be a good
>> addition for Tomcat?
> There is an entry in TOMCAT-NEXT around reducing the use of system
> properties. A ServiceLoader approach may be a good solution for some of
> those.
>
> Mark
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 02/02: Additional changes required to enable EnvironmentPropertySource
On 14/09/2019 20:01, Felix Schumacher wrote:
>
> Am 12.09.19 um 22:40 schrieb ma...@apache.org:
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> markt pushed a commit to branch master
>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>>
>> commit cae17a52598393680952aa21cee0e27b13a73455
>> Author: Mark Thomas
>> AuthorDate: Thu Sep 12 15:31:26 2019 +0100
>>
>> Additional changes required to enable EnvironmentPropertySource
>> ---
>> .../org/apache/tomcat/util/IntrospectionUtils.java | 49
>> --
>> java/org/apache/tomcat/util/digester/Digester.java | 33 ++-
>> webapps/docs/changelog.xml | 4 +-
>> 3 files changed, 69 insertions(+), 17 deletions(-)
>>
>> diff --git a/java/org/apache/tomcat/util/IntrospectionUtils.java
>> b/java/org/apache/tomcat/util/IntrospectionUtils.java
>> index 3ffa702..f6ac737 100644
>> --- a/java/org/apache/tomcat/util/IntrospectionUtils.java
>> +++ b/java/org/apache/tomcat/util/IntrospectionUtils.java
>> @@ -476,9 +499,27 @@ public final class IntrospectionUtils {
>> // This provides a layer of abstraction
>>
>> public static interface PropertySource {
>> -
>> public String getProperty(String key);
>> -
>> }
>>
>> +
>> +public static interface PropertySourceSecure extends PropertySource {
>
> I think a better name would be SecurePropertySource or
> ClassloaderAwarePropertySource. The thing that it represents should be
> at the end of the name IMHO.
Fair enough. I prefer "SecurePropertySource" so I'll go with that before
I tag.
> At work I prototyped a similar approach and introduced a
> NamespaceAwarePropertySource. It is basically an interface that has a
> getNamespace() method that returns a prefix for the keys. I think that
> it would be nice if these two approaches.
Sorry, I'm not quite understanding how this works or the use case it is
trying to address. Could you provide a simple example?
> My prototype didn't try to
> call a security manager, but with this commit it would be easy to add.
>
> On the other hand it uses a ServiceLoader approach to automatically find
> all NamespaceAwarePropertySources. Do you think this would be a good
> addition for Tomcat?
There is an entry in TOMCAT-NEXT around reducing the use of system
properties. A ServiceLoader approach may be a good solution for some of
those.
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 02/02: Additional changes required to enable EnvironmentPropertySource
Am 12.09.19 um 22:40 schrieb ma...@apache.org:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
> commit cae17a52598393680952aa21cee0e27b13a73455
> Author: Mark Thomas
> AuthorDate: Thu Sep 12 15:31:26 2019 +0100
>
> Additional changes required to enable EnvironmentPropertySource
> ---
> .../org/apache/tomcat/util/IntrospectionUtils.java | 49
> --
> java/org/apache/tomcat/util/digester/Digester.java | 33 ++-
> webapps/docs/changelog.xml | 4 +-
> 3 files changed, 69 insertions(+), 17 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/IntrospectionUtils.java
> b/java/org/apache/tomcat/util/IntrospectionUtils.java
> index 3ffa702..f6ac737 100644
> --- a/java/org/apache/tomcat/util/IntrospectionUtils.java
> +++ b/java/org/apache/tomcat/util/IntrospectionUtils.java
> @@ -476,9 +499,27 @@ public final class IntrospectionUtils {
> // This provides a layer of abstraction
>
> public static interface PropertySource {
> -
> public String getProperty(String key);
> -
> }
>
> +
> +public static interface PropertySourceSecure extends PropertySource {
I think a better name would be SecurePropertySource or
ClassloaderAwarePropertySource. The thing that it represents should be
at the end of the name IMHO.
At work I prototyped a similar approach and introduced a
NamespaceAwarePropertySource. It is basically an interface that has a
getNamespace() method that returns a prefix for the keys. I think that
it would be nice if these two approaches. My prototype didn't try to
call a security manager, but with this commit it would be easy to add.
On the other hand it uses a ServiceLoader approach to automatically find
all NamespaceAwarePropertySources. Do you think this would be a good
addition for Tomcat?
Regards
Felix
> +
> +/**
> + * Obtain a property value, checking that code associated with the
> + * provided class loader has permission to access the property. If
> the
> + * {@code classLoader} is {@code null} or if {@code classLoader} does
> + * not implement {@link PermissionCheck} then the property value
> will be
> + * looked up without a call to
> + * {@link PermissionCheck#check(java.security.Permission)}
> + *
> + * @param key The key of the requested property
> + * @param classLoader The class loader associated with the code
> that
> + * trigger the property lookup
> + * @return The property value or {@code null} if it could not be
> found
> + * or if {@link
> PermissionCheck#check(java.security.Permission)}
> + * fails
> + */
> +public String getProperty(String key, ClassLoader classLoader);
> +}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
