diff --git a/logback-access/src/main/java/ch/qos/logback/access/spi/AccessEvent.java b/logback-access/src/main/java/ch/qos/logback/access/spi/AccessEvent.java index b5d250820d..b6865673dd 100755 --- a/logback-access/src/main/java/ch/qos/logback/access/spi/AccessEvent.java +++ b/logback-access/src/main/java/ch/qos/logback/access/spi/AccessEvent.java @@ -65,6 +65,7 @@ public class AccessEvent implements Serializable, IAccessEvent { Map requestHeaderMap; Map requestParameterMap; Map responseHeaderMap; + Map attributeMap; long contentLength = SENTINEL; int statusCode = SENTINEL; @@ -292,21 +293,49 @@ public Map getRequestParameterMap() { return requestParameterMap; } - /** - * Attributes are not serialized - * - * @param key - */ public String getAttribute(String key) { - if (httpRequest != null) { - Object value = httpRequest.getAttribute(key); - if (value == null) { - return NA; - } else { - return value.toString(); + Object value = null; + if (attributeMap != null) { + // Event was prepared for deferred processing so we have a copy of attribute map and must use that copy + value = attributeMap.get(key); + } else if (httpRequest != null) { + // We have original request so take attribute from it + value = httpRequest.getAttribute(key); + } + + return value != null ? value.toString() : NA; + } + + private void copyAttributeMap() { + + if (httpRequest == null) { + return; + } + + attributeMap = new HashMap(); + + Enumeration names = httpRequest.getAttributeNames(); + while (names.hasMoreElements()) { + String name = names.nextElement(); + + Object value = httpRequest.getAttribute(name); + if (shouldCopyAttribute(name, value)) { + attributeMap.put(name, value); } + } + } + + private boolean shouldCopyAttribute(String name, Object value) { + if (AccessConstants.LB_INPUT_BUFFER.equals(name) || AccessConstants.LB_OUTPUT_BUFFER.equals(name)) { + // Do not copy attributes used by logback internally - these are available via other getters anyway + return false; + } else if (value == null) { + // No reasons to copy nulls - Map.get() will return null for missing keys and the list of attribute + // names is not available through IAccessEvent + return false; } else { - return NA; + // Only copy what is serializable + return value instanceof Serializable; } } @@ -500,5 +529,7 @@ public void prepareForDeferredProcessing() { getContentLength(); getRequestContent(); getResponseContent(); + + copyAttributeMap(); } } diff --git a/logback-access/src/main/java/ch/qos/logback/access/spi/IAccessEvent.java b/logback-access/src/main/java/ch/qos/logback/access/spi/IAccessEvent.java index 9c1ee780dd..049e8fe413 100644 --- a/logback-access/src/main/java/ch/qos/logback/access/spi/IAccessEvent.java +++ b/logback-access/src/main/java/ch/qos/logback/access/spi/IAccessEvent.java @@ -92,11 +92,6 @@ public interface IAccessEvent extends DeferredProcessingAware { Map getRequestParameterMap(); - /** - * Attributes are not serialized - * - * @param key - */ String getAttribute(String key); String[] getRequestParameter(String key); diff --git a/logback-access/src/test/java/ch/qos/logback/access/dummy/DummyRequest.java b/logback-access/src/test/java/ch/qos/logback/access/dummy/DummyRequest.java index 3c7e913268..9710d6bc1d 100644 --- a/logback-access/src/test/java/ch/qos/logback/access/dummy/DummyRequest.java +++ b/logback-access/src/test/java/ch/qos/logback/access/dummy/DummyRequest.java @@ -13,33 +13,42 @@ */ package ch.qos.logback.access.dummy; +import ch.qos.logback.access.AccessConstants; + +import javax.servlet.*; +import javax.servlet.http.*; import java.io.BufferedReader; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.security.Principal; import java.util.*; -import javax.servlet.*; -import javax.servlet.http.*; - -import ch.qos.logback.access.AccessConstants; - public class DummyRequest implements HttpServletRequest { public final static String DUMMY_CONTENT_STRING = "request contents"; - public final static byte[] DUMMY_CONTENT_BYTES = DUMMY_CONTENT_STRING.getBytes(); + public final static byte[] DUMMY_CONTENT_BYTES = DUMMY_CONTENT_STRING.getBytes(); + + public static final Map DUMMY_DEFAULT_ATTR_MAP = new HashMap(); - public static final String DUMMY_RESPONSE_CONTENT_STRING = "response contents"; public static final byte[] DUMMY_RESPONSE_CONTENT_BYTES =DUMMY_RESPONSE_CONTENT_STRING.getBytes(); Hashtable headerNames; String uri; + Map attributes; + + static { + DUMMY_DEFAULT_ATTR_MAP.put("testKey", "testKey"); + DUMMY_DEFAULT_ATTR_MAP.put(AccessConstants.LB_INPUT_BUFFER, DUMMY_CONTENT_BYTES); + DUMMY_DEFAULT_ATTR_MAP.put(AccessConstants.LB_OUTPUT_BUFFER, DUMMY_RESPONSE_CONTENT_BYTES); + } public DummyRequest() { headerNames = new Hashtable(); headerNames.put("headerName1", "headerValue1"); headerNames.put("headerName2", "headerValue2"); + + attributes = new HashMap(DUMMY_DEFAULT_ATTR_MAP); } public String getAuthType() { @@ -164,19 +173,11 @@ public boolean isUserInRole(String arg0) { } public Object getAttribute(String key) { - if (key.equals("testKey")) { - return "testKey"; - } else if (AccessConstants.LB_INPUT_BUFFER.equals(key)) { - return DUMMY_CONTENT_BYTES; - } else if (AccessConstants.LB_OUTPUT_BUFFER.equals(key)) { - return DUMMY_RESPONSE_CONTENT_BYTES; - } else { - return null; - } + return attributes.get(key); } public Enumeration getAttributeNames() { - return null; + return Collections.enumeration(attributes.keySet()); } public String getCharacterEncoding() { @@ -306,7 +307,8 @@ public boolean isSecure() { public void removeAttribute(String arg0) { } - public void setAttribute(String arg0, Object arg1) { + public void setAttribute(String name, Object value) { + attributes.put(name, value); } public void setCharacterEncoding(String arg0) diff --git a/logback-access/src/test/java/ch/qos/logback/access/spi/AccessEventSerializationTest.java b/logback-access/src/test/java/ch/qos/logback/access/spi/AccessEventSerializationTest.java index 3882bbf3d9..2d61a4f66e 100644 --- a/logback-access/src/test/java/ch/qos/logback/access/spi/AccessEventSerializationTest.java +++ b/logback-access/src/test/java/ch/qos/logback/access/spi/AccessEventSerializationTest.java @@ -13,20 +13,16 @@ */ package ch.qos.logback.access.spi; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; - -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; - -import org.junit.Test; - import ch.qos.logback.access.dummy.DummyAccessEventBuilder; import ch.qos.logback.access.dummy.DummyRequest; import ch.qos.logback.access.dummy.DummyResponse; +import ch.qos.logback.access.dummy.DummyServerAdapter; +import org.junit.Test; + +import java.io.*; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; public class AccessEventSerializationTest { @@ -70,7 +66,30 @@ public void testSerialization() throws IOException, ClassNotFoundException { assertEquals(DummyRequest.DUMMY_RESPONSE_CONTENT_STRING, aeBack .getResponseContent()); - + + assertEquals(DummyRequest.DUMMY_DEFAULT_ATTR_MAP.get("testKey"), aeBack + .getAttribute("testKey")); } + // Web containers may (and will) recycle requests objects. So we must make sure that after + // we prepared an event for deferred processing it won't be using data from the original + // HttpRequest object which may at that time represent another request + @Test + public void testAttributesAreNotTakenFromRecycledRequestWhenProcessingDeferred() { + + DummyRequest request = new DummyRequest(); + DummyResponse response = new DummyResponse(); + DummyServerAdapter adapter = new DummyServerAdapter(request, response); + + IAccessEvent event = new AccessEvent(request, response, adapter); + + request.setAttribute("testKey", "ORIGINAL"); + + event.prepareForDeferredProcessing(); + + request.setAttribute("testKey", "NEW"); + + // Event should capture the original value + assertEquals("ORIGINAL", event.getAttribute("testKey")); + } }