From 91ec81f2f279c7b0453c819e54933cd0617e8494 Mon Sep 17 00:00:00 2001 From: remm Date: Mon, 13 Aug 2007 18:13:47 +0000 Subject: [PATCH] - Experiment with reporting warnings for unmatched elements and attributes, based on Bill's ideas. - The NIO HTTP protocol handler does not seem very reporting friendly at the moment. - The change should be rather safe, if it causes problems that cannot be fixed easily I will revert it. git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc6.0.x/trunk@565464 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/connector/Connector.java | 40 +++++++---- java/org/apache/catalina/startup/Catalina.java | 12 +++- java/org/apache/catalina/startup/Embedded.java | 2 +- .../catalina/startup/SetAllPropertiesRule.java | 11 ++- .../catalina/startup/SetContextPropertiesRule.java | 8 ++- java/org/apache/coyote/ajp/AjpAprProtocol.java | 16 ----- java/org/apache/coyote/ajp/AjpProtocol.java | 16 ----- .../apache/coyote/http11/Http11AprProtocol.java | 25 +------ java/org/apache/coyote/http11/Http11Protocol.java | 14 ---- .../org/apache/tomcat/util/IntrospectionUtils.java | 8 ++- java/org/apache/tomcat/util/digester/Digester.java | 82 ++++++++++++++++++++++ .../tomcat/util/digester/SetPropertiesRule.java | 8 ++- .../tomcat/util/digester/SetPropertyRule.java | 9 ++- 13 files changed, 159 insertions(+), 92 deletions(-) diff --git a/java/org/apache/catalina/connector/Connector.java b/java/org/apache/catalina/connector/Connector.java index a55c002b8..345ee6435 100644 --- a/java/org/apache/catalina/connector/Connector.java +++ b/java/org/apache/catalina/connector/Connector.java @@ -318,6 +318,20 @@ public class Connector if (replacements.get(name) != null) { repl = (String) replacements.get(name); } + if (!IntrospectionUtils.setProperty(protocolHandler, repl, value)) { + log.warn("Property " + name + " not found on the protocol handler."); + } + } + + + /** + * Set a configured property. + */ + public void setPropertyInternal(String name, String value) { + String repl = name; + if (replacements.get(name) != null) { + repl = (String) replacements.get(name); + } IntrospectionUtils.setProperty(protocolHandler, repl, value); } @@ -388,7 +402,7 @@ public class Connector public void setAllowTrace(boolean allowTrace) { this.allowTrace = allowTrace; - setProperty("allowTrace", String.valueOf(allowTrace)); + setPropertyInternal("allowTrace", String.valueOf(allowTrace)); } @@ -466,7 +480,7 @@ public class Connector public void setEmptySessionPath(boolean emptySessionPath) { this.emptySessionPath = emptySessionPath; - setProperty("emptySessionPath", String.valueOf(emptySessionPath)); + setPropertyInternal("emptySessionPath", String.valueOf(emptySessionPath)); } @@ -489,7 +503,7 @@ public class Connector public void setEnableLookups(boolean enableLookups) { this.enableLookups = enableLookups; - setProperty("enableLookups", String.valueOf(enableLookups)); + setPropertyInternal("enableLookups", String.valueOf(enableLookups)); } @@ -559,7 +573,7 @@ public class Connector public void setMaxSavePostSize(int maxSavePostSize) { this.maxSavePostSize = maxSavePostSize; - setProperty("maxSavePostSize", String.valueOf(maxSavePostSize)); + setPropertyInternal("maxSavePostSize", String.valueOf(maxSavePostSize)); } @@ -581,7 +595,7 @@ public class Connector public void setPort(int port) { this.port = port; - setProperty("port", String.valueOf(port)); + setPropertyInternal("port", String.valueOf(port)); } @@ -745,7 +759,7 @@ public class Connector if(proxyName != null && proxyName.length() > 0) { this.proxyName = proxyName; - setProperty("proxyName", proxyName); + setPropertyInternal("proxyName", proxyName); } else { this.proxyName = null; removeProperty("proxyName"); @@ -772,7 +786,7 @@ public class Connector public void setProxyPort(int proxyPort) { this.proxyPort = proxyPort; - setProperty("proxyPort", String.valueOf(proxyPort)); + setPropertyInternal("proxyPort", String.valueOf(proxyPort)); } @@ -797,7 +811,7 @@ public class Connector public void setRedirectPort(int redirectPort) { this.redirectPort = redirectPort; - setProperty("redirectPort", String.valueOf(redirectPort)); + setPropertyInternal("redirectPort", String.valueOf(redirectPort)); } @@ -846,7 +860,7 @@ public class Connector public void setSecure(boolean secure) { this.secure = secure; - setProperty("secure", Boolean.toString(secure)); + setPropertyInternal("secure", Boolean.toString(secure)); } /** @@ -867,7 +881,7 @@ public class Connector public void setURIEncoding(String URIEncoding) { this.URIEncoding = URIEncoding; - setProperty("uRIEncoding", URIEncoding); + setPropertyInternal("uRIEncoding", URIEncoding); } @@ -890,7 +904,7 @@ public class Connector public void setUseBodyEncodingForURI(boolean useBodyEncodingForURI) { this.useBodyEncodingForURI = useBodyEncodingForURI; - setProperty + setPropertyInternal ("useBodyEncodingForURI", String.valueOf(useBodyEncodingForURI)); } @@ -918,7 +932,7 @@ public class Connector */ public void setXpoweredBy(boolean xpoweredBy) { this.xpoweredBy = xpoweredBy; - setProperty("xpoweredBy", String.valueOf(xpoweredBy)); + setPropertyInternal("xpoweredBy", String.valueOf(xpoweredBy)); } /** @@ -929,7 +943,7 @@ public class Connector */ public void setUseIPVHosts(boolean useIPVHosts) { this.useIPVHosts = useIPVHosts; - setProperty("useIPVHosts", String.valueOf(useIPVHosts)); + setPropertyInternal("useIPVHosts", String.valueOf(useIPVHosts)); } /** diff --git a/java/org/apache/catalina/startup/Catalina.java b/java/org/apache/catalina/startup/Catalina.java index ebb794567..415f75a58 100644 --- a/java/org/apache/catalina/startup/Catalina.java +++ b/java/org/apache/catalina/startup/Catalina.java @@ -21,10 +21,14 @@ package org.apache.catalina.startup; import java.io.File; import java.io.FileInputStream; -import java.io.InputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + import org.apache.catalina.Container; import org.apache.catalina.Lifecycle; import org.apache.catalina.LifecycleException; @@ -260,6 +264,12 @@ public class Catalina extends Embedded { // Initialize the digester Digester digester = new Digester(); digester.setValidating(false); + digester.setRulesValidation(true); + HashMap> fakeAttributes = new HashMap>(); + ArrayList attrs = new ArrayList(); + attrs.add("className"); + fakeAttributes.put(Object.class, attrs); + digester.setFakeAttributes(fakeAttributes); digester.setClassLoader(StandardServer.class.getClassLoader()); // Configure the actions we will be using diff --git a/java/org/apache/catalina/startup/Embedded.java b/java/org/apache/catalina/startup/Embedded.java index d3e21ff55..c1d8760d4 100644 --- a/java/org/apache/catalina/startup/Embedded.java +++ b/java/org/apache/catalina/startup/Embedded.java @@ -427,7 +427,7 @@ public class Embedded extends StandardService implements Lifecycle { connector = new Connector(); connector.setScheme("https"); connector.setSecure(true); - connector.setProperty("SSLEnabled","true"); + connector.setPropertyInternal("SSLEnabled","true"); // FIXME !!!! SET SSL PROPERTIES } else { connector = new Connector(protocol); diff --git a/java/org/apache/catalina/startup/SetAllPropertiesRule.java b/java/org/apache/catalina/startup/SetAllPropertiesRule.java index f971b65a9..b0518b510 100644 --- a/java/org/apache/catalina/startup/SetAllPropertiesRule.java +++ b/java/org/apache/catalina/startup/SetAllPropertiesRule.java @@ -62,8 +62,15 @@ public class SetAllPropertiesRule extends Rule { name = attributes.getQName(i); } String value = attributes.getValue(i); - if ( !excludes.containsKey(name)) - IntrospectionUtils.setProperty(digester.peek(), name, value); + if ( !excludes.containsKey(name)) { + if (!digester.isFakeAttribute(digester.peek(), name) + && !IntrospectionUtils.setProperty(digester.peek(), name, value) + && digester.getRulesValidation()) { + digester.getLogger().warn("[SetAllPropertiesRule]{" + digester.getMatch() + + "} Setting property '" + name + "' to '" + + value + "' did not find a matching property."); + } + } } } diff --git a/java/org/apache/catalina/startup/SetContextPropertiesRule.java b/java/org/apache/catalina/startup/SetContextPropertiesRule.java index 3ecb6d74f..c9b3094f5 100644 --- a/java/org/apache/catalina/startup/SetContextPropertiesRule.java +++ b/java/org/apache/catalina/startup/SetContextPropertiesRule.java @@ -60,7 +60,13 @@ public class SetContextPropertiesRule extends Rule { continue; } String value = attributes.getValue(i); - IntrospectionUtils.setProperty(digester.peek(), name, value); + if (!digester.isFakeAttribute(digester.peek(), name) + && !IntrospectionUtils.setProperty(digester.peek(), name, value) + && digester.getRulesValidation()) { + digester.getLogger().warn("[SetContextPropertiesRule]{" + digester.getMatch() + + "} Setting property '" + name + "' to '" + + value + "' did not find a matching property."); + } } } diff --git a/java/org/apache/coyote/ajp/AjpAprProtocol.java b/java/org/apache/coyote/ajp/AjpAprProtocol.java index 96cac28f9..e31bbfde5 100644 --- a/java/org/apache/coyote/ajp/AjpAprProtocol.java +++ b/java/org/apache/coyote/ajp/AjpAprProtocol.java @@ -137,22 +137,6 @@ public class AjpAprProtocol /** - * Set a property. - */ - public void setProperty(String name, String value) { - setAttribute(name, value); - } - - - /** - * Get a property - */ - public String getProperty(String name) { - return (String) getAttribute(name); - } - - - /** * The adapter, used to call the connector */ public void setAdapter(Adapter adapter) { diff --git a/java/org/apache/coyote/ajp/AjpProtocol.java b/java/org/apache/coyote/ajp/AjpProtocol.java index e844a2e23..6b129c7b0 100644 --- a/java/org/apache/coyote/ajp/AjpProtocol.java +++ b/java/org/apache/coyote/ajp/AjpProtocol.java @@ -137,22 +137,6 @@ public class AjpProtocol /** - * Set a property. - */ - public void setProperty(String name, String value) { - setAttribute(name, value); - } - - - /** - * Get a property - */ - public String getProperty(String name) { - return (String) getAttribute(name); - } - - - /** * The adapter, used to call the connector */ public void setAdapter(Adapter adapter) { diff --git a/java/org/apache/coyote/http11/Http11AprProtocol.java b/java/org/apache/coyote/http11/Http11AprProtocol.java index e846b8d13..20cf364a2 100644 --- a/java/org/apache/coyote/http11/Http11AprProtocol.java +++ b/java/org/apache/coyote/http11/Http11AprProtocol.java @@ -90,20 +90,6 @@ public class Http11AprProtocol implements ProtocolHandler, MBeanRegistration { } /** - * Set a property. - */ - public void setProperty(String name, String value) { - setAttribute(name, value); - } - - /** - * Get a property - */ - public String getProperty(String name) { - return (String)getAttribute(name); - } - - /** * The adapter, used to call the connector. */ protected Adapter adapter; @@ -325,14 +311,9 @@ public class Http11AprProtocol implements ProtocolHandler, MBeanRegistration { public void setRestrictedUserAgents(String valueS) { restrictedUserAgents = valueS; } - public String getProtocol() { - return getProperty("protocol"); - } - - public void setProtocol( String k ) { - setSecure(true); - setAttribute("protocol", k); - } + protected String protocol = null; + public String getProtocol() { return protocol; } + public void setProtocol(String protocol) { setSecure(true); this.protocol = protocol; } /** * Maximum number of requests which can be performed over a keepalive diff --git a/java/org/apache/coyote/http11/Http11Protocol.java b/java/org/apache/coyote/http11/Http11Protocol.java index 0d1aac856..064a78e1e 100644 --- a/java/org/apache/coyote/http11/Http11Protocol.java +++ b/java/org/apache/coyote/http11/Http11Protocol.java @@ -103,20 +103,6 @@ public class Http11Protocol /** - * Set a property. - */ - public void setProperty(String name, String value) { - setAttribute(name, value); - } - - /** - * Get a property - */ - public String getProperty(String name) { - return (String)getAttribute(name); - } - - /** * Pass config info */ public void setAttribute(String name, Object value) { diff --git a/java/org/apache/tomcat/util/IntrospectionUtils.java b/java/org/apache/tomcat/util/IntrospectionUtils.java index 6f497b85b..b1134befb 100644 --- a/java/org/apache/tomcat/util/IntrospectionUtils.java +++ b/java/org/apache/tomcat/util/IntrospectionUtils.java @@ -258,7 +258,7 @@ public final class IntrospectionUtils { * int or boolean we'll convert value to the right type before) - that means * you can have setDebug(1). */ - public static void setProperty(Object o, String name, String value) { + public static boolean setProperty(Object o, String name, String value) { if (dbg > 1) d("setProperty(" + o.getClass() + " " + name + "=" + value + ")"); @@ -275,7 +275,7 @@ public final class IntrospectionUtils { && "java.lang.String".equals(paramT[0].getName())) { methods[i].invoke(o, new Object[] { value }); - return; + return true; } } @@ -328,7 +328,7 @@ public final class IntrospectionUtils { if (ok) { methods[i].invoke(o, params); - return; + return true; } } @@ -344,6 +344,7 @@ public final class IntrospectionUtils { params[0] = name; params[1] = value; setPropertyMethod.invoke(o, params); + return true; } } catch (IllegalArgumentException ex2) { @@ -367,6 +368,7 @@ public final class IntrospectionUtils { if (dbg > 1) ie.printStackTrace(); } + return false; } public static Object getProperty(Object o, String name) { diff --git a/java/org/apache/tomcat/util/digester/Digester.java b/java/org/apache/tomcat/util/digester/Digester.java index 2c5d8c3d7..13371dd63 100644 --- a/java/org/apache/tomcat/util/digester/Digester.java +++ b/java/org/apache/tomcat/util/digester/Digester.java @@ -312,6 +312,19 @@ public class Digester extends DefaultHandler { protected boolean validating = false; + + /** + * Warn on missing attributes and elements. + */ + protected boolean rulesValidation = false; + + + /** + * Fake attributes map (attributes are often used for object creation). + */ + protected Map> fakeAttributes = null; + + /** * The Log to which most logging calls will be made. */ @@ -889,6 +902,72 @@ public class Digester extends DefaultHandler { /** + * Return the rules validation flag. + */ + public boolean getRulesValidation() { + + return (this.rulesValidation); + + } + + + /** + * Set the rules validation flag. This must be called before + * parse() is called the first time. + * + * @param rulesValidation The new rules validation flag. + */ + public void setRulesValidation(boolean rulesValidation) { + + this.rulesValidation = rulesValidation; + + } + + + /** + * Return the fake attributes list. + */ + public Map> getFakeAttributes() { + + return (this.fakeAttributes); + + } + + + /** + * Determine if an attribute is a fake attribute. + */ + public boolean isFakeAttribute(Object object, String name) { + + if (fakeAttributes == null) { + return false; + } + List result = fakeAttributes.get(object.getClass()); + if (result == null) { + result = fakeAttributes.get(Object.class); + } + if (result == null) { + return false; + } else { + return result.contains(name); + } + + } + + + /** + * Set the fake attributes. + * + * @param fakeAttributes The new fake attributes. + */ + public void setFakeAttributes(Map> fakeAttributes) { + + this.fakeAttributes = fakeAttributes; + + } + + + /** * Return the XMLReader to be used for parsing the input document. * * FIX ME: there is a bug in JAXP/XERCES that prevent the use of a @@ -1038,6 +1117,9 @@ public class Digester extends DefaultHandler { if (debug) { log.debug(" No rules found matching '" + match + "'."); } + if (rulesValidation) { + log.warn(" No rules found matching '" + match + "'."); + } } // Recover the body text from the surrounding element diff --git a/java/org/apache/tomcat/util/digester/SetPropertiesRule.java b/java/org/apache/tomcat/util/digester/SetPropertiesRule.java index f49ddbda5..e68bbf1d2 100644 --- a/java/org/apache/tomcat/util/digester/SetPropertiesRule.java +++ b/java/org/apache/tomcat/util/digester/SetPropertiesRule.java @@ -205,7 +205,13 @@ public class SetPropertiesRule extends Rule { "} Setting property '" + name + "' to '" + value + "'"); } - IntrospectionUtils.setProperty(top, name, value); + if (!digester.isFakeAttribute(top, name) + && !IntrospectionUtils.setProperty(top, name, value) + && digester.getRulesValidation()) { + digester.log.warn("[SetPropertiesRule]{" + digester.match + + "} Setting property '" + name + "' to '" + + value + "' did not find a matching property."); + } } } diff --git a/java/org/apache/tomcat/util/digester/SetPropertyRule.java b/java/org/apache/tomcat/util/digester/SetPropertyRule.java index 60d8e9060..f4b133ed7 100644 --- a/java/org/apache/tomcat/util/digester/SetPropertyRule.java +++ b/java/org/apache/tomcat/util/digester/SetPropertyRule.java @@ -125,8 +125,13 @@ public class SetPropertyRule extends Rule { } // Set the property (with conversion as necessary) - // FIXME: Exception if property doesn't exist ? - IntrospectionUtils.setProperty(top, actualName, actualValue); + if (!digester.isFakeAttribute(top, actualName) + && !IntrospectionUtils.setProperty(top, actualName, actualValue) + && digester.getRulesValidation()) { + digester.log.warn("[SetPropertyRule]{" + digester.match + + "} Setting property '" + name + "' to '" + + value + "' did not find a matching property."); + } } -- 2.11.0