tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1148471 - in /tomcat/trunk: java/org/apache/catalina/filters/CsrfPreventionFilter.java test/org/apache/catalina/filters/TestCsrfPreventionFilter2.java webapps/docs/changelog.xml
Date Tue, 19 Jul 2011 18:20:24 GMT
Author: markt
Date: Tue Jul 19 18:20:23 2011
New Revision: 1148471

URL: http://svn.apache.org/viewvc?rev=1148471&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51509
Fix potential concurrency issue in CSRF prevention filter that may lead to some requests failing
that should not.

Added:
    tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter2.java   (with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java?rev=1148471&r1=1148470&r2=1148471&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java (original)
+++ tomcat/trunk/java/org/apache/catalina/filters/CsrfPreventionFilter.java Tue Jul 19 18:20:23
2011
@@ -310,11 +310,15 @@ public class CsrfPreventionFilter extend
         }
         
         public void add(T key) {
-            cache.put(key, null);
+            synchronized (cache) {
+                cache.put(key, null);
+            }
         }
-        
+
         public boolean contains(T key) {
-            return cache.containsKey(key);
+            synchronized (cache) {
+                return cache.containsKey(key);
+            }
         }
     }
 }

Added: tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter2.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter2.java?rev=1148471&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter2.java (added)
+++ tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter2.java Tue Jul 19
18:20:23 2011
@@ -0,0 +1,88 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.catalina.filters;
+
+import junit.framework.TestCase;
+
+import org.apache.catalina.filters.CsrfPreventionFilter.LruCache;
+
+public class TestCsrfPreventionFilter2 extends TestCase {
+
+    /**
+     * When this test fails, it tends to enter a long running loop but it will
+     * eventually finish (after ~70s on a 8-core Windows box).
+     */
+    public void testLruCacheConcurrency() throws Exception {
+        int threadCount = 2;
+        long iterationCount = 100000L;
+        
+        assertTrue(threadCount > 1);
+
+        LruCache<String> cache = new LruCache<String>(threadCount - 1);
+        
+        LruTestThread[] threads = new LruTestThread[threadCount];
+        for (int i = 0; i < threadCount; i++) {
+            threads[i] = new LruTestThread(cache, iterationCount);
+        }
+        
+        for (int i = 0; i < threadCount; i++) {
+            threads[i].start();
+        }
+
+        for (int i = 0; i < threadCount; i++) {
+            threads[i].join();
+        }
+
+        for (int i = 0; i < threadCount; i++) {
+            assertTrue(threads[i].getResult());
+        }
+
+    }
+
+    private static class LruTestThread extends Thread {
+        private final LruCache<String> cache;
+        private long iterationCount = 0;
+        private volatile boolean result = false;
+
+        public LruTestThread(LruCache<String> cache, long iterationCount) {
+            this.cache = cache;
+            this.iterationCount = iterationCount;
+        }
+
+        public boolean getResult() {
+            return result;
+        }
+
+        @Override
+        public void run() {
+            String test = getName();
+            try {
+                for (long i = 0; i < iterationCount; i++) {
+                    cache.add(test + i);
+                    if (!cache.contains(test + i)) {
+                        // Expected
+                    }
+                }
+            } catch (Exception e) {
+                e.printStackTrace();
+                return;
+            }
+            result = true;
+        }
+    }
+}

Propchange: tomcat/trunk/test/org/apache/catalina/filters/TestCsrfPreventionFilter2.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1148471&r1=1148470&r2=1148471&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Jul 19 18:20:23 2011
@@ -63,6 +63,10 @@
         ignored when scanning jars for tag libraries. (kkolinko)
       </fix>
       <fix>
+        <bug>51509</bug>: Fix potential concurrency issue in CSRF prevention
+        filter that may lead to some requests failing that should not. (markt)
+      </fix>
+      <fix>
         <bug>51518</bug>: Correct error in web.xml parsing rules for the
         &lt;others/&gt; tag when using absolute ordering. (markt)
       </fix>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message