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
<others/> tag when using absolute ordering. (markt)
</fix>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
