You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Couball, James" <Ja...@cotelligent.com> on 2003/02/13 23:28:20 UTC
RE: HttpMethodBase::ParseResponseHeaders [Patch]
I would disagree with your interpretation. RFC2109 states:
=============================
4.3.2 Rejecting Cookies
To prevent possible security or privacy violations, a user agent rejects a
cookie (shall not store its information) if any of the following is true:
* The value for the Path attribute is not a prefix of the request-
URI.
* The value for the Domain attribute contains no embedded dots or
does not start with a dot.
* The value for the request-host does not domain-match the Domain
attribute.
* The request-host is a FQDN (not IP address) and has the form HD,
where D is the value of the Domain attribute, and H is a string
that contains one or more dots.
=============================
This says "rejects a cookie" not all the cookies in the header. I concede
that the part you quoted can be interpreted the way you did, but I don't
give it as much weight because it is an 'Examples' section. If you ask me,
these examples are inconsistent with the rest of the spec.
Note that the current implementation follows neither what you nor I are
proposing. Instead, it is in the middle. When there is a validation error,
some cookies get discarded and some don't.
In any case, RFC's are nice, but people who write servers tend to specialize
their cookies to what is accepted by the user agent. Both IE and
Netscape/Mozilla have the capability to accept some cookies in the header
and not others.
In fact, my application is a screen (html) scraper that depends on being
logged in. The site I am scraping (which is very popular -- has many users
who use the site without cookie problems) does this exact thing where it
sends two cookies in one set-cookie header: the first for a different domain
(evil marketing practice) and the second one that contains a session state
key.
I have appended the unidiff of my changes.
Sincerely,
James.
Index: HttpMethodBase.java
===================================================================
RCS file:
/home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpc
lient/HttpMethodBase.java,v
retrieving revision 1.111
diff -u -r1.111 HttpMethodBase.java
--- HttpMethodBase.java 11 Feb 2003 03:23:05 -0000 1.111
+++ HttpMethodBase.java 13 Feb 2003 22:22:44 -0000
@@ -75,6 +75,7 @@
import org.apache.commons.httpclient.cookie.CookiePolicy;
import org.apache.commons.httpclient.cookie.CookieSpec;
+import org.apache.commons.httpclient.cookie.MalformedCookieException;
import org.apache.commons.httpclient.util.HeaderParser;
import org.apache.commons.httpclient.util.URIUtil;
import org.apache.commons.logging.Log;
@@ -1589,43 +1590,69 @@
// add cookies, if any
// should we set cookies?
- Header setCookieHeader = getResponseHeader("set-cookie2");
+ String cookieHeaderName = "set-cookie2";
+ Header setCookieHeader = getResponseHeader(cookieHeaderName);
if (null == setCookieHeader) { //ignore old-style if new is
supported
- setCookieHeader = getResponseHeader("set-cookie");
+ cookieHeaderName = "set-cookie";
+ setCookieHeader = getResponseHeader(cookieHeaderName);
}
- if (setCookieHeader == null) {
- return;
- }
- try {
+ if (setCookieHeader != null) {
- CookieSpec parser =
CookiePolicy.getSpecByPolicy(state.getCookiePolicy());
- Cookie[] cookies = parser.parse(
- conn.getHost(),
- conn.getPort(),
- getPath(),
- conn.isSecure(),
- setCookieHeader);
- for (int i = 0; i < cookies.length; i++) {
- Cookie cookie = cookies[i];
- parser.validate(
- conn.getHost(),
- conn.getPort(),
- getPath(),
- conn.isSecure(),
- cookie);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Cookie accepted: \""
- + parser.formatCookie(cookie) + "\"");
+ // Parse cookies -- an error parsing the set-cookie header
dumps all
+ // cookies in this header.
+
+ CookieSpec parser = CookiePolicy.getSpecByPolicy(state.
+ getCookiePolicy());
+ Cookie[] cookies = null;
+ try {
+ cookies = parser.parse(
+ conn.getHost(),
+ conn.getPort(),
+ getPath(),
+ conn.isSecure(),
+ setCookieHeader);
+ }
+ catch (MalformedCookieException e) {
+ if (LOG.isWarnEnabled()) {
+ LOG.warn("Could not parse " + cookieHeaderName +
+ " header: \""
+ + setCookieHeader.getValue()
+ + "\". " + e.getMessage());
}
- state.addCookie(cookie);
}
- } catch (HttpException e) {
- if (LOG.isWarnEnabled()) {
- LOG.warn("Cookie rejected: \""
- + setCookieHeader.getValue()
- + "\". " + e.getMessage());
+ // Validate cookies -- only valid cookies are added. Invalid
+ // cookies are logged and ignored.
+
+ if (cookies != null) {
+ for (int i = 0; i < cookies.length; i++) {
+ Cookie cookie = cookies[i];
+ boolean accepted = true;
+ try {
+ parser.validate(
+ conn.getHost(),
+ conn.getPort(),
+ getPath(),
+ conn.isSecure(),
+ cookie);
+ }
+ catch (MalformedCookieException e) {
+ accepted = false;
+ if (LOG.isWarnEnabled()) {
+ LOG.warn("Cookie rejected: \""
+ + parser.formatCookie(cookie)
+ + "\". " + e.getMessage());
+ }
+ }
+ if (accepted) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Cookie accepted: \""
+ + parser.formatCookie(cookie) +
"\"");
+ }
+ state.addCookie(cookie);
+ }
+ }
}
}
}
-----Original Message-----
From: Oleg Kalnichevski [mailto:o.kalnichevski@dplanet.ch]
Sent: Thursday, February 13, 2003 1:43 PM
To: Commons HttpClient Project
Subject: Re: HttpMethodBase::ParseResponseHeaders handling of cookies
Jandalf, James
The wording of the RFC2109 is unsurprisingly vague, however, in my
opinion HttpClient is correct in rejecting the entire set Set-Cookie
header.
See the following quote from the RFC2109:
=============================
4.3.2 Rejecting Cookies
...
Examples:
* A Set-Cookie from request-host y.x.foo.com for Domain=.foo.com
would be rejected, because H is y.x and contains a dot.
* A Set-Cookie from request-host x.foo.com for Domain=.foo.com would
be accepted.
* A Set-Cookie with Domain=.com or Domain=.com., will always be
rejected, because there is no embedded dot.
* A Set-Cookie with Domain=ajax.com will be rejected because the
value for Domain does not begin with a dot.
===========================
I tend to interpret the above stated statements as referring to the
Set-Cookie header, rather than an individual cookie. Please let me know
if you see it differently
Faithfully yours,
Cookie Taliban
On Thu, 2003-02-13 at 21:02, Jeffrey Dever wrote:
> Good argument. I'd say you are right that cookies should be
> accepted/rejected based on individual merits and not on the entire
> cookie header. A patch (in unidiff format) would be helpful in
> evaluation what you propose to change.
>
> Jandalf.
>
>
> Couball, James wrote:
>
> >Hello All,
> >
> >I have a problem with my application of HTTPClient relating to the way
that
> >HttpMethodBase::ParseResponseHeaders handles rejecting cookies.
> >
> >My problem is that when one cookie in the set-cookie(2) header is
considered
> >invalid (call to parser.validate throws an exception) (because the domain
is
> >for a third party, for example) all cookies in the header that haven't
been
> >process are dropped. In my application, I want to reject cookies that
don't
> >match the domain and accept cookies that do match the domain. This
problem
> >can not be solved with a new cookie policy because the problem is in how
> >HttpMethodBase::ParseResponseHeaders handles the exception thrown by
> >parser.validate.
> >
> >RFC 2965 seems to suggest that accepting some cookies in the Set-Cookie2
> >header and rejecting others is ok. See section 3.3.2: "To prevent
possible
> >security or privacy violations, a user agent rejects A COOKIE according
to
> >rules below." (emphasis is mine)
> >
> >In addition, IE and Netscape do accept all of the valid cookies on a
> >Set-Cookie(2) header. What is a valid cookie to IE and Netscape depends
on
> >how you set the cookie policy within that program and is more complicated
> >that what HttpClient currently supports.
> >
> >If this is a desired change, I have attached my implementation of
> >HttpMethodBase::ParseResponseHeaders to be added to HttpClient. If
> >requested, I can also provide a patch.
> >
> >Sincerely,
> >James.
> >
> >protected void processResponseHeaders(HttpState state,
> > HttpConnection conn) {
> > LOG.trace("enter HttpMethodBase.processResponseHeaders(HttpState, "
> > + "HttpConnection)");
> >
> > // add cookies, if any
> > // should we set cookies?
> > String cookieHeaderName = "set-cookie2";
> > Header setCookieHeader = getResponseHeader(cookieHeaderName);
> > if (null == setCookieHeader) { //ignore old-style if new is supported
> > cookieHeaderName = "set-cookie";
> > setCookieHeader = getResponseHeader(cookieHeaderName);
> > }
> >
> > if (setCookieHeader != null) {
> >
> > // Parse cookies -- an error parsing the set-cookie header dumps
all
> > // cookies in this header.
> >
> > CookieSpec parser =
> >CookiePolicy.getSpecByPolicy(state.getCookiePolicy());
> > Cookie[] cookies = null;
> > try {
> > cookies = parser.parse(
> > conn.getHost(),
> > conn.getPort(),
> > getPath(),
> > conn.isSecure(),
> > setCookieHeader);
> > }
> > catch (MalformedCookieException e) {
> > if (LOG.isWarnEnabled()) {
> > LOG.warn("Could not parse " + cookieHeaderName + " header: \""
> > + setCookieHeader.getValue()
> > + "\". " + e.getMessage());
> > }
> > }
> >
> > // Validate cookies -- only valid cookies are added. Invalid
cookies
> > // are logged and ignored.
> >
> > if (cookies != null) {
> > for (int i = 0; i < cookies.length; i++) {
> > Cookie cookie = cookies[i];
> > boolean accepted = true;
> > try {
> > parser.validate(
> > conn.getHost(),
> > conn.getPort(),
> > getPath(),
> > conn.isSecure(),
> > cookie);
> > }
> > catch (MalformedCookieException e) {
> > accepted = false;
> > if (LOG.isWarnEnabled()) {
> > LOG.warn("Cookie rejected: \""
> > + parser.formatCookie(cookie)
> > + "\". " + e.getMessage());
> > }
> > }
> > if (accepted) {
> > if (LOG.isDebugEnabled()) {
> > LOG.debug("Cookie accepted: \""
> > + parser.formatCookie(cookie) + "\"");
> > }
> > state.addCookie(cookie);
> > }
> > }
> > }
> > }
> >}
> >
> >
> >---------------------------------------------------------------------
> >To unsubscribe, e-mail:
commons-httpclient-dev-unsubscribe@jakarta.apache.org
> >For additional commands, e-mail:
commons-httpclient-dev-help@jakarta.apache.org
> >
> >
> >
> >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail:
commons-httpclient-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail:
commons-httpclient-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail:
commons-httpclient-dev-help@jakarta.apache.org
RE: HttpMethodBase::ParseResponseHeaders [Patch]
Posted by Oleg Kalnichevski <o....@dplanet.ch>.
James
I concede you have presented a very convincing case. Besides, you are
right about current implementation not meeting my own interpretation of
the RFC.
Many thanks for tracking the bug down
Kind regards
Oleg
On Thu, 2003-02-13 at 23:28, Couball, James wrote:
> I would disagree with your interpretation. RFC2109 states:
>
> =============================
> 4.3.2 Rejecting Cookies
>
> To prevent possible security or privacy violations, a user agent rejects a
> cookie (shall not store its information) if any of the following is true:
>
>
> * The value for the Path attribute is not a prefix of the request-
> URI.
>
> * The value for the Domain attribute contains no embedded dots or
> does not start with a dot.
>
> * The value for the request-host does not domain-match the Domain
> attribute.
>
> * The request-host is a FQDN (not IP address) and has the form HD,
> where D is the value of the Domain attribute, and H is a string
> that contains one or more dots.
>
> =============================
>
> This says "rejects a cookie" not all the cookies in the header. I concede
> that the part you quoted can be interpreted the way you did, but I don't
> give it as much weight because it is an 'Examples' section. If you ask me,
> these examples are inconsistent with the rest of the spec.
>
> Note that the current implementation follows neither what you nor I are
> proposing. Instead, it is in the middle. When there is a validation error,
> some cookies get discarded and some don't.
>
> In any case, RFC's are nice, but people who write servers tend to specialize
> their cookies to what is accepted by the user agent. Both IE and
> Netscape/Mozilla have the capability to accept some cookies in the header
> and not others.
>
> In fact, my application is a screen (html) scraper that depends on being
> logged in. The site I am scraping (which is very popular -- has many users
> who use the site without cookie problems) does this exact thing where it
> sends two cookies in one set-cookie header: the first for a different domain
> (evil marketing practice) and the second one that contains a session state
> key.
>
> I have appended the unidiff of my changes.
>
> Sincerely,
> James.