jsdever 2002/11/29 11:03:02
Modified: httpclient/src/java/org/apache/commons/httpclient
Cookie.java
httpclient/src/test/org/apache/commons/httpclient
TestCookie.java
Log:
Make version 2 cookies the default.
Changes:
1) This patch fixes the problem of Cookie class assuming Netscape cookie format per
default. With this fix RFC 2109 compliant validation applies unless the cookie version
is explicitly set to 0 (Netscape cookie draft)
2) I have also taken liberty in heavily refactoring the Cookie.parse() method
- I have tried to restructure the code by separating parsing and validation
processes. The code is a bit more modular now
- I have improved (or so I'd like to hope) exception handling and logging, which was
next to awful, at least in my humble opinion. Stuff should be more consistent now
- The code should have gotten somewhat cleaner. (Code clarity is a subjective
matter, though, so critique is always welcome)
Contributed by: Oleg Kalnichevski
Revision Changes Path
1.27 +235 -221
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.26
retrieving revision 1.27
diff -u -r1.26 -r1.27
--- Cookie.java 28 Nov 2002 00:21:14 -0000 1.26
+++ Cookie.java 29 Nov 2002 19:03:02 -0000 1.27
@@ -791,257 +791,271 @@
/* 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.");
+ if (path.length() == 0) {
+ log.debug("Fixing up empty request path.");
path = PATH_DELIM;
}
String defaultPath = null;
int lastSlashIndex = path.lastIndexOf(PATH_DELIM);
if(lastSlashIndex == 0){
defaultPath = PATH_DELIM;
- }else if(lastSlashIndex > 0){
+ }
+ else if(lastSlashIndex > 0) {
defaultPath = path.substring(0, lastSlashIndex);
- }else{
+ }
+ else {
defaultPath = path;
}
- HeaderElement[] headerElements =
- HeaderElement.parse(setCookie.getValue());
-
- Cookie[] cookies = new Cookie[headerElements.length];
- int index = 0;
- for (int i = 0; i < headerElements.length; i++) {
-
- Cookie cookie = new Cookie(domain,
- headerElements[i].getName(),
- headerElements[i].getValue(),
- defaultPath,
- null,
- false);
-
- // cycle through the parameters
- NameValuePair[] parameters = headerElements[i].getParameters();
- // could be null. In case only a header element and no parameters.
- if (parameters != null) {
- boolean discard_set = false, secure_set = false;
- for (int j = 0; j < parameters.length; j++) {
- String name = parameters[j].getName().toLowerCase();
-
- // check for required value parts
- if ( (name.equals("version") || name.equals("max-age") ||
- name.equals("domain") || name.equals("path") ||
- name.equals("comment") || name.equals("expires")) &&
- parameters[j].getValue() == null) {
- if(log.isDebugEnabled()) {
- log.debug("Cookie.parse(): Unable to parse set-cookie
header \"" + setCookie.getValue() + "\" because \"" + parameters[j].getName() + "\"
requires a value in cookie \"" + headerElements[i].getName() + "\".");
- }
- throw new HttpException(
- "Bad Set-Cookie header: " + setCookie.getValue() +
- "\nMissing value for " +
- parameters[j].getName() +
- " attribute in cookie '" +
- headerElements[i].getName() + "'");
- }
-
- if (name.equals("version")) {
- try {
- cookie.setVersion(
- Integer.parseInt(parameters[j].getValue()));
- } catch (NumberFormatException nfe) {
- if(log.isDebugEnabled()) {
- log.debug("Cookie.parse(): Exception attempting to
parse set-cookie header \"" + setCookie.getValue() + "\" because version attribute
value \"" + parameters[j].getValue() + "\" is not a number in cookie \"" +
headerElements[i].getName() + "\".",nfe);
+
+ try {
+ HeaderElement[] headerElements =
+ HeaderElement.parse(setCookie.getValue());
+
+ Cookie[] cookies = new Cookie[headerElements.length];
+
+ int index = 0;
+ for (int i = 0; i < headerElements.length; i++) {
+
+ HeaderElement headerelement = headerElements[i];
+ Cookie cookie = new Cookie(domain,
+ headerelement.getName(),
+ headerelement.getValue(),
+ defaultPath,
+ null,
+ false);
+
+ // cycle through the parameters
+ NameValuePair[] parameters = headerelement.getParameters();
+ // could be null. In case only a header element and no parameters.
+ if (parameters != null) {
+
+ for (int j = 0; j < parameters.length; j++) {
+
+ String param_name = parameters[j].getName().toLowerCase();
+ String param_value = parameters[j].getValue();
+
+ if (param_name.equals("version")) {
+
+ if (param_value == null) {
+ throw new HttpException("Bad cookie header: missing
value for version attribute");
}
- throw new HttpException(
- "Bad Set-Cookie header: " +
- setCookie.getValue() + "\nVersion '" +
- parameters[j].getValue() + "' not a number");
- }
- } else if (name.equals("path")) {
- cookie.setPath(parameters[j].getValue());
- cookie.setPathAttributeSpecified(true);
- } else if (name.equals("domain")) {
- String d = parameters[j].getValue().toLowerCase();
- // add leading dot if not present and if domain is
- // not the full host name
-
- // FIXME: Is this the right thing to do?
- // According to 4.3.2 of RFC 2109,
- // we should reject these.
- // I'm not sure this rejection logic
- // is RFC 2109 compliant otherwise either
- // (probably MSIE and Netscape aren't
- // either)
-
- if (d.charAt(0) != '.' && !d.equals(domain))
- cookie.setDomain("." + d);
- else
- cookie.setDomain(d);
- cookie.setDomainAttributeSpecified(true);
- } else if (name.equals("max-age")) {
- int age;
- try {
- age = Integer.parseInt(parameters[j].getValue());
- } catch (NumberFormatException e) {
- if(log.isDebugEnabled()) {
- log.debug("Cookie.parse(): Exception attempting to
parse set-cookie header \"" + setCookie.getValue() + "\" because max-age attribute
value \"" + parameters[j].getValue() + "\" is not a number in cookie \"" +
headerElements[i].getName() + "\".",e);
+ try {
+ cookie.setVersion(Integer.parseInt(param_value));
+ } catch (NumberFormatException e) {
+ throw new HttpException( "Bad cookie header:
invalid version attribute: " + e.getMessage());
}
- throw new HttpException(
- "Bad Set-Cookie header: " +
- setCookie.getValue() + " Max-Age '" +
- parameters[j].getValue() + "' not a number");
- }
- cookie.setExpiryDate(new Date(System.currentTimeMillis() +
- age * 1000L));
- } else if (name.equals("secure")) {
- cookie.setSecure(true);
- } else if (name.equals("comment")) {
- cookie.setComment(parameters[j].getValue());
- } else if (name.equals("expires")) {
- boolean set = false;
- String expiryDate = parameters[j].getValue();
- // trim single quotes around expiry if present
- // see
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=5279
- if(null != expiryDate) {
- if(expiryDate.length() > 1 &&
- expiryDate.startsWith("'") &&
- expiryDate.endsWith("'")) {
- expiryDate =
expiryDate.substring(1,expiryDate.length()-1);
+
+ }
+ else if (param_name.equals("path")) {
+
+ if (param_value == null) {
+ throw new HttpException("Bad cookie header: missing
value for path attribute");
}
- }
- for(int k=0;k<expiryFormats.length;k++) {
+ cookie.setPath(param_value);
+ cookie.setPathAttributeSpecified(true);
+
+ }
+ else if (param_name.equals("domain")) {
+
+ if (param_value == null) {
+ throw new HttpException("Bad cookie header: missing
value for domain attribute");
+ }
+ cookie.setDomain(param_value);
+ cookie.setDomainAttributeSpecified(true);
+
+ }
+ else if (param_name.equals("max-age")) {
+
+ if (param_value == null) {
+ throw new HttpException("Bad cookie header: missing
value for max-age attribute");
+ }
+ int age;
try {
- Date date = expiryFormats[k].parse(expiryDate);
- cookie.setExpiryDate(date);
- set = true;
- break;
- } catch (ParseException e) {
- if(log.isDebugEnabled()) {
- log.debug("Cookie.parse(): Exception attempting
to parse set-cookie header \"" + setCookie.getValue() + "\" because expires attribute
value \"" + parameters[j].getValue() + "\" cannot be parsed by date format \"" + k +
"\" in cookie " + headerElements[i].getName() + "\". Will try another.");
+ age = Integer.parseInt(param_value);
+ } catch (NumberFormatException e) {
+ throw new HttpException( "Bad cookie header:
invalid max-age attribute: " + e.getMessage());
+ }
+ cookie.setExpiryDate(new
Date(System.currentTimeMillis() +
+ age * 1000L));
+
+ }
+ else if (param_name.equals("secure")) {
+
+ cookie.setSecure(true);
+
+ }
+ else if (param_name.equals("comment")) {
+
+ cookie.setComment(param_value);
+
+ }
+ else if (param_name.equals("expires")) {
+
+ if (param_value == null) {
+ throw new HttpException("Bad cookie header: missing
value for expires attribute");
+ }
+ boolean set = false;
+ // trim single quotes around expiry if present
+ // see
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=5279
+ if(param_value.length() > 1 &&
+ param_value.startsWith("'") &&
+ param_value.endsWith("'")) {
+ param_value =
param_value.substring(1,param_value.length()-1);
+ }
+
+ for(int k=0;k<expiryFormats.length;k++) {
+
+ try {
+ Date date = expiryFormats[k].parse(param_value);
+ cookie.setExpiryDate(date);
+ set = true;
+ break;
+ } catch (ParseException e) {
+ //Ignore and move on
}
}
- }
- if(!set) {
- if(log.isInfoEnabled()) {
- log.info("Cookie.parse(): Unable to parse
expiration date parameter: \"" + expiryDate + "\"");
+ if(!set) {
+ throw new HttpException("Bad cookie header: unable
to parse expiration date parameter: " + param_value);
}
- throw new HttpException("Unable to parse expiration
date parameter: \"" + expiryDate + "\"");
}
}
}
- }
-
- // check version
- if (cookie.getVersion() < 0 || cookie.getVersion() > 1) {
- if(log.isInfoEnabled()) {
- log.info("Cookie.parse(): Rejecting set cookie header \"" +
setCookie.getValue() + "\" because it has an unrecognized version attribute (" +
cookie.getVersion() + ").");
- }
- throw new HttpException(
- "Bad Set-Cookie header: " + setCookie.getValue() +
- " Illegal Version attribute");
- }
-
- // security check... we musn't allow the server to give us an
- // invalid domain scope
-
- // Validate the cookies domain attribute. NOTE: Domains without any
dots are
- // allowed to support hosts on private LANs that don't have DNS names.
Since
- // they have no dots, to domain-match the request-host and domain must
be identical
- // for the cookie to sent back to the origin-server.
- if (cookie.getDomain() != null &&
!cookie.getDomain().equals("localhost") && domain.indexOf(".") >= 0) {
-
- // Not required to have at least two dots. RFC 2965.
- // A Set-Cookie2 with Domain=ajax.com will be accepted.
-
- // domain must domain match host
- if (!domain.endsWith(cookie.getDomain())){
- if(log.isInfoEnabled()) {
- log.info("Cookie.parse(): Rejecting set cookie header \"" +
setCookie.getValue() + "\" because \"" + cookie.getName() + "\" has an illegal domain
attribute (\"" + cookie.getDomain() + "\") for the domain \"" + domain + "\".");
+
+ // Check if the cookie has all required attributes. If not, apply
defaults
+ if (cookie.getDomain() == null) {
+ cookie.setDomain(domain);
+ cookie.setDomainAttributeSpecified(false);
+ if (log.isInfoEnabled()) {
+ log.info("Domain attribute not explicitly set. Default
applied: \"" + domain + "\"");
}
- throw new HttpException(
- "Bad Set-Cookie header: " + setCookie.getValue() +
- " Illegal domain attribute" + cookie.getDomain());
}
-
- if(cookie.getVersion() == 0){
- // Validate domain using Netscape cookie specification
- int domainParts = new StringTokenizer(cookie.getDomain(),
".").countTokens();
- if(isSpecialDomain(cookie.getDomain())){
- if(domainParts < 2){
- if(log.isInfoEnabled()) {
- log.info("Cookie.parse(): Rejecting set cookie
header \"" + setCookie.getValue() + "\" because \"" + cookie.getName() + "\" has an
illegal domain attribute (\"" + cookie.getDomain() + "\") for the given domain \"" +
domain + "\". It violoates the Netscape cookie specification for special TLDs.");
- }
- throw new HttpException(
- "Bad Set-Cookie header: " + setCookie.getValue() +
- " Illegal domain attribute " + cookie.getDomain());
- }
- }else{
- if(domainParts < 3){
- if(log.isInfoEnabled()) {
- log.info("Cookie.parse(): Rejecting set cookie
header \"" + setCookie.getValue() + "\" because \"" + cookie.getName() + "\" has an
illegal domain attribute (\"" + cookie.getDomain() + "\") for the given domain \"" +
domain + "\". It violoates the Netscape cookie specification for non-special TLDs.");
- }
- throw new HttpException(
- "Bad Set-Cookie header: " + setCookie.getValue() +
- " Illegal domain attribute " + cookie.getDomain());
- }
-
- }
- }else{
- // domain must have at least one embedded dot
- int dotIndex = cookie.getDomain().indexOf('.', 1);
- if(dotIndex < 0 || dotIndex == cookie.getDomain().length()-1){
- throw new HttpException("Bad set-cookie header: " +
setCookie.getValue() +
- "Illegal domain attribute " +
cookie.getDomain() +
- ". The domain contains no embedded
dots.");
- }
- // host minus domain may not contain any dots
- if (domain.substring(0,
- domain.length() -
- cookie.getDomain().length()).indexOf('.') != -1) {
- if(log.isInfoEnabled()) {
- log.info("Cookie.parse(): Rejecting set cookie header
\"" + setCookie.getValue() + "\" because \"" + cookie.getName() + "\" has an illegal
domain attribute (\"" + cookie.getDomain() + "\") for the given domain \"" + domain +
"\".");
- }
- throw new HttpException(
- "Bad Set-Cookie header: " + setCookie.getValue() +
- " Illegal domain attribute " + cookie.getDomain());
+ if (cookie.getPath() == null) {
+ cookie.setPath(defaultPath);
+ cookie.setPathAttributeSpecified(false);
+ if (log.isInfoEnabled()) {
+ log.info("Path attribute not explicitly set. Default
applied: \"" + defaultPath + "\"");
}
}
+
+ validate(cookie, domain, port, path, secure);
+
+ if(log.isDebugEnabled()){
+ log.debug("Cookie accepted: \"" + cookie.toString() +"\"");
+ }
+ cookies[index++] = cookie;
+ }
+ return cookies;
+ }
+ catch(HttpException e)
+ {
+ if (log.isInfoEnabled()) {
+ log.info("Cookie rejected: \"" + setCookie.getValue() + "\". " +
e.getMessage());
}
+ throw e;
+ }
+ }
- // another security check... we musn't allow the server to give us a
- // cookie that doesn't match this path
- if(cookie.getPath() != null && (!path.startsWith(cookie.getPath()))) {
- if(log.isInfoEnabled()) {
- log.info("Cookie.parse(): Rejecting set cookie header \"" +
setCookie.getValue() + "\" because \"" + cookie.getName() + "\" has an illegal path
attribute (\"" + cookie.getPath() + "\") for the given path \"" + path + "\".");
- }
+ private static void validate(Cookie cookie, String domain, int port, String
path, boolean secure)
+ throws HttpException {
+ log.trace("enter Cookie.validate(String, int, String, boolean)");
+ // This private method does not validate any input parameters under
assumption
+ // that parameters are properly invalidated by the caller
+
+ // check version
+ if (cookie.getVersion() < 0) {
+ throw new HttpException( "Bad cookie header: illegal version number " +
cookie.getValue());
+ }
+
+ // security check... we musn't allow the server to give us an
+ // invalid domain scope
+
+ // Validate the cookies domain attribute. NOTE: Domains without any dots
are
+ // allowed to support hosts on private LANs that don't have DNS names.
Since
+ // they have no dots, to domain-match the request-host and domain must be
identical
+ // for the cookie to sent back to the origin-server.
+ if (!cookie.getDomain().equals("localhost") && domain.indexOf(".") >= 0) {
+
+ // Not required to have at least two dots. RFC 2965.
+ // A Set-Cookie2 with Domain=ajax.com will be accepted.
+
+ // domain must domain match host
+ if (!domain.endsWith(cookie.getDomain())){
+
throw new HttpException(
- "Bad Set-Cookie header: " + setCookie.getValue() +
- " Header targets a different path, found \"" +
- cookie.getPath() + "\" for \"" + path + "\"");
+ "Bad cookie header: illegal domain attribute \"" +
cookie.getDomain() + "\". Domain of origin: \"" + domain + "\"");
+
}
- // set path if not otherwise specified
- if(null == cookie.getPath()) {
- if(null != path) {
- if(!path.endsWith(PATH_DELIM)) {
- int x = path.lastIndexOf(PATH_DELIM);
- if(0 < x) {
- cookie.setPath(path.substring(0,x));
- } else {
- cookie.setPath(PATH_DELIM);
- }
- } else {
- cookie.setPath(path);
- }
- }
+ switch(cookie.getVersion()) {
+ case 0:
+ // Validate domain using Netscape cookie specification
+ validateDomainAttribVer0(cookie, domain);
+ break;
+ default:
+ validateDomainAttribVer1(cookie, domain);
+ break;
}
+ }
+
+ // another security check... we musn't allow the server to give us a
+ // cookie that doesn't match this path
- if(log.isDebugEnabled()){
- log.debug("Cookie.parse(): Adding cookie - " + cookie.toString());
+ if(!path.startsWith(cookie.getPath())) {
+
+ throw new HttpException(
+ "Bad cookie header: illegal path attribute \"" +
cookie.getPath() + "\". Path of origin: \"" + path + "\"");
+ }
+ }
+
+ /**
+ * Validate domain attribute using Netscape cookie specification draft
+ */
+
+ private static void validateDomainAttribVer0(final Cookie cookie, final String
domain)
+ throws HttpException {
+ log.trace("enter Cookie.validateDomainAttribVer0()");
+
+ int domainParts = new StringTokenizer(cookie.getDomain(),
".").countTokens();
+
+ if (isSpecialDomain(cookie.getDomain())) {
+
+ if(domainParts < 2) {
+
+ throw new HttpException("Bad cookie header: domain attribute \""+
cookie.getDomain() + "\" violates the Netscape cookie specification for special
domains");
}
- cookies[index++] = cookie;
}
+ else {
+
+ if(domainParts < 3) {
+
+ throw new HttpException("Bad cookie header: domain attribute \""+
cookie.getDomain() + "\" violates the Netscape cookie specification");
+ }
+ }
+ }
- return cookies;
+ /**
+ * Validate domain using RFC 2109
+ */
+
+ private static void validateDomainAttribVer1(final Cookie cookie, final String
domain)
+ throws HttpException {
+ log.trace("enter Cookie.validateDomainAttribVer1()");
+
+ // domain must have at least one embedded dot
+ int dotIndex = cookie.getDomain().indexOf('.', 1);
+ if(dotIndex < 0 || dotIndex == cookie.getDomain().length()-1){
+
+ throw new HttpException("Bad cookie header: illegal domain
attribute \"" + cookie.getDomain() + "\"");
+
+ }
+ // host minus domain may not contain any dots
+ if (domain.substring(0,
+ domain.length() -
+ cookie.getDomain().length()).indexOf('.') != -1) {
+
+ throw new HttpException("Bad cookie header: illegal domain
attribute \"" + cookie.getDomain() + "\"");
+ }
}
/**
@@ -1136,7 +1150,7 @@
private boolean _hasDomainAttribute = false;
/** The version of the cookie specification I was created from. */
- private int _version = 0;
+ private int _version = 1;
// -------------------------------------------------------------- Constants
1.14 +8 -8
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.13
retrieving revision 1.14
diff -u -r1.13 -r1.14
--- TestCookie.java 28 Nov 2002 00:21:14 -0000 1.13
+++ TestCookie.java 29 Nov 2002 19:03:02 -0000 1.14
@@ -222,7 +222,7 @@
assertEquals("Domain","127.0.0.1",parsed[0].getDomain());
assertEquals("Path","/path",parsed[0].getPath());
assertTrue("Secure",!parsed[0].getSecure());
- assertEquals("Version",0,parsed[0].getVersion());
+ assertEquals("Version",1,parsed[0].getVersion());
}
/*
public void testParseNoName() throws Exception {
@@ -254,7 +254,7 @@
assertEquals("Domain","127.0.0.1",parsed[0].getDomain());
assertEquals("Path","/",parsed[0].getPath());
assertTrue("Secure",!parsed[0].getSecure());
- assertEquals("Version",0,parsed[0].getVersion());
+ assertEquals("Version",1,parsed[0].getVersion());
}
public void testParseWithWhiteSpace() throws Exception {
@@ -443,7 +443,7 @@
}
public void testParseWithWrongNetscapeDomain2() throws Exception {
- Header setCookie = new Header("Set-Cookie","cookie-name=cookie-value;
domain=.y.z");
+ Header setCookie = new Header("Set-Cookie","cookie-name=cookie-value;
version=0; domain=.y.z");
try {
Cookie[] parsed = Cookie.parse("x.y.z","/",setCookie);
fail("HttpException exception should have been thrown");
@@ -695,7 +695,7 @@
*/
public void testDomainCaseInsensitivity() {
Header setCookie = new Header(
- "Set-Cookie", "name=value; domain=.sybase.com; path=/");
+ "Set-Cookie", "name=value; path=/; domain=.sybase.com");
try {
Cookie[] parsed = Cookie.parse("www.Sybase.com", "/", false, setCookie
);
}
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
