Skip to content

Commit

Permalink
Fix BZ 65736 replace forceString with a String setter lookup
Browse files Browse the repository at this point in the history
  • Loading branch information
markt-asf committed May 9, 2022
1 parent 7ce1b4c commit 48dd609
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 153 deletions.
77 changes: 21 additions & 56 deletions java/org/apache/naming/factory/BeanFactory.java
Expand Up @@ -19,13 +19,9 @@
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;
Expand All @@ -34,6 +30,8 @@
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;

Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -125,44 +125,14 @@ public Object getObjectInstance(Object obj, Name name, Context nameCtx, Hashtabl

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()) {

Expand All @@ -180,28 +150,13 @@ public Object getObjectInstance(Object obj, Name name, Context nameCtx, Hashtabl

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;
Expand All @@ -221,12 +176,22 @@ public Object getObjectInstance(Object obj, Name name, Context nameCtx, Hashtabl
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 {
Expand Down
1 change: 1 addition & 0 deletions java/org/apache/naming/factory/LocalStrings.properties
Expand Up @@ -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}]
Expand Down
67 changes: 67 additions & 0 deletions 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());
}
}
41 changes: 41 additions & 0 deletions 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);
}
}
}
6 changes: 6 additions & 0 deletions webapps/docs/changelog.xml
Expand Up @@ -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.
Expand Down
112 changes: 15 additions & 97 deletions webapps/docs/jndi-resources-howto.xml
Expand Up @@ -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>


Expand Down

0 comments on commit 48dd609

Please sign in to comment.