You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2022/05/09 18:23:53 UTC
[tomcat] branch 8.5.x updated: Fix BZ 65736 replace forceString with a String setter lookup
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 48dd609fd1 Fix BZ 65736 replace forceString with a String setter lookup
48dd609fd1 is described below
commit 48dd609fd193dbe8dd94fd231c45d987da6c359f
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Mar 30 12:37:26 2022 +0100
Fix BZ 65736 replace forceString with a String setter lookup
---
java/org/apache/naming/factory/BeanFactory.java | 77 ++++----------
.../apache/naming/factory/LocalStrings.properties | 1 +
.../org/apache/naming/factory/TestBeanFactory.java | 67 ++++++++++++
test/org/apache/naming/factory/TesterBean.java | 41 ++++++++
webapps/docs/changelog.xml | 6 ++
webapps/docs/jndi-resources-howto.xml | 112 +++------------------
6 files changed, 151 insertions(+), 153 deletions(-)
diff --git a/java/org/apache/naming/factory/BeanFactory.java b/java/org/apache/naming/factory/BeanFactory.java
index 7a42991b05..1f207cd712 100644
--- a/java/org/apache/naming/factory/BeanFactory.java
+++ b/java/org/apache/naming/factory/BeanFactory.java
@@ -19,13 +19,9 @@ package org.apache.naming.factory;
import java.beans.BeanInfo;
import java.beans.Introspector;
import java.beans.PropertyDescriptor;
-import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Enumeration;
-import java.util.HashMap;
import java.util.Hashtable;
-import java.util.Locale;
-import java.util.Map;
import javax.naming.Context;
import javax.naming.Name;
@@ -34,6 +30,8 @@ import javax.naming.RefAddr;
import javax.naming.Reference;
import javax.naming.spi.ObjectFactory;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
import org.apache.naming.ResourceRef;
import org.apache.naming.StringManager;
@@ -92,6 +90,8 @@ public class BeanFactory implements ObjectFactory {
private static final StringManager sm = StringManager.getManager(BeanFactory.class);
+ private final Log log = LogFactory.getLog(BeanFactory.class); // Not static
+
/**
* Create a new Bean instance.
*
@@ -125,44 +125,14 @@ public class BeanFactory implements ObjectFactory {
Object bean = beanClass.getConstructor().newInstance();
- /* Look for properties with explicitly configured setter */
+ // Look for the removed forceString option
RefAddr ra = ref.get("forceString");
- Map<String, Method> forced = new HashMap<>();
- String value;
-
if (ra != null) {
- value = (String)ra.getContent();
- Class<?> paramTypes[] = new Class[1];
- paramTypes[0] = String.class;
- String setterName;
- int index;
-
- /* Items are given as comma separated list */
- for (String param: value.split(",")) {
- param = param.trim();
- /* A single item can either be of the form name=method
- * or just a property name (and we will use a standard
- * setter) */
- index = param.indexOf('=');
- if (index >= 0) {
- setterName = param.substring(index + 1).trim();
- param = param.substring(0, index).trim();
- } else {
- setterName = "set" +
- param.substring(0, 1).toUpperCase(Locale.ENGLISH) +
- param.substring(1);
- }
- try {
- forced.put(param, beanClass.getMethod(setterName, paramTypes));
- } catch (NoSuchMethodException|SecurityException ex) {
- throw new NamingException
- ("Forced String setter " + setterName +
- " not found for property " + param);
- }
- }
+ log.warn(sm.getString("beanFactory.noForceString"));
}
Enumeration<RefAddr> e = ref.getAll();
+ String value;
while (e.hasMoreElements()) {
@@ -180,28 +150,13 @@ public class BeanFactory implements ObjectFactory {
Object[] valueArray = new Object[1];
- /* Shortcut for properties with explicitly configured setter */
- Method method = forced.get(propName);
- if (method != null) {
- valueArray[0] = value;
- try {
- method.invoke(bean, valueArray);
- } catch (IllegalAccessException|
- IllegalArgumentException|
- InvocationTargetException ex) {
- throw new NamingException
- ("Forced String setter " + method.getName() +
- " threw exception for property " + propName);
- }
- continue;
- }
-
int i = 0;
for (i = 0; i < pda.length; i++) {
if (pda[i].getName().equals(propName)) {
Class<?> propType = pda[i].getPropertyType();
+ Method setProp = pda[i].getWriteMethod();
if (propType.equals(String.class)) {
valueArray[0] = value;
@@ -221,12 +176,22 @@ public class BeanFactory implements ObjectFactory {
valueArray[0] = Double.valueOf(value);
} else if (propType.equals(Boolean.class) || propType.equals(boolean.class)) {
valueArray[0] = Boolean.valueOf(value);
+ } else if (setProp != null) {
+ // This is a Tomcat specific extension and is not part of the
+ // Java Bean specification.
+ String setterName = setProp.getName();
+ try {
+ setProp = bean.getClass().getMethod(setterName, String.class);
+ valueArray[0] = value;
+ } catch (NoSuchMethodException nsme) {
+ throw new NamingException(sm.getString(
+ "beanFactory.noStringConversion", propName, propType.getName()));
+ }
} else {
- throw new NamingException(
- sm.getString("beanFactory.noStringConversion", propName, propType.getName()));
+ throw new NamingException(sm.getString(
+ "beanFactory.noStringConversion", propName, propType.getName()));
}
- Method setProp = pda[i].getWriteMethod();
if (setProp != null) {
setProp.invoke(bean, valueArray);
} else {
diff --git a/java/org/apache/naming/factory/LocalStrings.properties b/java/org/apache/naming/factory/LocalStrings.properties
index a4d1d570f7..cbd43355ee 100644
--- a/java/org/apache/naming/factory/LocalStrings.properties
+++ b/java/org/apache/naming/factory/LocalStrings.properties
@@ -14,6 +14,7 @@
# limitations under the License.
beanFactory.classNotFound=Class not found: [{0}]
+beanFactory.noForceString=The forceString option has been removed as a security hardening measure. Instead, if the setter method doesn't use String, a primitive or a primitive wrapper, the factory will look for a method with the same name as the setter that accepts a String and use that if found.
beanFactory.noSetMethod=No set method found for property [{0}]
beanFactory.noStringConversion=String conversion for property [{0}] of type [{1}] not available
beanFactory.readOnlyProperty=Write not allowed for property [{0}]
diff --git a/test/org/apache/naming/factory/TestBeanFactory.java b/test/org/apache/naming/factory/TestBeanFactory.java
new file mode 100644
index 0000000000..ec85ff6136
--- /dev/null
+++ b/test/org/apache/naming/factory/TestBeanFactory.java
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.naming.factory;
+
+import javax.naming.StringRefAddr;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.naming.ResourceRef;
+
+public class TestBeanFactory {
+
+ private static final String IP_ADDRESS = "127.0.0.1";
+
+ @Test
+ public void testForceStringAlternativeWithout() throws Exception {
+ doTestForceStringAlternatove(false);
+ }
+
+
+ @Test
+ public void testForceStringAlternativeWith() throws Exception {
+ doTestForceStringAlternatove(true);
+ }
+
+
+ private void doTestForceStringAlternatove(boolean useForceString) throws Exception {
+
+ // Create the resource definition
+ ResourceRef resourceRef = new ResourceRef(TesterBean.class.getName(), null, null, null, false);
+ StringRefAddr server = new StringRefAddr("server", IP_ADDRESS);
+ resourceRef.add(server);
+ if (useForceString) {
+ StringRefAddr force = new StringRefAddr("forceString", "server");
+ resourceRef.add(force);
+ }
+
+ // Create the factory
+ BeanFactory factory = new BeanFactory();
+
+ // Use the factory to create the resource from the definition
+ Object obj = factory.getObjectInstance(resourceRef, null, null, null);
+
+ // Check the correct type was created
+ Assert.assertNotNull(obj);
+ Assert.assertEquals(obj.getClass(), TesterBean.class);
+ // Check the server field was set
+ TesterBean result = (TesterBean) obj;
+ Assert.assertNotNull(result.getServer());
+ Assert.assertEquals(IP_ADDRESS, result.getServer().getHostAddress());
+ }
+}
diff --git a/test/org/apache/naming/factory/TesterBean.java b/test/org/apache/naming/factory/TesterBean.java
new file mode 100644
index 0000000000..d2beef275c
--- /dev/null
+++ b/test/org/apache/naming/factory/TesterBean.java
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.naming.factory;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+public class TesterBean {
+
+ private InetAddress server;
+
+ public InetAddress getServer() {
+ return server;
+ }
+
+ public void setServer(InetAddress server) {
+ this.server = server;
+ }
+
+ public void setServer(String server) {
+ try {
+ this.server = InetAddress.getByName(server);
+ } catch (UnknownHostException e) {
+ throw new IllegalArgumentException(e);
+ }
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 743cdc76bd..77dff10b0a 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -107,6 +107,12 @@
<section name="Tomcat 8.5.79 (schultz)" rtext="in development">
<subsection name="Catalina">
<changelog>
+ <fix>
+ <bug>65736</bug>: Disable the <code>forceString</code> option for the
+ JNDI <code>BeanFactory</code> and replace it with an automatic search
+ for an alternative setter with the same name that accepts a
+ <code>String</code>. This is a security hardening measure. (markt)
+ </fix>
<scode>
<bug>65853</bug>: Refactor the <code>CsrfPreventionFilter</code> to make
it easier for sub-classes to modify the nonce generation and storage.
diff --git a/webapps/docs/jndi-resources-howto.xml b/webapps/docs/jndi-resources-howto.xml
index d8a20fcf24..391e565e64 100644
--- a/webapps/docs/jndi-resources-howto.xml
+++ b/webapps/docs/jndi-resources-howto.xml
@@ -328,103 +328,21 @@ writer.println("foo = " + bean.getFoo() + ", bar = " +
<code>foo</code> property (although we could have), the bean will
contain whatever default value is set up by its constructor.</p>
- <p>Some beans have properties with types that cannot automatically be
- converted from a string value. Setting such properties using the Tomcat
- BeanFactory will fail with a NamingException. In cases were those beans
- provide methods to set the properties from a string value, the Tomcat
- BeanFactory can be configured to use these methods. The configuration is
- done with the <code>forceString</code> attribute.</p>
-
- <p>Assume our bean looks like this:</p>
-
-<source><![CDATA[package com.mycompany;
-
-import java.net.InetAddress;
-import java.net.UnknownHostException;
-
-public class MyBean2 {
-
- private InetAddress local = null;
-
- public InetAddress getLocal() {
- return local;
- }
-
- public void setLocal(InetAddress ip) {
- local = ip;
- }
-
- public void setLocal(String localHost) {
- try {
- local = InetAddress.getByName(localHost);
- } catch (UnknownHostException ex) {
- }
- }
-
- private InetAddress remote = null;
-
- public InetAddress getRemote() {
- return remote;
- }
-
- public void setRemote(InetAddress ip) {
- remote = ip;
- }
-
- public void host(String remoteHost) {
- try {
- remote = InetAddress.getByName(remoteHost);
- } catch (UnknownHostException ex) {
- }
- }
-
-}]]></source>
-
- <p>The bean has two properties, both are of type <code>InetAddress</code>.
- The first property <code>local</code> has an additional setter taking a
- string argument. By default the Tomcat BeanFactory would try to use the
- automatically detected setter with the same argument type as the property
- type and then throw a NamingException, because it is not prepared to convert
- the given string attribute value to <code>InetAddress</code>.
- We can tell the Tomcat BeanFactory to use the other setter like that:</p>
-
-<source><![CDATA[<Context ...>
- ...
- <Resource name="bean/MyBeanFactory" auth="Container"
- type="com.mycompany.MyBean2"
- factory="org.apache.naming.factory.BeanFactory"
- forceString="local"
- local="localhost"/>
- ...
-</Context>]]></source>
-
- <p>The bean property <code>remote</code> can also be set from a string,
- but one has to use the non-standard method name <code>host</code>.
- To set <code>local</code> and <code>remote</code> use the following
- configuration:</p>
-
-<source><![CDATA[<Context ...>
- ...
- <Resource name="bean/MyBeanFactory" auth="Container"
- type="com.mycompany.MyBean2"
- factory="org.apache.naming.factory.BeanFactory"
- forceString="local,remote=host"
- local="localhost"
- remote="tomcat.apache.org"/>
- ...
-</Context>]]></source>
-
- <p>Multiple property descriptions can be combined in
- <code>forceString</code> by concatenation with comma as a separator.
- Each property description consists of either only the property name
- in which case the BeanFactory calls the setter method. Or it consist
- of <code>name=method</code> in which case the property named
- <code>name</code> is set by calling method <code>method</code>.
- For properties of types <code>String</code> or of primitive type
- or of their associated primitive wrapper classes using
- <code>forceString</code> is not needed. The correct setter will be
- automatically detected and argument conversion will be applied.</p>
-
+ <p>If the bean property is of type <code>String</code>, the BeanFactory
+ will call the property setter using the provided property value. If the
+ bean property type is a primitive or a primitive wrapper, the the
+ BeanFactory will convert the value to the appropriate primitive or
+ primitive wrapper and then use that value when calling the setter. Some
+ beans have properties with types that cannot automatically be converted
+ from <code>String</code>. If the bean provides an alternative setter with
+ the same name that does take a <code>String</code>, the BeanFactory will
+ attempt to use that setter. If the BeanFactory cannot use the value or
+ perform an appropriate conversion, setting the property will fail with a
+ NamingException.</p>
+
+ <p>The <code>forceString</code> property available in earlier Tomcat
+ releases has been removed as a security hardening measure.</p>
+
</subsection>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org