[prev in list] [next in list] [prev in thread] [next in thread]
List: jakarta-commons-dev
Subject: cvs commit: jakarta-commons/httpclient/src/test/org/apache/commons/httpclient TestCookie.java
From: jsdever () apache ! org
Date: 2002-11-29 19:03:03
[Download RAW message or body]
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:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic