This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 163004d96f Expand test coverage
163004d96f is described below
commit 163004d96fe6614d518ce18493c55149eadacff0
Author: remm <r...@apache.org>
AuthorDate: Sat Mar 8 00:00:00 2025 +0100
Expand test coverage
Cleanup.
---
.../catalina/valves/AbstractAccessLogValve.java | 10 ++-
.../catalina/valves/ExtendedAccessLogValve.java | 80 ++++++++++------------
.../valves/TestExtendedAccessLogValve.java | 16 ++++-
.../valves/TestExtendedAccessLogValveWrap.java | 34 ++++++---
4 files changed, 86 insertions(+), 54 deletions(-)
diff --git a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
index 3f39d609f0..c223e0de30 100644
--- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
+++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
@@ -1816,6 +1816,10 @@ public abstract class AbstractAccessLogValve extends
ValveBase implements Access
* encoding which may not be true for Tomcat so Tomcat uses the Java
\\uXXXX encoding.
*/
protected static void escapeAndAppend(String input, CharArrayWriter dest) {
+ escapeAndAppend(input, dest, false);
+ }
+
+ protected static void escapeAndAppend(String input, CharArrayWriter dest,
boolean escapeQuoteAsDouble) {
if (input == null || input.isEmpty()) {
dest.append('-');
return;
@@ -1851,7 +1855,11 @@ public abstract class AbstractAccessLogValve extends
ValveBase implements Access
dest.write(input, next, current - next);
}
next = current + 1;
- dest.append("\\\"");
+ if (escapeQuoteAsDouble) {
+ dest.append("\"\"");
+ } else {
+ dest.append("\\\"");
+ }
break;
// Don't output individual unchanged chars,
// write the sub string only when the first char to encode
diff --git a/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
b/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
index 745ba56889..42d10ec7ae 100644
--- a/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
+++ b/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
@@ -80,26 +80,6 @@ import org.apache.tomcat.util.ExceptionUtils;
* <li><code>x-H(scheme)</code>: getScheme</li>
* <li><code>x-H(secure)</code>: isSecure</li>
* </ul>
- * <p>
- * Log rotation can be on or off. This is dictated by the
<code>rotatable</code> property.
- * </p>
- * <p>
- * For UNIX users, another field called <code>checkExists</code> is also
available. If set to true, the log file's
- * existence will be checked before each logging. This way an external log
rotator can move the file somewhere and
- * Tomcat will start with a new file.
- * </p>
- * <p>
- * For JMX junkies, a public method called <code>rotate</code> has been made
available to allow you to tell this
- * instance to move the existing log file to somewhere else and start writing
a new log file.
- * </p>
- * <p>
- * Conditional logging is also supported. This can be done with the
<code>condition</code> property. If the value
- * returned from ServletRequest.getAttribute(condition) yields a non-null
value, the logging will be skipped.
- * </p>
- * <p>
- * For extended attributes coming from a getAttribute() call, it is you
responsibility to ensure there are no newline or
- * control characters.
- * </p>
*
* @author Peter Rossbach
*/
@@ -114,15 +94,17 @@ public class ExtendedAccessLogValve extends AccessLogValve
{
* double quotes ("").
*
* @param value - The value to wrap
+ * @param buf the buffer to write to
*
* @return '-' if null. Otherwise, toString() will be called on the object
and the value will be wrapped in quotes
* and any quotes will be escaped with 2 sets of quotes.
*/
- static String wrap(Object value) {
+ static void wrap(Object value, CharArrayWriter buf) {
String svalue;
// Does the value contain a " ? If so must encode it
if (value == null || "-".equals(value)) {
- return "-";
+ buf.append('-');
+ return;
}
try {
@@ -130,11 +112,15 @@ public class ExtendedAccessLogValve extends
AccessLogValve {
} catch (Throwable e) {
ExceptionUtils.handleThrowable(e);
/* Log error */
- return "-";
+ buf.append('-');
+ return;
}
- /* Wrap all values in double quotes. */
- return "\"" + svalue.replace("\"", "\"\"") + "\"";
+ buf.append('\"');
+ if (!svalue.isEmpty()) {
+ escapeAndAppend(svalue, buf, true);
+ }
+ buf.append('\"');
}
@Override
@@ -198,7 +184,7 @@ public class ExtendedAccessLogValve extends AccessLogValve {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap(request.getHeader(header)));
+ wrap(request.getHeader(header), buf);
}
}
@@ -211,7 +197,7 @@ public class ExtendedAccessLogValve extends AccessLogValve {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap(response.getHeader(header)));
+ wrap(response.getHeader(header), buf);
}
}
@@ -224,7 +210,7 @@ public class ExtendedAccessLogValve extends AccessLogValve {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
-
buf.append(wrap(request.getContext().getServletContext().getAttribute(attribute)));
+
wrap(request.getContext().getServletContext().getAttribute(attribute), buf);
}
}
@@ -253,7 +239,7 @@ public class ExtendedAccessLogValve extends AccessLogValve {
if (value.length() == 0) {
buf.append('-');
} else {
- buf.append(wrap(value.toString()));
+ wrap(value, buf);
}
}
}
@@ -283,7 +269,9 @@ public class ExtendedAccessLogValve extends AccessLogValve {
}
buffer.append(iter.next());
}
- buf.append(wrap(buffer.toString()));
+ wrap(buffer, buf);
+ } else {
+ buf.append('-');
}
return;
}
@@ -300,7 +288,7 @@ public class ExtendedAccessLogValve extends AccessLogValve {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap(request.getAttribute(attribute)));
+ wrap(request.getAttribute(attribute), buf);
}
}
@@ -317,7 +305,7 @@ public class ExtendedAccessLogValve extends AccessLogValve {
if (request != null) {
session = request.getSession(false);
if (session != null) {
- buf.append(wrap(session.getAttribute(attribute)));
+ wrap(session.getAttribute(attribute), buf);
}
}
}
@@ -342,7 +330,13 @@ public class ExtendedAccessLogValve extends AccessLogValve
{
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap(urlEncode(request.getParameter(parameter))));
+ String parameterValue;
+ try {
+ parameterValue = request.getParameter(parameter);
+ } catch (IllegalStateException ise) {
+ parameterValue = null;
+ }
+ wrap(urlEncode(parameterValue), buf);
}
}
@@ -695,63 +689,63 @@ public class ExtendedAccessLogValve extends
AccessLogValve {
return new AccessLogElement() {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap(request.getAuthType()));
+ wrap(request.getAuthType(), buf);
}
};
} else if ("remoteUser".equals(parameter)) {
return new AccessLogElement() {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap(request.getRemoteUser()));
+ wrap(request.getRemoteUser(), buf);
}
};
} else if ("requestedSessionId".equals(parameter)) {
return new AccessLogElement() {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap(request.getRequestedSessionId()));
+ wrap(request.getRequestedSessionId(), buf);
}
};
} else if ("requestedSessionIdFromCookie".equals(parameter)) {
return new AccessLogElement() {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap("" +
request.isRequestedSessionIdFromCookie()));
+
wrap(String.valueOf(request.isRequestedSessionIdFromCookie()), buf);
}
};
} else if ("requestedSessionIdValid".equals(parameter)) {
return new AccessLogElement() {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap("" + request.isRequestedSessionIdValid()));
+ wrap(String.valueOf(request.isRequestedSessionIdValid()),
buf);
}
};
} else if ("contentLength".equals(parameter)) {
return new AccessLogElement() {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap("" + request.getContentLengthLong()));
+ wrap(String.valueOf(request.getContentLengthLong()), buf);
}
};
} else if ("characterEncoding".equals(parameter)) {
return new AccessLogElement() {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap(request.getCharacterEncoding()));
+ wrap(request.getCharacterEncoding(), buf);
}
};
} else if ("locale".equals(parameter)) {
return new AccessLogElement() {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap(request.getLocale()));
+ wrap(request.getLocale(), buf);
}
};
} else if ("protocol".equals(parameter)) {
return new AccessLogElement() {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap(request.getProtocol()));
+ wrap(request.getProtocol(), buf);
}
};
} else if ("scheme".equals(parameter)) {
@@ -765,7 +759,7 @@ public class ExtendedAccessLogValve extends AccessLogValve {
return new AccessLogElement() {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- buf.append(wrap("" + request.isSecure()));
+ wrap(Boolean.valueOf(request.isSecure()), buf);
}
};
}
diff --git a/test/org/apache/catalina/valves/TestExtendedAccessLogValve.java
b/test/org/apache/catalina/valves/TestExtendedAccessLogValve.java
index b6cecb6fa1..e5f5bde5ab 100644
--- a/test/org/apache/catalina/valves/TestExtendedAccessLogValve.java
+++ b/test/org/apache/catalina/valves/TestExtendedAccessLogValve.java
@@ -52,7 +52,12 @@ public class TestExtendedAccessLogValve extends
TomcatBaseTest {
patterns.add(new Object[]{"basic", "time cs-method cs-uri-stem
cs-uri-query"});
patterns.add(new Object[]{"ip", "time cs-method sc-status c-ip s-ip
s-dns c-dns"});
patterns.add(new Object[]{"headers", "time cs-method cs(Referer)
cs(Cookie) sc(Content-Type)"});
- patterns.add(new Object[]{"bytes", "date time cs-method cs-uri-stem
bytes time-taken cached"});
+ patterns.add(new Object[]{"bytes", "date time cs-method cs-uri bytes
time-taken cached"});
+ patterns.add(new Object[]{"tomcat1", "x-threadname x-A(testSCAttr)
x-C(COOKIE-1_3) x-O(Custom)"});
+ patterns.add(new Object[]{"tomcat2", "x-R(testRAttr) x-S(sessionAttr)
x-P(testParam)"});
+ patterns.add(new Object[]{"tomcat3", "x-H(authType)
x-H(characterEncoding) x-H(contentLength)"});
+ patterns.add(new Object[]{"tomcat4", "x-H(locale) x-H(protocol)
x-H(remoteUser) x-H(requestedSessionId)"});
+ patterns.add(new Object[]{"tomcat5",
"x-H(requestedSessionIdFromCookie) x-H(requestedSessionIdValid) x-H(scheme)
x-H(secure)"});
return patterns;
}
@@ -85,6 +90,11 @@ public class TestExtendedAccessLogValve extends
TomcatBaseTest {
private static final long serialVersionUID = 1L;
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse
resp) throws IOException {
+ req.getServletContext().setAttribute("testSCAttr",
"testSCAttrValue");
+ req.getSession(true).setAttribute("sessionAttr",
"sessionAttrValue");
+ req.setAttribute("testRAttr", "testRValue");
+ resp.addHeader("Custom", "value1");
+ resp.addHeader("Custom", "value2");
resp.getWriter().write("Test response");
}
});
@@ -92,7 +102,7 @@ public class TestExtendedAccessLogValve extends
TomcatBaseTest {
tomcat.start();
- String url = "http://localhost:"; + getPort() + "/test";
+ String url = "http://localhost:"; + getPort() +
"/test?testParam=testValue";
ByteChunk out = new ByteChunk();
Map<String, List<String>> reqHead = new HashMap<>();
List<String> cookieHeaders = new ArrayList<>();
@@ -139,6 +149,8 @@ public class TestExtendedAccessLogValve extends
TomcatBaseTest {
Assert.assertTrue("No data entries found", !dataLines.isEmpty());
String entryLine = dataLines.get(0);
+ System.out.println(name + ": " + entryLine);
+
String[] parts = entryLine.split("\\s+");
String[] expectedFields = logPattern.split("\\s+");
diff --git
a/test/org/apache/catalina/valves/TestExtendedAccessLogValveWrap.java
b/test/org/apache/catalina/valves/TestExtendedAccessLogValveWrap.java
index 129f6a371e..da357dde6b 100644
--- a/test/org/apache/catalina/valves/TestExtendedAccessLogValveWrap.java
+++ b/test/org/apache/catalina/valves/TestExtendedAccessLogValveWrap.java
@@ -16,6 +16,8 @@
*/
package org.apache.catalina.valves;
+import java.io.CharArrayWriter;
+
import org.junit.Assert;
import org.junit.Test;
@@ -23,41 +25,57 @@ public class TestExtendedAccessLogValveWrap {
@Test
public void alpha() {
- Assert.assertEquals("\"foo\"", ExtendedAccessLogValve.wrap("foo"));
+ CharArrayWriter buf = new CharArrayWriter();
+ ExtendedAccessLogValve.wrap("foo", buf);
+ Assert.assertEquals("\"foo\"", buf.toString());
}
@Test
public void testNull() {
- Assert.assertEquals("-", ExtendedAccessLogValve.wrap(null));
+ CharArrayWriter buf = new CharArrayWriter();
+ ExtendedAccessLogValve.wrap(null, buf);
+ Assert.assertEquals("-", buf.toString());
}
@Test
public void empty() {
- Assert.assertEquals("\"\"", ExtendedAccessLogValve.wrap(""));
+ CharArrayWriter buf = new CharArrayWriter();
+ ExtendedAccessLogValve.wrap("", buf);
+ Assert.assertEquals("\"\"", buf.toString());
}
@Test
public void singleQuoteMiddle() {
- Assert.assertEquals("\"foo'bar\"",
ExtendedAccessLogValve.wrap("foo'bar"));
+ CharArrayWriter buf = new CharArrayWriter();
+ ExtendedAccessLogValve.wrap("foo'bar", buf);
+ Assert.assertEquals("\"foo'bar\"", buf.toString());
}
@Test
public void doubleQuoteMiddle() {
- Assert.assertEquals("\"foo\"\"bar\"",
ExtendedAccessLogValve.wrap("foo\"bar"));
+ CharArrayWriter buf = new CharArrayWriter();
+ ExtendedAccessLogValve.wrap("foo\"bar", buf);
+ Assert.assertEquals("\"foo\"\"bar\"", buf.toString());
}
@Test
public void doubleQuoteStart() {
- Assert.assertEquals("\"\"\"foobar\"",
ExtendedAccessLogValve.wrap("\"foobar"));
+ CharArrayWriter buf = new CharArrayWriter();
+ ExtendedAccessLogValve.wrap("\"foobar", buf);
+ Assert.assertEquals("\"\"\"foobar\"", buf.toString());
}
@Test
public void doubleQuoteEnd() {
- Assert.assertEquals("\"foobar\"\"\"",
ExtendedAccessLogValve.wrap("foobar\""));
+ CharArrayWriter buf = new CharArrayWriter();
+ ExtendedAccessLogValve.wrap("foobar\"", buf);
+ Assert.assertEquals("\"foobar\"\"\"", buf.toString());
}
@Test
public void doubleQuote() {
- Assert.assertEquals("\"\"\"\"", ExtendedAccessLogValve.wrap("\""));
+ CharArrayWriter buf = new CharArrayWriter();
+ ExtendedAccessLogValve.wrap("\"", buf);
+ Assert.assertEquals("\"\"\"\"", buf.toString());
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
