This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/stormcrawler.git
The following commit(s) were added to refs/heads/main by this push:
new 88fcf3bb Scope cookies to the host that set them (#2075)
88fcf3bb is described below
commit 88fcf3bb2c6919d79937e851ca9e13493acfdcf3
Author: Richard Zowalla <[email protected]>
AuthorDate: Sat Aug 29 13:21:56 2026 +0200
Scope cookies to the host that set them (#2075)
CookieConverter only checked the domain when the cookie carried a Domain
attribute, so a cookie without one was sent to any target URL, a Domain
which covers every host under it such as "com" or "co.uk" was accepted as
a scope, and checkDomainMatchToUrl returned true when it threw. getCookies
now takes the URL whose response set the cookies, keeps a cookie without a
Domain attribute for that host only, and fails closed on error.
A Domain attribute is normalised with IDN.toASCII and validated before it
is used, so the unicode and punycode forms of a name are interchangeable
and a malformed value such as "com.." is rejected rather than matching
every host under "com". The public suffix list shipped with
crawler-commons, its private section included, decides whether a domain
may cover subdomains at all; a domain the list does not know about, e.g.
an internal name under ".local", falls back to the plain suffix match so
that unlisted domains keep working. A domain which can not own subdomains
is ignored and the cookie is bound to the host, as RFC 6265 5.3 requires,
so a single label intranet domain keeps its cookies while "com" no longer
scopes anything. An address is only matched by itself, per RFC 6265 5.1.3,
instead of covering the hosts under it as "2.3.4" covered "9.2.3.4".
The protocols now record which host set the cookies. When a response
carries Set-Cookie, okhttp and playwright write its url next to the header
as set-cookie-origin, dropping any key of that name from the response
first so that a response can not choose where its cookies are sent. The
key has to travel with protocol.set-cookie for host-only cookies to be
sent back, so it is added to the metadata.persist and metadata.transfer
examples, and a warning naming both keys is logged once when cookies are
present without it. The metadata.transfer example in internals.adoc named
set-cookie instead of protocol.set-cookie and is corrected too.
A Set-Cookie header whose first token has no "=" threw
StringIndexOutOfBoundsException out of getCookies, failing the whole
fetch, and is now skipped. Cookies dropped on the path, secure and expiry
checks are logged like the other ones.
---
.../protocol/AbstractHttpProtocol.java | 7 +
.../stormcrawler/protocol/okhttp/HttpProtocol.java | 54 ++-
.../apache/stormcrawler/util/CookieConverter.java | 278 ++++++++++-
core/src/main/resources/crawler-default.yaml | 4 +-
.../protocol/okhttp/HttpProtocolCookieTest.java | 204 ++++++++
.../stormcrawler/util/CookieConverterTest.java | 511 ++++++++++++++++++++-
.../stormcrawler/util/MetadataTransferTest.java | 38 ++
docs/src/main/asciidoc/configuration.adoc | 2 +-
docs/src/main/asciidoc/internals.adoc | 12 +-
.../protocol/playwright/HttpProtocol.java | 18 +
10 files changed, 1087 insertions(+), 41 deletions(-)
diff --git
a/core/src/main/java/org/apache/stormcrawler/protocol/AbstractHttpProtocol.java
b/core/src/main/java/org/apache/stormcrawler/protocol/AbstractHttpProtocol.java
index f847d567..a2f37b70 100644
---
a/core/src/main/java/org/apache/stormcrawler/protocol/AbstractHttpProtocol.java
+++
b/core/src/main/java/org/apache/stormcrawler/protocol/AbstractHttpProtocol.java
@@ -70,6 +70,13 @@ public abstract class AbstractHttpProtocol implements
Protocol {
protected static final String RESPONSE_COOKIES_HEADER = "set-cookie";
+ /**
+ * Metadata key holding the url whose response carried the cookies stored
under {@link
+ * #RESPONSE_COOKIES_HEADER}. It is written by the protocol and not taken
from a response
+ * header, and is used to scope cookies without a domain attribute to the
host which set them.
+ */
+ protected static final String RESPONSE_COOKIES_ORIGIN =
"set-cookie-origin";
+
protected static final String SET_HEADER_BY_REQUEST = "set-header";
protected String protocolMetadataPrefix = "";
diff --git
a/core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java
b/core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java
index ba60d7f9..90cb4d74 100644
---
a/core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java
+++
b/core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java
@@ -23,6 +23,7 @@ import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.MalformedURLException;
import java.net.Proxy;
+import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.security.cert.CertificateException;
import java.util.ArrayList;
@@ -35,6 +36,7 @@ import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLSession;
@@ -103,6 +105,9 @@ public class HttpProtocol extends AbstractHttpProtocol {
// track the time spent for each URL in DNS resolution
private final Map<String, Long> DNStimes = new ConcurrentHashMap<>();
+ // makes sure that a missing cookie origin is reported once and not for
every url
+ private final AtomicBoolean missingCookieOriginLogged = new
AtomicBoolean();
+
private OkHttpClient.Builder builder;
private static final TrustManager[] trustAllCerts =
@@ -287,7 +292,8 @@ public class HttpProtocol extends AbstractHttpProtocol {
}
try {
final List<Cookie> cookies =
- CookieConverter.getCookies(cookieStrings,
URLUtil.toURL(url));
+ CookieConverter.getCookies(
+ cookieStrings, getCookieOrigin(md, url),
URLUtil.toURL(url));
for (Cookie c : cookies) {
rb.addHeader("Cookie", c.getName() + "=" + c.getValue());
}
@@ -295,6 +301,42 @@ public class HttpProtocol extends AbstractHttpProtocol {
}
}
+ /**
+ * Returns the url whose response set the cookies, or null when it was not
recorded, in which
+ * case cookies without a domain attribute are not sent.
+ */
+ private URL getCookieOrigin(Metadata md, String url) {
+ final String origin = md.getFirstValue(RESPONSE_COOKIES_ORIGIN,
protocolMetadataPrefix);
+ if (StringUtils.isBlank(origin)) {
+ if (missingCookieOriginLogged.compareAndSet(false, true)) {
+ LOG.warn(
+ "No {}{} for {}, cookies without a domain attribute
are not sent. Add {}{}"
+ + " to metadata.transfer and metadata.persist
next to {}{}.",
+ protocolMetadataPrefix,
+ RESPONSE_COOKIES_ORIGIN,
+ url,
+ protocolMetadataPrefix,
+ RESPONSE_COOKIES_ORIGIN,
+ protocolMetadataPrefix,
+ RESPONSE_COOKIES_HEADER);
+ } else {
+ LOG.debug("No {}{} for {}", protocolMetadataPrefix,
RESPONSE_COOKIES_ORIGIN, url);
+ }
+ return null;
+ }
+ try {
+ return URLUtil.toURL(origin);
+ } catch (MalformedURLException e) {
+ LOG.warn(
+ "Invalid {}{} {} for {}",
+ protocolMetadataPrefix,
+ RESPONSE_COOKIES_ORIGIN,
+ origin,
+ url);
+ return null;
+ }
+ }
+
protected void addHeadersToRequest(Builder rb, Metadata md) {
final String[] headerStrings = md.getValues(SET_HEADER_BY_REQUEST,
protocolMetadataPrefix);
@@ -445,6 +487,16 @@ public class HttpProtocol extends AbstractHttpProtocol {
responsemetadata.addValue(key.toLowerCase(Locale.ROOT), value);
}
+ // the Set-Cookie header does not say which host sent it: record
the url of this
+ // response so that the cookies can be scoped to it when they are
sent back. The
+ // key is dropped first so that a server sending a header of that
name can not
+ // forge the origin of the cookies inherited from another page.
+ responsemetadata.remove(RESPONSE_COOKIES_ORIGIN);
+ if (responsemetadata.getFirstValue(RESPONSE_COOKIES_HEADER) !=
null) {
+ responsemetadata.setValue(
+ RESPONSE_COOKIES_ORIGIN,
response.request().url().toString());
+ }
+
final MutableObject<TrimmedContentReason> trimmed =
new MutableObject<>(TrimmedContentReason.NOT_TRIMMED);
final byte[] bytes = toByteArray(response.body(), pageMaxContent,
trimmed);
diff --git
a/core/src/main/java/org/apache/stormcrawler/util/CookieConverter.java
b/core/src/main/java/org/apache/stormcrawler/util/CookieConverter.java
index 5592fd50..e8501b81 100644
--- a/core/src/main/java/org/apache/stormcrawler/util/CookieConverter.java
+++ b/core/src/main/java/org/apache/stormcrawler/util/CookieConverter.java
@@ -17,6 +17,8 @@
package org.apache.stormcrawler.util;
+import crawlercommons.domains.EffectiveTldFinder;
+import java.net.IDN;
import java.net.URL;
import java.util.ArrayList;
import java.util.Date;
@@ -31,15 +33,49 @@ public class CookieConverter {
private static final org.slf4j.Logger LOG =
org.slf4j.LoggerFactory.getLogger(CookieConverter.class);
+ /** Maximum length of a domain name in ASCII characters, see RFC 1034. */
+ private static final int MAX_DOMAIN_LENGTH = 253;
+
/**
* Get a list of cookies based on the cookies string taken from response
header and the target
- * url.
+ * url. As the host which set the cookies is unknown, cookies without a
domain attribute are
+ * dropped instead of being sent to the target url.
*
* @param cookiesStrings the value(s) of the http header for "Cookie" in
the http response.
* @param targetURL the url for which we wish to pass the cookies in the
request.
* @return List off cookies to add to the request.
+ * @deprecated use {@link #getCookies(String[], URL, URL)} instead
*/
+ @Deprecated
public static List<Cookie> getCookies(String[] cookiesStrings, URL
targetURL) {
+ return getCookies(cookiesStrings, null, targetURL);
+ }
+
+ /**
+ * Get a list of cookies based on the cookies string taken from response
header, the url which
+ * set them and the target url.
+ *
+ * <p>A cookie without a domain attribute is only returned when the target
url has the same host
+ * as the url which set the cookie, as required by RFC 6265.
+ *
+ * <p>A cookie with a domain attribute is only returned when both urls
match that domain.
+ * Malformed domain attributes are rejected. A domain which is itself a
public suffix, looked up
+ * in the public suffix list shipped with crawler-commons and including
its private section such
+ * as "github.io", or which is made of a single label, does not cover
subdomains: the cookie is
+ * then bound to the host it was set on, as browsers do. Domains which the
list does not know
+ * about, such as internal names under ".local" or ".internal", keep the
plain suffix match so
+ * that they remain usable as a cookie scope. A public suffix missing from
the list, whether
+ * because the bundled list is a snapshot or because the suffix is
genuinely internal, therefore
+ * still behaves as a normal domain.
+ *
+ * @param cookiesStrings the value(s) of the http header for "Cookie" in
the http response.
+ * @param originURL the url whose response set the cookies, or null when
it is unknown. The
+ * okhttp protocol takes it from the metadata written next to the
cookies when a response
+ * carries them.
+ * @param targetURL the url for which we wish to pass the cookies in the
request.
+ * @return List off cookies to add to the request.
+ */
+ public static List<Cookie> getCookies(String[] cookiesStrings, URL
originURL, URL targetURL) {
ArrayList<Cookie> list = new ArrayList<>();
for (String cs : cookiesStrings) {
@@ -55,6 +91,11 @@ public class CookieConverter {
String[] tokens = cs.split(";");
int equals = tokens[0].indexOf("=");
+ if (equals < 1) {
+ // no name=value pair to take: the header is not usable
+ LOG.debug("Skipping cookie: no name in {}", tokens[0]);
+ continue;
+ }
name = tokens[0].substring(0, equals);
value = tokens[0].substring(equals + 1);
@@ -77,10 +118,49 @@ public class CookieConverter {
BasicClientCookie cookie = new BasicClientCookie(name, value);
// check domain
- if (domain != null) {
+ if (domain != null && !domain.isBlank()) {
+ final String scope = normaliseDomain(domain);
+ if (scope == null) {
+ LOG.debug("Skipping cookie {}: malformed domain {}", name,
domain);
+ continue;
+ }
+
cookie.setDomain(domain);
- if (!checkDomainMatchToUrl(domain, targetURL.getHost())) {
+ // a domain which can not own subdomains, i.e. a public suffix
such as
+ // "com" or "co.uk" or a single label, binds the cookie to the
host itself
+ final boolean subdomains = allowsSubdomains(scope);
+
+ if (!hostMatches(scope, targetURL.getHost(), subdomains)) {
+ LOG.debug(
+ "Skipping cookie {}: domain {} does not cover the
target {}",
+ name,
+ domain,
+ targetURL);
+ continue;
+ }
+
+ // the host which set the cookie must be covered by the domain
too
+ if (originURL != null && !hostMatches(scope,
originURL.getHost(), subdomains)) {
+ LOG.debug(
+ "Skipping cookie {}: domain {} does not cover {},
which set it",
+ name,
+ domain,
+ originURL);
+ continue;
+ }
+ } else {
+ // host only cookie: valid for the host which set it and
nothing else
+ if (originURL == null) {
+ LOG.debug("Skipping cookie {}: the host which set it is
unknown", name);
+ continue;
+ }
+ if
(!originURL.getHost().equalsIgnoreCase(targetURL.getHost())) {
+ LOG.debug(
+ "Skipping cookie {}: set by {} and not valid for
{}",
+ name,
+ originURL,
+ targetURL);
continue;
}
}
@@ -92,6 +172,11 @@ public class CookieConverter {
if (!path.equals("")
&& !path.equals("/")
&& !targetURL.getPath().startsWith(path)) {
+ LOG.debug(
+ "Skipping cookie {}: path {} does not cover the
target {}",
+ name,
+ path,
+ targetURL);
continue;
}
}
@@ -101,6 +186,7 @@ public class CookieConverter {
cookie.setSecure(secure);
if (!targetURL.getProtocol().equalsIgnoreCase("https")) {
+ LOG.debug("Skipping cookie {}: secure and {} is not
https", name, targetURL);
continue;
}
}
@@ -114,6 +200,7 @@ public class CookieConverter {
// check that it hasn't expired?
if (cookie.isExpired(new Date())) {
+ LOG.debug("Skipping cookie {}: expired on {}",
name, expires);
continue;
}
}
@@ -130,33 +217,182 @@ public class CookieConverter {
}
/**
- * Helper method to check if url matches a cookie domain.
+ * Normalises the value of a cookie domain attribute into the domain it
scopes the cookie to.
*
- * @param cookieDomain the domain in the cookie
- * @param urlHostName the host name of the url
- * @return does the cookie match the host name
+ * @param domain the value of the domain attribute
+ * @return the lower cased domain, without the leading dot allowed by RFC
6265 and without the
+ * root label, or null when it is not a well formed domain name
*/
- public static boolean checkDomainMatchToUrl(String cookieDomain, String
urlHostName) {
- try {
- if (cookieDomain.startsWith(".")) {
- cookieDomain = cookieDomain.substring(1);
+ private static String normaliseDomain(String domain) {
+ String d = domain.trim().toLowerCase(Locale.ROOT);
+ // RFC 6265 5.2.3 allows exactly one leading dot
+ if (d.startsWith(".")) {
+ d = d.substring(1);
+ }
+ d = toAscii(stripRootDot(d));
+ if (d == null || d.length() > MAX_DOMAIN_LENGTH) {
+ return null;
+ }
+ for (String label : d.split("\\.", -1)) {
+ if (!isValidLabel(label)) {
+ return null;
}
- String[] domainTokens = cookieDomain.split("\\.");
- String[] hostTokens = urlHostName.split("\\.");
+ }
+ return d;
+ }
+
+ /**
+ * Converts a name to the ascii form in which the public suffix list and
the hosts are compared,
+ * so that a domain attribute written in unicode, e.g. "münchen.de",
scopes its cookie to
+ * "xn--mnchen-3ya.de" and the two forms are interchangeable.
+ *
+ * @param name a lower cased name, without its root label
+ * @return the ascii name, or null when it is not a well formed domain
name, e.g. "com.."
+ */
+ private static String toAscii(String name) {
+ if (name.isEmpty()) {
+ return null;
+ }
+ try {
+ // rejects the empty labels which the default split drops, so that
a domain such as
+ // "com.." can not reach every host under "com"
+ return IDN.toASCII(name, IDN.ALLOW_UNASSIGNED);
+ } catch (IllegalArgumentException e) {
+ return null;
+ }
+ }
- int tokenDif = hostTokens.length - domainTokens.length;
- if (tokenDif < 0) {
+ /** Checks that an ascii domain label is made of letters, digits and inner
hyphens. */
+ private static boolean isValidLabel(String label) {
+ if (label.isEmpty() || label.length() >
EffectiveTldFinder.MAX_DOMAIN_LENGTH_PART) {
+ return false;
+ }
+ if (label.charAt(0) == '-' || label.charAt(label.length() - 1) == '-')
{
+ return false;
+ }
+ for (int i = 0; i < label.length(); i++) {
+ final char c = label.charAt(i);
+ if (c != '-' && (c < 'a' || c > 'z') && (c < '0' || c > '9')) {
return false;
}
+ }
+ return true;
+ }
- for (int i = domainTokens.length - 1; i >= 0; i--) {
- if (!domainTokens[i].equalsIgnoreCase(hostTokens[i +
tokenDif])) {
- return false;
- }
- }
+ /**
+ * Checks whether a name is an address rather than a domain name. RFC 6265
5.1.3 only lets a
+ * cookie go back to that exact address, so an address never covers
anything below it.
+ *
+ * @param name a host name or a normalised cookie domain
+ * @return true when the name is an ipv4 or ipv6 address
+ */
+ private static boolean isAddress(String name) {
+ if (name.isEmpty()) {
+ return false;
+ }
+ // an ipv6 literal, bracketed as URL.getHost() returns it or bare
+ if (name.charAt(0) == '[' || name.indexOf(':') >= 0) {
return true;
- } catch (Exception e) {
+ }
+ // a top level domain is never made of digits only, so this is the
last part of an
+ // address, e.g. "1.2.3.4" but also a truncation of it such as "2.3.4"
+ final String last = name.substring(name.lastIndexOf('.') + 1);
+ if (last.isEmpty()) {
+ return false;
+ }
+ for (int i = 0; i < last.length(); i++) {
+ final char c = last.charAt(i);
+ if (c < '0' || c > '9') {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Checks whether a cookie can be scoped to every host under a domain. The
public suffix list
+ * shipped with crawler-commons is used, its private section included, so
that domains such as
+ * "github.io" are treated as suffixes like browsers do. A domain which
the list does not know
+ * about, e.g. an internal name under ".local", falls back to the plain
suffix match so that
+ * unlisted domains keep working.
+ *
+ * @param domain a domain returned by {@link #normaliseDomain(String)}
+ * @return true when hosts below the domain are covered by it
+ */
+ private static boolean allowsSubdomains(String domain) {
+ if (domain.indexOf('.') < 0) {
+ // a single label is either an intranet host name or a bare top
level domain
+ return false;
+ }
+ if (isAddress(domain)) {
+ return false;
+ }
+ if (EffectiveTldFinder.getEffectiveTLD(domain, false) == null) {
return true;
}
+ // the domain is covered by a public suffix: usable only when it sits
below it
+ return EffectiveTldFinder.getAssignedDomain(domain, true, false) !=
null;
+ }
+
+ /** Checks whether a host is covered by a cookie domain. */
+ private static boolean hostMatches(String domain, String host, boolean
subdomains) {
+ if (host == null) {
+ return false;
+ }
+ final String h = stripRootDot(host.toLowerCase(Locale.ROOT));
+ if (isAddress(h)) {
+ // RFC 6265 5.1.3: an address is only matched by itself
+ return domain.equals(h);
+ }
+ final String ascii = toAscii(h);
+ if (ascii == null) {
+ return false;
+ }
+ if (subdomains) {
+ return checkDomainMatchToUrl(domain, ascii);
+ }
+ // RFC 6265 5.3: a domain attribute which can not own subdomains is
ignored and the
+ // cookie is bound to the host it was set on
+ return domain.equals(ascii);
+ }
+
+ /** Removes the root label from a domain name, e.g. "example.com." becomes
"example.com". */
+ private static String stripRootDot(String name) {
+ return name.endsWith(".") ? name.substring(0, name.length() - 1) :
name;
+ }
+
+ /**
+ * Helper method to check if url matches a cookie domain. It only answers
whether the host falls
+ * under the domain and does not decide whether the domain may be used as
a cookie scope, e.g.
+ * "com" matches "example.com".
+ *
+ * @param cookieDomain the domain in the cookie
+ * @param urlHostName the host name of the url
+ * @return does the cookie match the host name
+ */
+ public static boolean checkDomainMatchToUrl(String cookieDomain, String
urlHostName) {
+ if (cookieDomain == null || urlHostName == null) {
+ return false;
+ }
+ if (cookieDomain.startsWith(".")) {
+ cookieDomain = cookieDomain.substring(1);
+ }
+ // the negative limit keeps the empty labels which the default split
drops, so that
+ // a domain such as "com.." can not match every host under "com"
+ final String[] domainTokens = stripRootDot(cookieDomain).split("\\.",
-1);
+ final String[] hostTokens = stripRootDot(urlHostName).split("\\.", -1);
+
+ final int tokenDif = hostTokens.length - domainTokens.length;
+ if (tokenDif < 0) {
+ return false;
+ }
+
+ for (int i = domainTokens.length - 1; i >= 0; i--) {
+ if (domainTokens[i].isEmpty()
+ || !domainTokens[i].equalsIgnoreCase(hostTokens[i +
tokenDif])) {
+ return false;
+ }
+ }
+ return true;
}
}
diff --git a/core/src/main/resources/crawler-default.yaml
b/core/src/main/resources/crawler-default.yaml
index 0343f644..5293b8a3 100644
--- a/core/src/main/resources/crawler-default.yaml
+++ b/core/src/main/resources/crawler-default.yaml
@@ -73,7 +73,9 @@ config:
# Lists the metadata to transfer to outlinks
# Used by Fetcher and SiteMapParser for redirections,
- # discovered links, passing cookies to child pages, etc.
+ # discovered links, passing cookies to child pages
+ # (protocol.set-cookie and protocol.set-cookie-origin,
+ # which have to be transferred together), etc.
# These are also persisted for the parent document (see below).
# Allows wildcards, eg. "follow.*" transfers all metadata starting with
"follow.".
# metadata.transfer:
diff --git
a/core/src/test/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocolCookieTest.java
b/core/src/test/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocolCookieTest.java
new file mode 100644
index 00000000..a30d5248
--- /dev/null
+++
b/core/src/test/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocolCookieTest.java
@@ -0,0 +1,204 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.stormcrawler.protocol.okhttp;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+import org.apache.storm.Config;
+import org.apache.stormcrawler.Metadata;
+import org.apache.stormcrawler.protocol.AbstractProtocolTest;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Checks that the url whose response set the cookies is recorded in the
metadata and used to send
+ * cookies without a domain attribute back to the host which set them.
+ */
+class HttpProtocolCookieTest extends AbstractProtocolTest {
+
+ private static final String ORIGIN_KEY = "set-cookie-origin";
+
+ private static final CookieHandler handler = new CookieHandler();
+
+ @BeforeEach
+ void resetHandler() {
+ handler.lastCookieHeader = null;
+ }
+
+ @Override
+ protected Handler[] getHandlers() {
+ return new Handler[] {handler};
+ }
+
+ @Test
+ void responseCarryingSetCookieRecordsTheOrigin() throws Exception {
+ final String url = url("/setcookie");
+ final Metadata md = fetch(protocol(false), url, new Metadata());
+ assertEquals(url, md.getFirstValue(ORIGIN_KEY), "the origin is the url
which was fetched");
+ }
+
+ @Test
+ void responseWithoutSetCookieRecordsNoOrigin() throws Exception {
+ final Metadata md = fetch(protocol(false), url("/nocookie"), new
Metadata());
+ assertNull(md.getFirstValue(ORIGIN_KEY), "no cookies, no origin");
+ }
+
+ @Test
+ void serverSuppliedOriginHeaderIsDiscarded() throws Exception {
+ final Metadata md = fetch(protocol(false), url("/forgedorigin"), new
Metadata());
+ assertNull(md.getFirstValue(ORIGIN_KEY), "the origin can not be set by
the server");
+ }
+
+ @Test
+ void serverSuppliedOriginHeaderIsOverwritten() throws Exception {
+ final String url = url("/setcookieandforgedorigin");
+ final Metadata md = fetch(protocol(false), url, new Metadata());
+ assertEquals(url, md.getFirstValue(ORIGIN_KEY), "the origin can not be
set by the server");
+ }
+
+ @Test
+ void redirectResponseRecordsTheOrigin() throws Exception {
+ final String url = url("/loginredirect");
+ final Metadata md = fetch(protocol(false), url, new Metadata());
+ assertEquals(url, md.getFirstValue(ORIGIN_KEY), "a 302 can set cookies
too");
+ }
+
+ @Test
+ void hostOnlyCookieIsSentBackToTheSameHost() throws Exception {
+ final HttpProtocol protocol = protocol(true);
+ final Metadata md = outlinkMetadata(protocol, url("/setcookie"));
+ fetch(protocol, url("/echo"), md);
+ assertEquals("sid=x", handler.lastCookieHeader, "the cookie is sent
back to the same host");
+ }
+
+ @Test
+ void hostOnlyCookieIsNotSentToAnotherHost() throws Exception {
+ final HttpProtocol protocol = protocol(true);
+ final Metadata md = outlinkMetadata(protocol, url("/setcookie"));
+ fetch(protocol, "http://127.0.0.1:"; + HTTP_PORT + "/echo", md);
+ assertNull(handler.lastCookieHeader, "the cookie is bound to the host
which set it");
+ }
+
+ @Test
+ void cookieWithoutRecordedOriginIsNotSent() throws Exception {
+ final HttpProtocol protocol = protocol(true);
+ final Metadata md = outlinkMetadata(protocol, url("/setcookie"));
+ md.remove("protocol." + ORIGIN_KEY);
+ fetch(protocol, url("/echo"), md);
+ assertNull(handler.lastCookieHeader, "without an origin the cookie can
not be scoped");
+ }
+
+ @Test
+ void unparseableOriginIsIgnored() throws Exception {
+ final HttpProtocol protocol = protocol(true);
+ final Metadata md = outlinkMetadata(protocol,
url("/setcookieforlocalhost"));
+ md.setValue("protocol." + ORIGIN_KEY, "not a url");
+ fetch(protocol, url("/echo"), md);
+ assertEquals(
+ "domainsid=y",
+ handler.lastCookieHeader,
+ "only the cookie without a domain attribute needs the origin");
+ }
+
+ /** Fetches a url and returns the metadata of the response. */
+ private Metadata fetch(HttpProtocol protocol, String url, Metadata md)
throws Exception {
+ return protocol.getProtocolOutput(url, md).getMetadata();
+ }
+
+ /** Fetches a url and returns its response metadata as an outlink would
inherit it. */
+ private Metadata outlinkMetadata(HttpProtocol protocol, String url) throws
Exception {
+ final Metadata md = new Metadata();
+ md.putAll(fetch(protocol, url, new Metadata()), "protocol.");
+ return md;
+ }
+
+ private String url(String path) {
+ return "http://localhost:"; + HTTP_PORT + path;
+ }
+
+ private HttpProtocol protocol(boolean useCookies) {
+ final Config conf = new Config();
+ conf.put("http.agent.name", "test");
+ conf.put("http.agent.version", "1.0");
+ conf.put("http.agent.description", "test");
+ conf.put("http.agent.url", "http://test.example.com";);
+ conf.put("http.agent.email", "[email protected]");
+ conf.put("http.use.cookies", useCookies);
+ conf.put("protocol.md.prefix", "protocol.");
+ final HttpProtocol protocol = new HttpProtocol();
+ protocol.configure(conf);
+ return protocol;
+ }
+
+ static class CookieHandler extends AbstractHandler {
+
+ private volatile String lastCookieHeader;
+
+ @Override
+ public void handle(
+ String target,
+ Request baseRequest,
+ HttpServletRequest request,
+ HttpServletResponse response)
+ throws IOException {
+ baseRequest.setHandled(true);
+ switch (target) {
+ case "/setcookie":
+ response.addHeader("Set-Cookie", "sid=x; Path=/");
+ break;
+ case "/setcookieforlocalhost":
+ response.addHeader("Set-Cookie", "sid=x; Path=/");
+ response.addHeader("Set-Cookie", "domainsid=y;
Domain=localhost; Path=/");
+ break;
+ case "/forgedorigin":
+ response.addHeader("Set-Cookie-Origin",
"http://evil.example.com/";);
+ break;
+ case "/setcookieandforgedorigin":
+ response.addHeader("Set-Cookie", "sid=x; Path=/");
+ response.addHeader("Set-Cookie-Origin",
"http://evil.example.com/";);
+ break;
+ case "/loginredirect":
+ response.addHeader("Set-Cookie", "sid=x; Path=/");
+ response.setHeader("Location", "/home");
+ response.setStatus(HttpServletResponse.SC_FOUND);
+ return;
+ case "/echo":
+ lastCookieHeader = request.getHeader("Cookie");
+ break;
+ default:
+ break;
+ }
+ final byte[] content = "Success!".getBytes(StandardCharsets.UTF_8);
+ response.setStatus(HttpServletResponse.SC_OK);
+ response.setContentType("text/html");
+ response.setContentLength(content.length);
+ try (OutputStream out = response.getOutputStream()) {
+ out.write(content);
+ }
+ }
+ }
+}
diff --git
a/core/src/test/java/org/apache/stormcrawler/util/CookieConverterTest.java
b/core/src/test/java/org/apache/stormcrawler/util/CookieConverterTest.java
index 57b95f45..e5b651f3 100644
--- a/core/src/test/java/org/apache/stormcrawler/util/CookieConverterTest.java
+++ b/core/src/test/java/org/apache/stormcrawler/util/CookieConverterTest.java
@@ -40,7 +40,9 @@ class CookieConverterTest {
String dummyCookieString =
buildCookieString(dummyCookieHeader, dummyCookieValue, null,
null, null, null);
cookiesStrings[0] = dummyCookieString;
- List<Cookie> result = CookieConverter.getCookies(cookiesStrings,
getUrl(unsecuredUrl));
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl));
Assertions.assertEquals(1, result.size(), "Should have 1 cookie");
Assertions.assertEquals(
dummyCookieHeader, result.get(0).getName(), "Cookie header
should be as defined");
@@ -60,7 +62,9 @@ class CookieConverterTest {
null,
null);
cookiesStrings[0] = dummyCookieString;
- List<Cookie> result = CookieConverter.getCookies(cookiesStrings,
getUrl(unsecuredUrl));
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl));
Assertions.assertEquals(1, result.size(), "Should have 1 cookie");
Assertions.assertEquals(
dummyCookieHeader, result.get(0).getName(), "Cookie header
should be as defined");
@@ -80,7 +84,9 @@ class CookieConverterTest {
null,
null);
cookiesStrings[0] = dummyCookieString;
- List<Cookie> result = CookieConverter.getCookies(cookiesStrings,
getUrl(unsecuredUrl));
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl));
Assertions.assertEquals(
0, result.size(), "Should have 0 cookies, since cookie was
expired");
}
@@ -92,7 +98,8 @@ class CookieConverterTest {
buildCookieString(dummyCookieHeader, dummyCookieValue, null,
null, "/", null);
cookiesStrings[0] = dummyCookieString;
List<Cookie> result =
- CookieConverter.getCookies(cookiesStrings, getUrl(unsecuredUrl
+ "/somepage"));
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl + "/somepage"));
Assertions.assertEquals(1, result.size(), "Should have 1 cookie");
Assertions.assertEquals(
dummyCookieHeader, result.get(0).getName(), "Cookie header
should be as defined");
@@ -106,7 +113,9 @@ class CookieConverterTest {
String dummyCookieString =
buildCookieString(dummyCookieHeader, dummyCookieValue, null,
null, "/", null);
cookiesStrings[0] = dummyCookieString;
- List<Cookie> result = CookieConverter.getCookies(cookiesStrings,
getUrl(unsecuredUrl));
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl));
Assertions.assertEquals(1, result.size(), "Should have 1 cookie");
Assertions.assertEquals(
dummyCookieHeader, result.get(0).getName(), "Cookie header
should be as defined");
@@ -122,7 +131,8 @@ class CookieConverterTest {
dummyCookieHeader, dummyCookieValue, null, null,
"/someFolder", null);
cookiesStrings[0] = dummyCookieString;
List<Cookie> result =
- CookieConverter.getCookies(cookiesStrings, getUrl(unsecuredUrl
+ "/someFolder"));
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl + "/someFolder"));
Assertions.assertEquals(1, result.size(), "Should have 1 cookie");
Assertions.assertEquals(
dummyCookieHeader, result.get(0).getName(), "Cookie header
should be as defined");
@@ -139,7 +149,9 @@ class CookieConverterTest {
cookiesStrings[0] = dummyCookieString;
List<Cookie> result =
CookieConverter.getCookies(
- cookiesStrings, getUrl(unsecuredUrl +
"/someFolder/SomeOtherFolder"));
+ cookiesStrings,
+ getUrl(unsecuredUrl),
+ getUrl(unsecuredUrl + "/someFolder/SomeOtherFolder"));
Assertions.assertEquals(1, result.size(), "Should have 1 cookie");
Assertions.assertEquals(
dummyCookieHeader, result.get(0).getName(), "Cookie header
should be as defined");
@@ -156,7 +168,9 @@ class CookieConverterTest {
cookiesStrings[0] = dummyCookieString;
List<Cookie> result =
CookieConverter.getCookies(
- cookiesStrings, getUrl(unsecuredUrl +
"/someOtherFolder/SomeFolder"));
+ cookiesStrings,
+ getUrl(unsecuredUrl),
+ getUrl(unsecuredUrl + "/someOtherFolder/SomeFolder"));
Assertions.assertEquals(0, result.size(), "path mismatch, should have
0 cookies");
}
@@ -169,7 +183,9 @@ class CookieConverterTest {
cookiesStrings[0] = dummyCookieString;
List<Cookie> result =
CookieConverter.getCookies(
- cookiesStrings, getUrl(unsecuredUrl +
"/someFolder/SomeOtherFolder"));
+ cookiesStrings,
+ getUrl(unsecuredUrl),
+ getUrl(unsecuredUrl + "/someFolder/SomeOtherFolder"));
Assertions.assertEquals(1, result.size(), "Should have 1 cookie");
Assertions.assertEquals(
dummyCookieHeader, result.get(0).getName(), "Cookie header
should be as defined");
@@ -186,7 +202,9 @@ class CookieConverterTest {
cookiesStrings[0] = dummyCookieString;
List<Cookie> result =
CookieConverter.getCookies(
- cookiesStrings, getUrl(unsecuredUrl +
"/someFolder/SomeOtherFolder"));
+ cookiesStrings,
+ getUrl(unsecuredUrl),
+ getUrl(unsecuredUrl + "/someFolder/SomeOtherFolder"));
Assertions.assertEquals(0, result.size(), "Domain is not valid -
Should have 0 cookies");
}
@@ -199,7 +217,9 @@ class CookieConverterTest {
cookiesStrings[0] = dummyCookieString;
List<Cookie> result =
CookieConverter.getCookies(
- cookiesStrings, getUrl(unsecuredUrl +
"/someFolder/SomeOtherFolder"));
+ cookiesStrings,
+ getUrl(unsecuredUrl),
+ getUrl(unsecuredUrl + "/someFolder/SomeOtherFolder"));
Assertions.assertEquals(
0, result.size(), "Target url is not secured - Should have 0
cookies");
}
@@ -213,7 +233,9 @@ class CookieConverterTest {
cookiesStrings[0] = dummyCookieString;
List<Cookie> result =
CookieConverter.getCookies(
- cookiesStrings, getUrl(securedUrl +
"/someFolder/SomeOtherFolder"));
+ cookiesStrings,
+ getUrl(unsecuredUrl),
+ getUrl(securedUrl + "/someFolder/SomeOtherFolder"));
Assertions.assertEquals(1, result.size(), "Target url is secured -
Should have 1 cookie");
Assertions.assertEquals(
dummyCookieHeader, result.get(0).getName(), "Cookie header
should be as defined");
@@ -235,7 +257,9 @@ class CookieConverterTest {
cookiesStrings[0] = dummyCookieString;
List<Cookie> result =
CookieConverter.getCookies(
- cookiesStrings, getUrl(securedUrl +
"/someFolder/SomeOtherFolder"));
+ cookiesStrings,
+ getUrl(unsecuredUrl),
+ getUrl(securedUrl + "/someFolder/SomeOtherFolder"));
Assertions.assertEquals(1, result.size(), "Should have 1 cookie");
Assertions.assertEquals(
dummyCookieHeader, result.get(0).getName(), "Cookie header
should be as defined");
@@ -266,7 +290,9 @@ class CookieConverterTest {
cookiesStrings[1] = dummyCookieString2;
List<Cookie> result =
CookieConverter.getCookies(
- cookiesStrings, getUrl(securedUrl +
"/someFolder/SomeOtherFolder"));
+ cookiesStrings,
+ getUrl(unsecuredUrl),
+ getUrl(securedUrl + "/someFolder/SomeOtherFolder"));
Assertions.assertEquals(2, result.size(), "Should have 2 cookies");
Assertions.assertEquals(
dummyCookieHeader, result.get(0).getName(), "Cookie header
should be as defined");
@@ -282,6 +308,457 @@ class CookieConverterTest {
"Cookie value should be as defined");
}
+ @Test
+ void cookieWithoutDomainIsSentToSameHost() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue, null,
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl + "/somepage"));
+ Assertions.assertEquals(1, result.size(), "Should have 1 cookie");
+ }
+
+ @Test
+ void cookieWithoutDomainIsNotSentToOtherHost() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue, null,
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl("http://someotherurl.com";));
+ Assertions.assertEquals(
+ 0, result.size(), "Cookie without domain is bound to the host
which set it");
+ }
+
+ @Test
+ void cookieWithoutDomainIsNotSentWhenOriginIsUnknown() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue, null,
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(cookiesStrings, null,
getUrl(unsecuredUrl));
+ Assertions.assertEquals(
+ 0, result.size(), "Cookie without domain needs the host which
set it");
+ }
+
+ @Test
+ void cookieWithSingleLabelDomainIsNotSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue, "com",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl));
+ Assertions.assertEquals(0, result.size(), "Domain com must not match
someurl.com");
+ }
+
+ @Test
+ void cookieWithDomainIsNotSentWhenOriginIsOutsideTheDomain() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "someurl.com",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://someotherurl.com";),
+ getUrl(unsecuredUrl + "/somepage"));
+ Assertions.assertEquals(
+ 0, result.size(), "Domain does not cover the host which set
the cookie");
+ }
+
+ @Test
+ void cookieWithConsecutiveDotsInDomainIsNotSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue,
"com..", null, null, null);
+ Assertions.assertEquals(
+ 0,
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://evil.com";),
+ getUrl("http://victim.com";))
+ .size(),
+ "Domain com.. must not match every host under com");
+ Assertions.assertEquals(
+ 0,
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl))
+ .size(),
+ "Domain com.. must not match someurl.com");
+ }
+
+ @Test
+ void cookieWithTrailingDotsInDomainIsNotSent() {
+ for (String domain : new String[] {"com...", ".com.."}) {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, domain, null,
null, null);
+ Assertions.assertEquals(
+ 0,
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl))
+ .size(),
+ "Domain " + domain + " is malformed");
+ }
+ }
+
+ @Test
+ void cookieWithEmptyLabelInDomainIsNotSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "some..url.com",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl));
+ Assertions.assertEquals(0, result.size(), "Domain with an empty label
is malformed");
+ }
+
+ @Test
+ void cookieWithLeadingDoubleDotDomainIsNotSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "..someurl.com",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl));
+ Assertions.assertEquals(0, result.size(), "Only one leading dot is
allowed");
+ }
+
+ @Test
+ void cookieWithTrailingRootDotDomainIsSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "someurl.com.",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl));
+ Assertions.assertEquals(1, result.size(), "The root label is not part
of the domain");
+ }
+
+ @Test
+ void cookieWithInvalidCharactersInDomainIsNotSent() {
+ for (String domain :
+ new String[] {"some_url.com", "-someurl.com", "someurl-.com",
"\"someurl.com\""}) {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, domain, null,
null, null);
+ Assertions.assertEquals(
+ 0,
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl))
+ .size(),
+ "Domain " + domain + " is not a valid domain name");
+ }
+ }
+
+ @Test
+ void cookieWithOverlongLabelInDomainIsNotSent() {
+ String label = "a".repeat(64);
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, label + ".com",
null, null, null);
+ Assertions.assertEquals(
+ 0,
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://"; + label + ".com"),
+ getUrl("http://"; + label + ".com"))
+ .size(),
+ "A label of more than 63 characters is not valid");
+
+ String validLabel = "a".repeat(63);
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, validLabel +
".com", null, null, null);
+ Assertions.assertEquals(
+ 1,
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://"; + validLabel + ".com"),
+ getUrl("http://"; + validLabel + ".com"))
+ .size(),
+ "A label of 63 characters is valid");
+ }
+
+ @Test
+ void cookieWithEmptyDomainIsTreatedAsHostOnly() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue, "",
null, null, null);
+ Assertions.assertEquals(
+ 1,
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl))
+ .size(),
+ "An empty domain attribute binds the cookie to the host which
set it");
+ Assertions.assertEquals(
+ 0,
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl(unsecuredUrl),
+ getUrl("http://someotherurl.com";))
+ .size(),
+ "An empty domain attribute does not cover another host");
+ }
+
+ @Test
+ void cookieWithPublicSuffixDomainIsNotSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue,
"co.uk", null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl("http://evil.co.uk";),
getUrl("http://bank.co.uk";));
+ Assertions.assertEquals(0, result.size(), "A public suffix is not
usable as a scope");
+ }
+
+ @Test
+ void cookieWithUpperCasePublicSuffixDomainIsNotSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue,
"CO.UK", null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl("http://evil.co.uk";),
getUrl("http://bank.co.uk";));
+ Assertions.assertEquals(
+ 0, result.size(), "The public suffix lookup must not depend on
the case");
+ }
+
+ @Test
+ void cookieWithMultiLabelPublicSuffixDomainIsNotSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "pvt.k12.ma.us",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://evil.pvt.k12.ma.us";),
+ getUrl("http://victim.pvt.k12.ma.us";));
+ Assertions.assertEquals(
+ 0, result.size(), "A multi label public suffix is not usable
as a scope");
+ }
+
+ @Test
+ void cookieWithPrivateSectionSuffixDomainIsNotSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "github.io",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://evil.github.io";),
+ getUrl("http://victim.github.io";));
+ Assertions.assertEquals(
+ 0, result.size(), "A suffix of the private section is not
usable as a scope");
+ }
+
+ @Test
+ void cookieWithWildcardPublicSuffixDomainIsNotSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue,
"foo.ck", null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl("http://a.foo.ck";),
getUrl("http://b.foo.ck";));
+ Assertions.assertEquals(
+ 0, result.size(), "A suffix matched by a wildcard rule is not
usable as a scope");
+ }
+
+ @Test
+ void cookieBelowPublicSuffixIsSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "example.co.uk",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://www.example.co.uk";),
+ getUrl("http://shop.example.co.uk";));
+ Assertions.assertEquals(
+ 1, result.size(), "A domain below a public suffix covers its
hosts");
+ }
+
+ @Test
+ void cookieBelowPrivateSectionSuffixIsSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "user.github.io",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://www.user.github.io";),
+ getUrl("http://blog.user.github.io";));
+ Assertions.assertEquals(
+ 1, result.size(), "A domain below a private suffix covers its
hosts");
+ }
+
+ @Test
+ void cookieWithUnlistedInternalDomainIsSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "intranet.local",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://host.intranet.local";),
+ getUrl("http://other.intranet.local";));
+ Assertions.assertEquals(
+ 1, result.size(), "A domain unknown to the public suffix list
keeps working");
+ }
+
+ @Test
+ void cookieWithUnicodeDomainIsSent() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "münchen.de",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://münchen.de";),
+ getUrl("http://www.münchen.de";));
+ Assertions.assertEquals(
+ 1, result.size(), "A domain attribute written in unicode keeps
scoping its cookie");
+ }
+
+ @Test
+ void cookieWithUnicodeDomainIsSentToThePunycodeHost() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "münchen.de",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://xn--mnchen-3ya.de";),
+ getUrl("http://www.xn--mnchen-3ya.de";));
+ Assertions.assertEquals(
+ 1, result.size(), "The unicode and punycode forms are
interchangeable");
+ }
+
+ @Test
+ void cookieWithAddressDomainIsNotSentToAnotherAddress() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue,
"2.3.4", null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl("http://1.2.3.4";),
getUrl("http://9.2.3.4";));
+ Assertions.assertEquals(0, result.size(), "An address does not cover
anything below it");
+ }
+
+ @Test
+ void cookieWithAddressDomainIsSentToThatAddress() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(dummyCookieHeader, dummyCookieValue,
"1.2.3.4", null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl("http://1.2.3.4";),
getUrl("http://1.2.3.4";));
+ Assertions.assertEquals(1, result.size(), "An address is matched by
itself");
+ }
+
+ @Test
+ void cookieHeaderWithoutANameIsSkipped() {
+ String[] cookiesStrings = new String[] {"novalue", "=novalue",
"sid=ok"};
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings, getUrl(unsecuredUrl),
getUrl(unsecuredUrl));
+ Assertions.assertEquals(1, result.size(), "Only the usable cookie is
kept");
+ Assertions.assertEquals("sid", result.get(0).getName());
+ }
+
+ @Test
+ void cookieWithUnlistedInternalDomainIsSent2() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "host.internal",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://a.host.internal";),
+ getUrl("http://b.host.internal";));
+ Assertions.assertEquals(
+ 1, result.size(), "A domain unknown to the public suffix list
keeps working");
+ }
+
+ @Test
+ void cookieWithUnlistedDomainIsNotSentToLookalikeHost() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "intranet.local",
null, null, null);
+ List<Cookie> result =
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://host.intranet.local";),
+ getUrl("http://intranet.local.evil.com";));
+ Assertions.assertEquals(
+ 0, result.size(), "The domain must be a suffix of the host,
not a prefix");
+ }
+
+ @Test
+ void cookieWithSingleLabelIntranetDomainIsSentToThatHostOnly() {
+ String[] cookiesStrings = new String[1];
+ cookiesStrings[0] =
+ buildCookieString(
+ dummyCookieHeader, dummyCookieValue, "intranet", null,
null, null);
+ Assertions.assertEquals(
+ 1,
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://intranet";),
+ getUrl("http://intranet/somepage";))
+ .size(),
+ "A single label domain binds the cookie to that host");
+ Assertions.assertEquals(
+ 0,
+ CookieConverter.getCookies(
+ cookiesStrings,
+ getUrl("http://intranet";),
+ getUrl("http://other.intranet";))
+ .size(),
+ "A single label domain does not cover its subdomains");
+ }
+
+ @Test
+ void domainsCheckerRejectsEmptyLabels() {
+ Assertions.assertFalse(
+ CookieConverter.checkDomainMatchToUrl("com..", "evil.com"),
+ "an empty label must not be dropped");
+ Assertions.assertFalse(
+ CookieConverter.checkDomainMatchToUrl("..example.com",
"www.example.com"),
+ "an empty label must not be dropped");
+ }
+
+ @Test
+ void domainsCheckerRejectsNullDomain() {
+ Assertions.assertFalse(
+ CookieConverter.checkDomainMatchToUrl(null, "example.com"),
+ "domain can not be checked");
+ }
+
+ @Test
+ void domainsCheckerIgnoresTheRootLabelOfTheHost() {
+ Assertions.assertTrue(
+ CookieConverter.checkDomainMatchToUrl("example.com",
"www.example.com."),
+ "the root label is not part of the host name");
+ }
+
@Test
void testDomainsChecker() {
boolean result = CookieConverter.checkDomainMatchToUrl(".example.com",
"www.example.com");
@@ -306,6 +783,12 @@ class CookieConverterTest {
Assertions.assertFalse(result, "domain is not valid");
}
+ @Test
+ void testDomainsChecker5() {
+ boolean result = CookieConverter.checkDomainMatchToUrl("example.com",
null);
+ Assertions.assertFalse(result, "domain can not be checked");
+ }
+
private URL getUrl(String urlString) {
try {
return URLUtil.toURL(urlString);
diff --git
a/core/src/test/java/org/apache/stormcrawler/util/MetadataTransferTest.java
b/core/src/test/java/org/apache/stormcrawler/util/MetadataTransferTest.java
index b2b297ad..43368e97 100644
--- a/core/src/test/java/org/apache/stormcrawler/util/MetadataTransferTest.java
+++ b/core/src/test/java/org/apache/stormcrawler/util/MetadataTransferTest.java
@@ -54,6 +54,44 @@ class MetadataTransferTest {
Assertions.assertEquals(1, urlpath.length);
}
+ @Test
+ void testCookieTransfer() throws MalformedURLException {
+ Map<String, Object> conf = new HashMap<>();
+ conf.put(
+ MetadataTransfer.metadataTransferParamName,
+ List.of("protocol.set-cookie", "protocol.set-cookie-origin"));
+ Metadata parentMD = new Metadata();
+ parentMD.addValue("protocol.set-cookie", "sid=42; Path=/");
+ parentMD.addValue("protocol.set-cookie-origin",
"http://www.example.com";);
+ Metadata outlinkMD =
+ MetadataTransfer.getInstance(conf)
+ .getMetaForOutlink(
+ "http://www.example.com/outlink.html";,
+ "http://www.example.com";,
+ parentMD);
+ Assertions.assertEquals("sid=42; Path=/",
outlinkMD.getFirstValue("protocol.set-cookie"));
+ Assertions.assertEquals(
+ "http://www.example.com";,
outlinkMD.getFirstValue("protocol.set-cookie-origin"));
+ }
+
+ @Test
+ void testCookieTransferWithWildcard() throws MalformedURLException {
+ Map<String, Object> conf = new HashMap<>();
+ conf.put(MetadataTransfer.metadataTransferParamName,
List.of("protocol.set-cookie*"));
+ Metadata parentMD = new Metadata();
+ parentMD.addValue("protocol.set-cookie", "sid=42; Path=/");
+ parentMD.addValue("protocol.set-cookie-origin",
"http://www.example.com";);
+ Metadata outlinkMD =
+ MetadataTransfer.getInstance(conf)
+ .getMetaForOutlink(
+ "http://www.example.com/outlink.html";,
+ "http://www.example.com";,
+ parentMD);
+ Assertions.assertEquals("sid=42; Path=/",
outlinkMD.getFirstValue("protocol.set-cookie"));
+ Assertions.assertEquals(
+ "http://www.example.com";,
outlinkMD.getFirstValue("protocol.set-cookie-origin"));
+ }
+
@Test
void testCustomTransferClass() throws MalformedURLException {
Map<String, Object> conf = new HashMap<>();
diff --git a/docs/src/main/asciidoc/configuration.adoc
b/docs/src/main/asciidoc/configuration.adoc
index 7aaa0ccf..9f272d8e 100644
--- a/docs/src/main/asciidoc/configuration.adoc
+++ b/docs/src/main/asciidoc/configuration.adoc
@@ -224,7 +224,7 @@ implementation.
| robots.noFollow.strict | true | If true, remove all outlinks from pages
marked as noFollow.
| http.store.headers | false | Whether to store response headers.
| http.timeout | 10000 | Connection timeout (ms).
-| http.use.cookies | false | Use cookies in subsequent requests.
+| http.use.cookies | false | Use cookies in subsequent requests. Requires
`protocol.set-cookie` and `protocol.set-cookie-origin` in `metadata.transfer`.
| https.protocol.implementation |
org.apache.stormcrawler.protocol.okhttp.HttpProtocol | HTTPS Protocol
implementation.
| partition.url.mode | byHost | Defines how URLs are partitioned: byHost,
byDomain, or byIP.
diff --git a/docs/src/main/asciidoc/internals.adoc
b/docs/src/main/asciidoc/internals.adoc
index ff74e9a4..7c11e40e 100644
--- a/docs/src/main/asciidoc/internals.adoc
+++ b/docs/src/main/asciidoc/internals.adoc
@@ -475,7 +475,9 @@ The `metadata` argument to
link:https://github.com/apache/stormcrawler/blob/main
* `http.accept.language`: If this key is present in `metadata`, the protocol
will use the value to override the value for the `Accept-Language` header field
of the HTTP request. If the key is not present, the `http.accept.language`
global configuration value is used instead. (Available in v1.11+)
-* `protocol.set-cookie`: If this key is present in `metadata` and
`http.use.cookies` is true, the protocol will send cookies stored from the
response this page was linked to, given the cookie is applicable to the domain
of the link.
+* `protocol.set-cookie`: If this key is present in `metadata` and
`http.use.cookies` is true, the protocol sends the cookies stored from the
response this page was linked to, given the cookie applies to the url being
fetched. A cookie without a `Domain` attribute is only sent when the url has
the same host as the response which set it, which requires
`protocol.set-cookie-origin` to be present too.
+
+* `protocol.set-cookie-origin`: Written by the protocol next to
`protocol.set-cookie` when a response carries a `Set-Cookie` header, and holds
the url of that response. It must be transferred and persisted together with
`protocol.set-cookie`, otherwise cookies without a `Domain` attribute are not
sent. A value sent by a server as a header of that name is discarded.
* `http.method.head`: If this key is present in `metadata`, the protocol sends
a HEAD request. (see link:https://github.com/apache/stormcrawler/pull/923[#923])
@@ -510,13 +512,17 @@ metadata.persist:
- last-modified
- protocol.etag
- protocol.set-cookie
+ - protocol.set-cookie-origin
- ...
----
-* Cookies need to be transferred to outlinks by setting:
+* Cookies and the host which set them need to be transferred to outlinks by
setting:
[source,yaml]
----
metadata.transfer:
- - set-cookie
+ - protocol.set-cookie
+ - protocol.set-cookie-origin
----
+
+The wildcard `protocol.set-cookie*` covers both keys.
diff --git
a/external/playwright/src/main/java/org/apache/stormcrawler/protocol/playwright/HttpProtocol.java
b/external/playwright/src/main/java/org/apache/stormcrawler/protocol/playwright/HttpProtocol.java
index 3a9561d0..3e947e91 100644
---
a/external/playwright/src/main/java/org/apache/stormcrawler/protocol/playwright/HttpProtocol.java
+++
b/external/playwright/src/main/java/org/apache/stormcrawler/protocol/playwright/HttpProtocol.java
@@ -227,6 +227,7 @@ public class HttpProtocol extends AbstractHttpProtocol {
(k, v) -> {
responseMetaData.addValue(k, v);
});
+ recordCookieOrigin(responseMetaData,
url);
storeVerbatimHeaders(response,
responseMetaData);
}
}
@@ -271,6 +272,7 @@ public class HttpProtocol extends AbstractHttpProtocol {
(k, v) -> {
responseMetaData.addValue(k, v);
});
+ recordCookieOrigin(responseMetaData, url);
storeVerbatimHeaders(response, responseMetaData);
int httpStatus = response.status();
@@ -330,6 +332,22 @@ public class HttpProtocol extends AbstractHttpProtocol {
}
}
+ /**
+ * Records the url of the response which carried the cookies, as the
Set-Cookie header does not
+ * say which host sent it and the cookies are scoped to that host when
they are sent back. The
+ * key is always removed first, so that a response sending a header of
that name can not choose
+ * the host its cookies are sent to.
+ *
+ * @param responseMetaData the metadata built from the response headers
+ * @param url the url of the response
+ */
+ private static void recordCookieOrigin(final Metadata responseMetaData,
final String url) {
+ responseMetaData.remove(RESPONSE_COOKIES_ORIGIN);
+ if (responseMetaData.getFirstValue(RESPONSE_COOKIES_HEADER) != null) {
+ responseMetaData.setValue(RESPONSE_COOKIES_ORIGIN, url);
+ }
+ }
+
/**
* Reconstructs verbatim request and response header blocks and stores
them in the metadata
* under the same keys as the other protocol implementations, so that
downstream consumers (e.g.
