It was a mistake to use original httpRequest as long as it is availab…

…le because web containers may recycle it and populate with the fields from another request. So make sure that when prepareForDeferredProcessing() is called, we will be only using copied data.

logback-access/src/main/java/ch/qos/logback/access/spi/AccessEvent.java

@@ -295,11 +295,12 @@ public Map<String, String[]> getRequestParameterMap() {
 
   public String getAttribute(String key) {
     Object value = null;
-    if (httpRequest != null) {
-      value = httpRequest.getAttribute(key);
-    } else if (attributeMap != null) {
-      // Event was prepared for deferred processing so we have a copy of attribute map
+    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;

logback-access/src/test/java/ch/qos/logback/access/dummy/DummyRequest.java

@@ -35,6 +35,7 @@ public class DummyRequest implements HttpServletRequest {
 
   Hashtable<String, String> headerNames;
   String uri;
+  Map<String, Object> attributes;
 
   static {
     DUMMY_DEFAULT_ATTR_MAP.put("testKey", "testKey");
@@ -46,6 +47,8 @@ public DummyRequest() {
     headerNames = new Hashtable<String, String>();
     headerNames.put("headerName1", "headerValue1");
     headerNames.put("headerName2", "headerValue2");
+
+    attributes = new HashMap<String, Object>(DUMMY_DEFAULT_ATTR_MAP);
   }
 
   public String getAuthType() {
@@ -170,11 +173,11 @@ public boolean isUserInRole(String arg0) {
   }
 
   public Object getAttribute(String key) {
-    return DUMMY_DEFAULT_ATTR_MAP.get(key);
+    return attributes.get(key);
   }
 
   public Enumeration getAttributeNames() {
-    return Collections.enumeration(DUMMY_DEFAULT_ATTR_MAP.keySet());
+    return Collections.enumeration(attributes.keySet());
   }
 
   public String getCharacterEncoding() {
@@ -304,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)

logback-access/src/test/java/ch/qos/logback/access/spi/AccessEventSerializationTest.java

@@ -16,6 +16,7 @@
 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.*;
@@ -70,4 +71,25 @@ public void testSerialization() throws IOException, ClassNotFoundException {
         .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"));
+  }
 }