Author: carnold
Date: Fri Aug 10 15:05:16 2007
New Revision: 564779
URL: http://svn.apache.org/viewvc?view=rev&rev=564779
Log:
Bug 34875: XML and HTMLLayout do not always escape special characters
Modified:
logging/log4j/branches/v1_2-branch/src/changes/changes.xml
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/HTMLLayout.java
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/helpers/Transform.java
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/xml/XMLLayout.java
logging/log4j/branches/v1_2-branch/tests/src/java/org/apache/log4j/HTMLLayoutTest.java
logging/log4j/branches/v1_2-branch/tests/src/java/org/apache/log4j/xml/XMLLayoutTest.java
Modified: logging/log4j/branches/v1_2-branch/src/changes/changes.xml
URL:
http://svn.apache.org/viewvc/logging/log4j/branches/v1_2-branch/src/changes/changes.xml?view=diff&rev=564779&r1=564778&r2=564779
==============================================================================
--- logging/log4j/branches/v1_2-branch/src/changes/changes.xml (original)
+++ logging/log4j/branches/v1_2-branch/src/changes/changes.xml Fri Aug 10
15:05:16 2007
@@ -22,6 +22,7 @@
<body>
<release version="1.2.15" date="2007-06-27" description="SyslogAppender
enhancements, NTEventLogAppender and Maven build.">
+ <action action="fix" issue="34875">XML and HTMLLayout do not always
escape special characters.</action>
<action action="add" issue="43078">Optionally render MDC content in
XMLLayout</action>
<action action="fix" issue="42694">Typo in log4j.dtd concerning
threshold.</action>
<action action="fix" issue="42611">ERFATestCase fails on some
JDK's.</action>
Modified:
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/HTMLLayout.java
URL:
http://svn.apache.org/viewvc/logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/HTMLLayout.java?view=diff&rev=564779&r1=564778&r2=564779
==============================================================================
---
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/HTMLLayout.java
(original)
+++
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/HTMLLayout.java
Fri Aug 10 15:05:16 2007
@@ -135,27 +135,29 @@
sbuf.append(event.timeStamp - LoggingEvent.getStartTime());
sbuf.append("</td>" + Layout.LINE_SEP);
- sbuf.append("<td title=\"" + event.getThreadName() + " thread\">");
- sbuf.append(Transform.escapeTags(event.getThreadName()));
+ String escapedThread = Transform.escapeTags(event.getThreadName());
+ sbuf.append("<td title=\"" + escapedThread + " thread\">");
+ sbuf.append(escapedThread);
sbuf.append("</td>" + Layout.LINE_SEP);
sbuf.append("<td title=\"Level\">");
if (event.getLevel().equals(Level.DEBUG)) {
sbuf.append("<font color=\"#339933\">");
- sbuf.append(event.getLevel());
+ sbuf.append(Transform.escapeTags(String.valueOf(event.getLevel())));
sbuf.append("</font>");
}
else if(event.getLevel().isGreaterOrEqual(Level.WARN)) {
sbuf.append("<font color=\"#993300\"><strong>");
- sbuf.append(event.getLevel());
+ sbuf.append(Transform.escapeTags(String.valueOf(event.getLevel())));
sbuf.append("</strong></font>");
} else {
- sbuf.append(event.getLevel());
+ sbuf.append(Transform.escapeTags(String.valueOf(event.getLevel())));
}
sbuf.append("</td>" + Layout.LINE_SEP);
- sbuf.append("<td title=\"" + event.getLoggerName() + " category\">");
- sbuf.append(Transform.escapeTags(event.getLoggerName()));
+ String escapedLogger = Transform.escapeTags(event.getLoggerName());
+ sbuf.append("<td title=\"" + escapedLogger + " category\">");
+ sbuf.append(escapedLogger);
sbuf.append("</td>" + Layout.LINE_SEP);
if(locationInfo) {
Modified:
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/helpers/Transform.java
URL:
http://svn.apache.org/viewvc/logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/helpers/Transform.java?view=diff&rev=564779&r1=564778&r2=564779
==============================================================================
---
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/helpers/Transform.java
(original)
+++
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/helpers/Transform.java
Fri Aug 10 15:05:16 2007
@@ -33,18 +33,23 @@
/**
* This method takes a string which may contain HTML tags (ie,
- * <b>, <table>, etc) and replaces any '<' and '>'
+ * <b>, <table>, etc) and replaces any
+ * '<', '>' , '&' or '"'
* characters with respective predefined entity references.
*
* @param input The text to be converted.
- * @return The input string with the characters '<' and '>' replaced with
- * &lt; and &gt; respectively.
+ * @return The input string with the special characters replaced.
* */
- static public String escapeTags(String input) {
- //Check if the string is null or zero length -- if so, return
- //what was sent in.
+ static public String escapeTags(final String input) {
+ //Check if the string is null, zero length or devoid of special characters
+ // if so, return what was sent in.
- if( input == null || input.length() == 0 ) {
+ if(input == null
+ || input.length() == 0
+ || (input.indexOf('"') == -1 &&
+ input.indexOf('&') == -1 &&
+ input.indexOf('<') == -1 &&
+ input.indexOf('>') == -1)) {
return input;
}
@@ -57,12 +62,18 @@
int len = input.length();
for(int i=0; i < len; i++) {
ch = input.charAt(i);
- if(ch == '<') {
- buf.append("<");
+ if (ch > '>') {
+ buf.append(ch);
+ } else if(ch == '<') {
+ buf.append("<");
} else if(ch == '>') {
- buf.append(">");
+ buf.append(">");
+ } else if(ch == '&') {
+ buf.append("&");
+ } else if(ch == '"') {
+ buf.append(""");
} else {
- buf.append(ch);
+ buf.append(ch);
}
}
return buf.toString();
@@ -77,30 +88,26 @@
* section are the responsibility of the calling method.
* @param str The String that is inserted into an existing CDATA Section
within buf.
* */
- static public void appendEscapingCDATA(StringBuffer buf, String str) {
- if(str == null) {
- buf.append("");
- return;
- }
-
- int end = str.indexOf(CDATA_END);
- if (end < 0) {
- buf.append(str);
- return;
- }
-
- int start = 0;
- while (end > -1) {
- buf.append(str.substring(start,end));
- buf.append(CDATA_EMBEDED_END);
- start = end + CDATA_END_LEN;
- if (start < str.length()) {
- end = str.indexOf(CDATA_END, start);
- } else {
- return;
+ static public void appendEscapingCDATA(final StringBuffer buf,
+ final String str) {
+ if (str != null) {
+ int end = str.indexOf(CDATA_END);
+ if (end < 0) {
+ buf.append(str);
+ } else {
+ int start = 0;
+ while (end > -1) {
+ buf.append(str.substring(start, end));
+ buf.append(CDATA_EMBEDED_END);
+ start = end + CDATA_END_LEN;
+ if (start < str.length()) {
+ end = str.indexOf(CDATA_END, start);
+ } else {
+ return;
+ }
+ }
+ buf.append(str.substring(start));
+ }
}
- }
-
- buf.append(str.substring(start));
}
}
Modified:
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/xml/XMLLayout.java
URL:
http://svn.apache.org/viewvc/logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/xml/XMLLayout.java?view=diff&rev=564779&r1=564778&r2=564779
==============================================================================
---
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/xml/XMLLayout.java
(original)
+++
logging/log4j/branches/v1_2-branch/src/main/java/org/apache/log4j/xml/XMLLayout.java
Fri Aug 10 15:05:16 2007
@@ -115,7 +115,7 @@
/**
* Formats a [EMAIL PROTECTED] org.apache.log4j.spi.LoggingEvent} in
conformance with the log4j.dtd.
* */
- public String format(LoggingEvent event) {
+ public String format(final LoggingEvent event) {
// Reset working buffer. If the buffer is too large, then we need a new
// one in order to avoid the penalty of creating a large array.
@@ -128,13 +128,13 @@
// We yield to the \r\n heresy.
buf.append("<log4j:event logger=\"");
- buf.append(event.getLoggerName());
+ buf.append(Transform.escapeTags(event.getLoggerName()));
buf.append("\" timestamp=\"");
buf.append(event.timeStamp);
buf.append("\" level=\"");
- buf.append(event.getLevel());
+ buf.append(Transform.escapeTags(String.valueOf(event.getLevel())));
buf.append("\" thread=\"");
- buf.append(event.getThreadName());
+ buf.append(Transform.escapeTags(event.getThreadName()));
buf.append("\">\r\n");
buf.append("<log4j:message><![CDATA[");
@@ -167,7 +167,7 @@
buf.append("\" method=\"");
buf.append(Transform.escapeTags(locationInfo.getMethodName()));
buf.append("\" file=\"");
- buf.append(locationInfo.getFileName());
+ buf.append(Transform.escapeTags(locationInfo.getFileName()));
buf.append("\" line=\"");
buf.append(locationInfo.getLineNumber());
buf.append("\"/>\r\n");
@@ -186,7 +186,7 @@
buf.append("<log4j:data name=\"");
buf.append(Transform.escapeTags(key));
buf.append("\" value=\"");
- buf.append(Transform.escapeTags(val.toString()));
+ buf.append(Transform.escapeTags(String.valueOf(val)));
buf.append("\"/>\r\n");
}
}
Modified:
logging/log4j/branches/v1_2-branch/tests/src/java/org/apache/log4j/HTMLLayoutTest.java
URL:
http://svn.apache.org/viewvc/logging/log4j/branches/v1_2-branch/tests/src/java/org/apache/log4j/HTMLLayoutTest.java?view=diff&rev=564779&r1=564778&r2=564779
==============================================================================
---
logging/log4j/branches/v1_2-branch/tests/src/java/org/apache/log4j/HTMLLayoutTest.java
(original)
+++
logging/log4j/branches/v1_2-branch/tests/src/java/org/apache/log4j/HTMLLayoutTest.java
Fri Aug 10 15:05:16 2007
@@ -18,16 +18,14 @@
package org.apache.log4j;
import org.apache.log4j.spi.LoggingEvent;
-
import org.w3c.dom.Document;
-
import org.xml.sax.InputSource;
-import java.io.Reader;
-import java.io.StringReader;
-
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
+import java.io.Reader;
+import java.io.StringReader;
+import java.util.Hashtable;
/**
@@ -177,4 +175,66 @@
Layout.LINE_SEP + "<tr>",
result.substring(0, Layout.LINE_SEP.length() + 4));
}
+
+
+ /**
+ * Level with arbitrary toString value.
+ */
+ private static final class ProblemLevel extends Level {
+ /**
+ * Construct new instance.
+ * @param levelName level name, may not be null.
+ */
+ public ProblemLevel(final String levelName) {
+ super(6000, levelName, 6);
+ }
+ }
+
+ /**
+ * Tests problematic characters in multiple fields.
+ * @throws Exception if parser can not be constructed
+ * or source is not a valid XML document.
+ */
+ public void testProblemCharacters() throws Exception {
+ String problemName = "com.example.bar<>&\"'";
+ Logger logger = Logger.getLogger(problemName);
+ Level level = new ProblemLevel(problemName);
+ Exception ex = new IllegalArgumentException(problemName);
+ String threadName = Thread.currentThread().getName();
+ Thread.currentThread().setName(problemName);
+ NDC.push(problemName);
+ Hashtable mdcMap = MDC.getContext();
+ if (mdcMap != null) {
+ mdcMap.clear();
+ }
+ MDC.put(problemName, problemName);
+ LoggingEvent event =
+ new LoggingEvent(
+ problemName, logger, level, problemName, ex);
+ HTMLLayout layout = (HTMLLayout) createLayout();
+ String result = layout.format(event);
+ mdcMap = MDC.getContext();
+ if (mdcMap != null) {
+ mdcMap.clear();
+ }
+
+ Thread.currentThread().setName(threadName);
+
+ //
+ // do a little fixup to make output XHTML
+ //
+ StringBuffer buf = new StringBuffer(
+ "<!DOCTYPE table [<!ENTITY nbsp ' '>]><table>");
+ buf.append(result);
+ buf.append("</table>");
+ String doc = buf.toString();
+ for(int i = doc.lastIndexOf("<br>");
+ i != -1;
+ i = doc.lastIndexOf("<br>", i - 1)) {
+ buf.replace(i, i + 4, "<br/>");
+ }
+
+ parse(buf.toString());
+ }
+
}
Modified:
logging/log4j/branches/v1_2-branch/tests/src/java/org/apache/log4j/xml/XMLLayoutTest.java
URL:
http://svn.apache.org/viewvc/logging/log4j/branches/v1_2-branch/tests/src/java/org/apache/log4j/xml/XMLLayoutTest.java?view=diff&rev=564779&r1=564778&r2=564779
==============================================================================
---
logging/log4j/branches/v1_2-branch/tests/src/java/org/apache/log4j/xml/XMLLayoutTest.java
(original)
+++
logging/log4j/branches/v1_2-branch/tests/src/java/org/apache/log4j/xml/XMLLayoutTest.java
Fri Aug 10 15:05:16 2007
@@ -22,6 +22,7 @@
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
import org.apache.log4j.NDC;
+import org.apache.log4j.MDC;
import org.apache.log4j.spi.LoggingEvent;
import org.w3c.dom.Document;
@@ -32,6 +33,7 @@
import java.io.Reader;
import java.io.StringReader;
+import java.util.Hashtable;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
@@ -86,10 +88,10 @@
final Element element, final LoggingEvent event) {
assertEquals("log4j:event", element.getTagName());
assertEquals(
- "org.apache.log4j.xml.XMLLayoutTest", element.getAttribute("logger"));
+ event.getLoggerName(), element.getAttribute("logger"));
assertEquals(
Long.toString(event.timeStamp), element.getAttribute("timestamp"));
- assertEquals("INFO", element.getAttribute("level"));
+ assertEquals(event.getLevel().toString(), element.getAttribute("level"));
assertEquals(event.getThreadName(), element.getAttribute("thread"));
}
@@ -142,6 +144,31 @@
assertNull(messageNode.getNextSibling());
}
+ /**
+ * Checks a log4j:properties element against expectations.
+ * @param element element, may not be null.
+ * @param key key.
+ * @param value value.
+ */
+ private void checkPropertiesElement(
+ final Element element, final String key, final String value) {
+ assertEquals("log4j:properties", element.getTagName());
+
+ int childNodeCount = 0;
+ for(Node child = element.getFirstChild();
+ child != null;
+ child = child.getNextSibling()) {
+ if (child.getNodeType() == Node.ELEMENT_NODE) {
+ assertEquals("log4j:data", child.getNodeName());
+ Element childElement = (Element) child;
+ assertEquals(key, childElement.getAttribute("name"));
+ assertEquals(value, childElement.getAttribute("value"));
+ childNodeCount++;
+ }
+ }
+ assertEquals(1, childNodeCount);
+ }
+
/**
* Tests formatted results.
* @throws Exception if parser can not be constructed or source is not a
valid XML document.
@@ -308,4 +335,98 @@
XMLLayout layout = new XMLLayout();
layout.activateOptions();
}
+
+ /**
+ * Level with arbitrary toString value.
+ */
+ private static final class ProblemLevel extends Level {
+ /**
+ * Construct new instance.
+ * @param levelName level name, may not be null.
+ */
+ public ProblemLevel(final String levelName) {
+ super(6000, levelName, 6);
+ }
+ }
+
+ /**
+ * Tests problematic characters in multiple fields.
+ * @throws Exception if parser can not be constructed or source is not a
valid XML document.
+ */
+ public void testProblemCharacters() throws Exception {
+ String problemName = "com.example.bar<>&\"'";
+ Logger logger = Logger.getLogger(problemName);
+ Level level = new ProblemLevel(problemName);
+ Exception ex = new IllegalArgumentException(problemName);
+ String threadName = Thread.currentThread().getName();
+ Thread.currentThread().setName(problemName);
+ NDC.push(problemName);
+ Hashtable mdcMap = MDC.getContext();
+ if (mdcMap != null) {
+ mdcMap.clear();
+ }
+ MDC.put(problemName, problemName);
+ LoggingEvent event =
+ new LoggingEvent(
+ problemName, logger, level, problemName, ex);
+ XMLLayout layout = (XMLLayout) createLayout();
+ layout.setProperties(true);
+ String result = layout.format(event);
+ mdcMap = MDC.getContext();
+ if (mdcMap != null) {
+ mdcMap.clear();
+ }
+ Thread.currentThread().setName(threadName);
+
+ Element parsedResult = parse(result);
+ checkEventElement(parsedResult, event);
+
+ int childElementCount = 0;
+
+ for (
+ Node node = parsedResult.getFirstChild(); node != null;
+ node = node.getNextSibling()) {
+ switch (node.getNodeType()) {
+ case Node.ELEMENT_NODE:
+ childElementCount++;
+ switch(childElementCount) {
+ case 1:
+ checkMessageElement((Element) node, problemName);
+ break;
+
+ case 2:
+ checkNDCElement((Element) node, problemName);
+ break;
+
+ case 3:
+ checkThrowableElement((Element) node, ex);
+ break;
+
+ case 4:
+ checkPropertiesElement((Element) node, problemName, problemName);
+ break;
+
+ default:
+ fail("Unexpected element");
+ break;
+ }
+
+ break;
+
+ case Node.COMMENT_NODE:
+ break;
+
+ case Node.TEXT_NODE:
+
+ // should only be whitespace
+ break;
+
+ default:
+ fail("Unexpected node type");
+
+ break;
+ }
+ }
+ }
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
