You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jmeter-dev@jakarta.apache.org by se...@apache.org on 2009/03/13 02:57:54 UTC

svn commit: r753086 - in /jakarta/jmeter/trunk: bin/ src/protocol/http/org/apache/jmeter/protocol/http/control/ test/src/org/apache/jmeter/protocol/http/control/ xdocs/ xdocs/usermanual/

Author: sebb
Date: Fri Mar 13 01:57:53 2009
New Revision: 753086

URL: http://svn.apache.org/viewvc?rev=753086&view=rev
Log:
Check validity of cookies before storing them.

Modified:
    jakarta/jmeter/trunk/bin/jmeter.properties
    jakarta/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java
    jakarta/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCookieManager.java
    jakarta/jmeter/trunk/xdocs/changes.xml
    jakarta/jmeter/trunk/xdocs/usermanual/component_reference.xml

Modified: jakarta/jmeter/trunk/bin/jmeter.properties
URL: http://svn.apache.org/viewvc/jakarta/jmeter/trunk/bin/jmeter.properties?rev=753086&r1=753085&r2=753086&view=diff
==============================================================================
--- jakarta/jmeter/trunk/bin/jmeter.properties (original)
+++ jakarta/jmeter/trunk/bin/jmeter.properties Fri Mar 13 01:57:53 2009
@@ -624,6 +624,10 @@
 # Default is COOKIE_; to remove the prefix, define it as one or more spaces
 #CookieManager.name.prefix=
  
+# CookieManager behaviour - check received cookies are valid before storing them?
+# Default is true. Use false to revert to previous behaviour
+#CookieManager.check.cookies=true
+
 # (2.0.3) JMeterThread behaviour has been changed to set the started flag before
 # the controllers are initialised. This is so controllers can access variables earlier. 
 # In case this causes problems, the previous behaviour can be restored by uncommenting

Modified: jakarta/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java
URL: http://svn.apache.org/viewvc/jakarta/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java?rev=753086&r1=753085&r2=753086&view=diff
==============================================================================
--- jakarta/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java (original)
+++ jakarta/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java Fri Mar 13 01:57:53 2009
@@ -83,6 +83,9 @@
     private static final boolean SAVE_COOKIES =
         JMeterUtils.getPropDefault("CookieManager.save.cookies", false);// $NON-NLS-1$
 
+    private static final boolean CHECK_COOKIES =
+        JMeterUtils.getPropDefault("CookieManager.check.cookies", true);// $NON-NLS-1$
+
     private transient CookieSpec cookieSpec;
 
     private transient CollectionProperty initialCookies;
@@ -383,31 +386,41 @@
             return;
         }
         for(int i=0;i<cookies.length;i++){
-            Date expiryDate = cookies[i].getExpiryDate();
-            long exp = 0;
-            if (expiryDate!= null) {
-                exp=expiryDate.getTime();
-            }
-            Cookie newCookie = new Cookie(
-                    cookies[i].getName(),
-                    cookies[i].getValue(),
-                    cookies[i].getDomain(),
-                    cookies[i].getPath(),
-                    cookies[i].getSecure(),
-                    exp / 1000,
-                    cookies[i].isPathAttributeSpecified(),
-                    cookies[i].isDomainAttributeSpecified()
-                    );
-
-            // Store session cookies as well as unexpired ones
-            if (exp == 0 || exp >= System.currentTimeMillis()) {
-                newCookie.setVersion(cookies[i].getVersion());
-                add(newCookie); // Has its own debug log; removes matching cookies
-            } else {
-                removeMatchingCookies(newCookie);
-                if (debugEnabled){
-                    log.debug("Dropping expired Cookie: "+newCookie.toString());
+            org.apache.commons.httpclient.Cookie cookie = cookies[i];
+            try {
+                if (CHECK_COOKIES) {
+                    cookieSpec.validate(host, port, path, isSecure, cookie);
+                }
+                Date expiryDate = cookie.getExpiryDate();
+                long exp = 0;
+                if (expiryDate!= null) {
+                    exp=expiryDate.getTime();
+                }
+                Cookie newCookie = new Cookie(
+                        cookie.getName(),
+                        cookie.getValue(),
+                        cookie.getDomain(),
+                        cookie.getPath(),
+                        cookie.getSecure(),
+                        exp / 1000,
+                        cookie.isPathAttributeSpecified(),
+                        cookie.isDomainAttributeSpecified()
+                        );
+
+                // Store session cookies as well as unexpired ones
+                if (exp == 0 || exp >= System.currentTimeMillis()) {
+                    newCookie.setVersion(cookie.getVersion());
+                    add(newCookie); // Has its own debug log; removes matching cookies
+                } else {
+                    removeMatchingCookies(newCookie);
+                    if (debugEnabled){
+                        log.debug("Dropping expired Cookie: "+newCookie.toString());
+                    }
                 }
+            } catch (MalformedCookieException e) { // This means the cookie was wrong for the URL
+                log.debug("Not storing invalid cookie: <"+cookieHeader+"> for URL "+url+" ("+e.getLocalizedMessage()+")");
+            } catch (IllegalArgumentException e) {
+                log.warn(cookieHeader+e.getLocalizedMessage());
             }
         }
 

Modified: jakarta/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCookieManager.java
URL: http://svn.apache.org/viewvc/jakarta/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCookieManager.java?rev=753086&r1=753085&r2=753086&view=diff
==============================================================================
--- jakarta/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCookieManager.java (original)
+++ jakarta/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCookieManager.java Fri Mar 13 01:57:53 2009
@@ -90,6 +90,15 @@
             assertNotNull(man.getCookieHeaderForURL(url));
         }
 
+        public void testCrossDomainHandling() throws Exception {
+            URL url = new URL("http://jakarta.apache.org/");
+            assertEquals(0,man.getCookieCount()); // starts empty
+            man.addCookieFromHeader("test=2;domain=.hc.apache.org", url);
+            assertEquals(0,man.getCookieCount()); // should not be stored
+            man.addCookieFromHeader("test=1;domain=.jakarta.apache.org", url);
+            assertEquals(1,man.getCookieCount()); // OK
+        }
+
         /**
          * Test that we won't be tricked by similar host names (this was a past
          * bug, although it never got reported in the bug database):
@@ -309,7 +318,7 @@
         public void testCookiePolicyNetscape() throws Exception {
             man.setCookiePolicy(CookiePolicy.NETSCAPE);
             man.testStarted(); // ensure policy is picked up
-            URL url = new URL("http://order.now/sub1/moo.html");
+            URL url = new URL("http://www.order.now/sub1/moo.html");
             man.addCookieFromHeader("test1=moo1;", url);
             man.addCookieFromHeader("test2=moo2;path=/sub1", url);
             man.addCookieFromHeader("test2=moo3;path=/", url);

Modified: jakarta/jmeter/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/jakarta/jmeter/trunk/xdocs/changes.xml?rev=753086&r1=753085&r2=753086&view=diff
==============================================================================
--- jakarta/jmeter/trunk/xdocs/changes.xml (original)
+++ jakarta/jmeter/trunk/xdocs/changes.xml Fri Mar 13 01:57:53 2009
@@ -190,6 +190,7 @@
 <li>Bug 46821 - JDBC select request doesn't store the first column in the variables</li>
 <li>Fix bug in HTTP file: handling - read bytes, not characters in the default encoding.</li>
 <li>Change HTTPS spoofing so https: links are replaced even when URL match fails</li>
+<li>Check validity of cookies before storing them.</li>
 </ul>
 
 <h3>Improvements</h3>

Modified: jakarta/jmeter/trunk/xdocs/usermanual/component_reference.xml
URL: http://svn.apache.org/viewvc/jakarta/jmeter/trunk/xdocs/usermanual/component_reference.xml?rev=753086&r1=753085&r2=753086&view=diff
==============================================================================
--- jakarta/jmeter/trunk/xdocs/usermanual/component_reference.xml (original)
+++ jakarta/jmeter/trunk/xdocs/usermanual/component_reference.xml Fri Mar 13 01:57:53 2009
@@ -2640,6 +2640,12 @@
 the <complink name="View Results Tree"/> Listener.
 </p>
 <p>
+JMeter version 2.3.2 and earlier did not check that received cookies were valid for the URL.
+This meant that cross-domain cookies were stored, and might be used later.
+This has been fixed in later versions.
+To revert to the earlier behaviour, define the JMeter property "CookieManager.check.cookies=false".
+</p>
+<p>
 Received Cookies can be stored as JMeter thread variables
 (versions of JMeter after 2.3.2 no longer do this by default).
 To save cookies as variables, define the property "CookieManager.save.cookies=true".



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