You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by js...@apache.org on 2002/11/27 02:32:34 UTC
cvs commit: jakarta-commons/httpclient/src/test/org/apache/commons/httpclient TestCookie.java
jsdever 2002/11/26 17:32:34
Modified: httpclient/src/java/org/apache/commons/httpclient
Cookie.java
httpclient/src/test/org/apache/commons/httpclient
TestCookie.java
Log:
Fixes for Cookie.java
Fixes bugs:
http://issues.apache.org/bugzilla/show_bug.cgi?id=6513
http://issues.apache.org/bugzilla/show_bug.cgi?id=11223
Whats Changed:
1) Constructors throw IllegalArguementException if name parameter is
null or blank. Current implementation of the cookie class accepts blank
cookie name, which is disallowed if I interpret RFC-2109 right. Actually
as far as I can see blank cookie values also violate the RFC-2109.
Please let me know if you agree or disagree
2) Default (parameterless) constructor added. The constructor assigns
'noname' per default for a lack of a better idea on my part. Please let
me know if you see a problem with this kind of naming convention
3) Method Cookie.match() logs a warning message if a cookie has invalid state
(domain and/or path not specified) and returns false. IllegalArguementException
is thrown if any of the input parameters is null.
4) Path matching logic is somewhat improved as the current
implementation is a bit flaky. Currently /foo and /foobar paths would
match which is not supposed to be the case, unless I have missed something
in the RFC
5) All methods should react appropriately to null parameter input
throwing an exception if null parameter is disallowed
Contributed by: Oleg Kalnichevski
Revision Changes Path
1.25 +74 -22 jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/Cookie.java
Index: Cookie.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/Cookie.java,v
retrieving revision 1.24
retrieving revision 1.25
diff -u -r1.24 -r1.25
--- Cookie.java 21 Oct 2002 14:39:22 -0000 1.24
+++ Cookie.java 27 Nov 2002 01:32:34 -0000 1.25
@@ -91,6 +91,7 @@
* @author Sean C. Sullivan
* @author <a href="mailto:JEvans@Cyveillance.com">John Evans</a>
* @author Marc A. Saegesser
+ * @author <a href="mailto:oleg@ural.ru">Oleg Kalnichevski</a>
* @version $Revision$ $Date$
*/
@@ -99,6 +100,16 @@
// ----------------------------------------------------------- Constructors
/**
+ * Create a cookie. Default constructor
+ *
+ * The new cookie is assigned
+ */
+
+ public Cookie() {
+ this(null, "noname", null, null, null, false);
+ }
+
+ /**
* Create a cookie.
*
* @param name the cookie name
@@ -124,6 +135,12 @@
public Cookie(String domain, String name, String value, String path, Date expires, boolean secure) {
super(name, value);
log.trace("enter Cookie(String, String, String, String, Date, boolean)");
+ if (name == null) {
+ throw new IllegalArgumentException("Cookie name may not be null");
+ }
+ if (name.equals("")) {
+ throw new IllegalArgumentException("Cookie name may not be blank");
+ }
this.setPath(path);
this.setDomain(domain);
this.setExpiryDate(expires);
@@ -448,7 +465,7 @@
* @param secure <tt>true</tt> if the request is using the HTTPS protocol
* @param date the time at which the request is submitted
*/
- public boolean matches(String domain, int port, String path, boolean secure, Date now) {
+ public boolean matches(String domain, int port, String path, boolean secure, Date date) {
log.trace("enter Cookie.matches(Strinng, int, String, boolean, Date");
// FIXME: RFC2109 doesn't consider ports when filtering/matching
// cookies. Quoting from Section 2 - Terminology:
@@ -464,18 +481,32 @@
//
// The current implementation doesn't support RFC2965,
// and ignores ports when matching cookies.
+ if (domain == null) {
+ throw new IllegalArgumentException("domain parameter may not be null");
+ }
+ if (path == null) {
+ throw new IllegalArgumentException("path parameter may not be null");
+ }
+ if (date == null) {
+ throw new IllegalArgumentException("date parameter may not be null");
+ }
+ if (getDomain() == null) {
+ log.warn("Invalid cookie state: domain not specified");
+ return false;
+ }
+ if (getPath() == null) {
+ log.warn("Invalid cookie state: path not specified");
+ return false;
+ }
+
domain = domain.toLowerCase();
- // FIXME: Is path.startsWith(cookie.getPath()) enough?
- // Or do we need to check that we are comparing
- // at a path-element break?
- // E.g.., if "/foo" is the cookie's path,
- // should /foobar see the cookie? Probably not.
return (
- (getExpiryDate() == null || getExpiryDate().after(now)) && // only add the cookie if it hasn't yet expired
- (getDomain() != null && domainMatch(domain, getDomain())) &&// and the domain pattern matches
- ((getPath() != null) && (path.startsWith(getPath()))) && // and the path is null or matching
- (getSecure() ? secure : true) // and if the secure flag is set, only if the request is actually secure
+ (getExpiryDate() == null ||
+ getExpiryDate().after(date)) && // only add the cookie if it hasn't yet expired
+ (domainMatch(domain, getDomain())) && // and the domain pattern matches
+ (pathMatch(path, getPath())) && // and the path is null or matching
+ (getSecure() ? secure : true) // and if the secure flag is set, only if the request is actually secure
);
}
@@ -631,14 +662,14 @@
return 0;
} else if (c1.getPath() == null) {
// null is assumed to be "/"
- if (c2.getPath().equals("/")) {
+ if (c2.getPath().equals(PATH_DELIM)) {
return 0;
} else {
return -1;
}
} else if (c2.getPath() == null) {
// null is assumed to be "/"
- if (c1.getPath().equals("/")) {
+ if (c1.getPath().equals(PATH_DELIM)) {
return 0;
} else {
return 1;
@@ -750,18 +781,21 @@
if(path == null){
throw new IllegalArgumentException("path may not be null.");
}
+ if(setCookie == null){
+ throw new IllegalArgumentException("set-cookie header may not be null.");
+ }
/* Build the default path. Per RFC 2109/4.3.1 this is the
* request path up to, but not including, the right-most / charater.
*/
if(path.length() == 0){
log.debug("Cookie.parse(): Fixing up empty request path.");
- path = "/";
+ path = PATH_DELIM;
}
String defaultPath = null;
- int lastSlashIndex = path.lastIndexOf("/");
+ int lastSlashIndex = path.lastIndexOf(PATH_DELIM);
if(lastSlashIndex == 0){
- defaultPath = "/";
+ defaultPath = PATH_DELIM;
}else if(lastSlashIndex > 0){
defaultPath = path.substring(0, lastSlashIndex);
}else{
@@ -985,12 +1019,12 @@
// set path if not otherwise specified
if(null == cookie.getPath()) {
if(null != path) {
- if(!path.endsWith("/")) {
- int x = path.lastIndexOf("/");
+ if(!path.endsWith(PATH_DELIM)) {
+ int x = path.lastIndexOf(PATH_DELIM);
if(0 < x) {
cookie.setPath(path.substring(0,x));
} else {
- cookie.setPath("/");
+ cookie.setPath(PATH_DELIM);
}
} else {
cookie.setPath(path);
@@ -1040,6 +1074,20 @@
}
/**
+ * Performs a path-match slightly smarter than a straight-forward startsWith check.
+ */
+ private static boolean pathMatch(final String path, final String topmostPath)
+ {
+ boolean match = path.startsWith( topmostPath );
+ if (match) {
+ if (!topmostPath.endsWith(PATH_DELIM)) {
+ match = (path.charAt(topmostPath.length()) == PATH_DELIM_CHAR);
+ }
+ }
+ return match;
+ }
+
+ /**
* Adds the given cookie into the given list in descending path order. That is,
* more specific path to least specific paths. This may not be the fastest
* algorythm, but it'll work OK for the small number of cookies we're
@@ -1108,6 +1156,10 @@
private static final RuleBasedCollator stringCollator =
(RuleBasedCollator)RuleBasedCollator.getInstance(
new Locale("en", "US", ""));
+
+ /** Path delimiter */
+ private static final String PATH_DELIM = "/";
+ private static final char PATH_DELIM_CHAR = PATH_DELIM.charAt(0);
/** Log object for this class */
private static final Log log = LogFactory.getLog(Cookie.class);
1.12 +29 -10 jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestCookie.java
Index: TestCookie.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestCookie.java,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -r1.11 -r1.12
--- TestCookie.java 21 Oct 2002 14:39:22 -0000 1.11
+++ TestCookie.java 27 Nov 2002 01:32:34 -0000 1.12
@@ -92,6 +92,7 @@
private static final String OLD_EXPIRY = "Expires=Thu, 01-Jan-1970 00:00:10 GMT";
private static final String SEP = ";";
private static final String ROOT_PATH = "/";
+ private static final int DEFAULT_PORT = 80;
private String[] testName = { "custno", "name", "name" };
private String[] testValue = { "12345", "John", "Doe, John" };
@@ -222,7 +223,7 @@
assertTrue("Secure",!parsed[0].getSecure());
assertEquals("Version",0,parsed[0].getVersion());
}
-
+/*
public void testParseNoName() throws Exception {
Header setCookie = new Header("Set-Cookie","=cookie-value");
Cookie[] parsed = Cookie.parse("127.0.0.1","/",setCookie);
@@ -238,7 +239,7 @@
assertTrue("Secure",!parsed[0].getSecure());
assertEquals("Version",0,parsed[0].getVersion());
}
-
+*/
public void testParseNoValue() throws Exception {
Header setCookie = new Header("Set-Cookie","cookie-name=");
Cookie[] parsed = Cookie.parse("127.0.0.1","/",setCookie);
@@ -508,6 +509,18 @@
}
}
+ public void testParseWithPathMismatch2() {
+ Header setCookie = new Header("Set-Cookie","cookie-name=cookie-value; path=/foobar");
+ try {
+ Cookie[] parsed = Cookie.parse("127.0.0.1","/foo",false,setCookie);
+ fail("HttpException should have been thrown.");
+ } catch (HttpException e) {
+ // expected
+ } catch (Exception e){
+ fail("Should have thrown HttpException.");
+ }
+ }
+
public void testComparator() throws Exception {
Header setCookie = null;
Cookie[] parsed = null;
@@ -578,7 +591,7 @@
Cookie[] parsed = Cookie.parse(DOMAIN_NAME, ROOT_PATH, true, setCookie);
try{
- Header header = Cookie.createCookieHeader(null, ROOT_PATH, false, parsed);
+ Header header = Cookie.createCookieHeader(null, DEFAULT_PORT, ROOT_PATH, false, parsed);
fail("IllegalArgumentException should have been thrown.");
}catch(IllegalArgumentException e){
// Expected
@@ -595,7 +608,7 @@
Cookie[] parsed = Cookie.parse(DOMAIN_NAME, ROOT_PATH, false, setCookie);
try{
- Header header = Cookie.createCookieHeader(DOMAIN_NAME, null, false, parsed);
+ Header header = Cookie.createCookieHeader(DOMAIN_NAME, DEFAULT_PORT, null, false, parsed);
fail("IllegalArgumentException should have been thrown.");
}catch(IllegalArgumentException e){
// Expected
@@ -612,7 +625,7 @@
cookies[0] = new Cookie(null, "name0", "value0");
cookies[1] = new Cookie(null, "name1", "value1", null, null, false);
- Header header = Cookie.createCookieHeader(DOMAIN_NAME, ROOT_PATH, false, cookies);
+ Header header = Cookie.createCookieHeader(DOMAIN_NAME, DEFAULT_PORT, ROOT_PATH, false, cookies);
assertEquals("createCookieHeader added cookies with null domains or paths", null, header);
}
@@ -625,7 +638,7 @@
Cookie[] parsed = Cookie.parse(DOMAIN_NAME, ROOT_PATH, true, setCookie);
try{
- Header header = Cookie.createCookieHeader(null, null, false, parsed);
+ Header header = Cookie.createCookieHeader(null, DEFAULT_PORT, null, false, parsed);
fail("IllegalArgumentException should have been thrown.");
}catch(IllegalArgumentException e){
// Expected
@@ -665,6 +678,12 @@
private void checkDate(String date) throws Exception {
Header setCookie = new Header("Set-Cookie", "custno=12345;Expires='"+date+"'");
Cookie.parse("localhost","/",setCookie);
+ }
+
+
+ public void testDefaultConsttuctor() throws Exception {
+ Cookie dummy = new Cookie();
+ assertEquals( dummy.toExternalForm(), "noname=null" );
}
}
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>