From 8f00119f92638e641194775cdc59c38d00fad377 Mon Sep 17 00:00:00 2001 From: Richard Zowalla <13417392+rzo1@users.noreply.github.com> Date: Thu, 27 Aug 2026 14:20:10 +0200 Subject: [PATCH] Scope cookies to the host that set them 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 + .../protocol/okhttp/HttpProtocol.java | 54 +- .../stormcrawler/util/CookieConverter.java | 278 +++++++++- core/src/main/resources/crawler-default.yaml | 4 +- .../okhttp/HttpProtocolCookieTest.java | 204 +++++++ .../util/CookieConverterTest.java | 511 +++++++++++++++++- .../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(-) create mode 100644 core/src/test/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocolCookieTest.java 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 f847d567d..a2f37b70b 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 ba60d7f99..90cb4d742 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.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.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 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 @@ private void addCookiesToRequest(Builder rb, String url, Metadata md) { } try { final List 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 @@ private void addCookiesToRequest(Builder rb, String url, Metadata md) { } } + /** + * 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 ProtocolResponse getProtocolOutput(String url, final Metadata metadata) 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 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 5592fd508..e8501b816 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 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. + * + *

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. + * + *

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 getCookies(String[] cookiesStrings, URL originURL, URL targetURL) { ArrayList list = new ArrayList<>(); for (String cs : cookiesStrings) { @@ -55,6 +91,11 @@ public static List getCookies(String[] cookiesStrings, URL targetURL) { 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 static List getCookies(String[] cookiesStrings, URL targetURL) { 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 static List getCookies(String[] cookiesStrings, URL targetURL) { 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 static List getCookies(String[] cookiesStrings, URL targetURL) { 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 static List getCookies(String[] cookiesStrings, URL targetURL) { // 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 static List getCookies(String[] cookiesStrings, URL targetURL) { } /** - * 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 0343f644f..5293b8a39 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 000000000..a30d52480 --- /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", "test@example.com"); + 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 57b95f458..e5b651f33 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 @@ void testSimpleCookieAndUrl() { String dummyCookieString = buildCookieString(dummyCookieHeader, dummyCookieValue, null, null, null, null); cookiesStrings[0] = dummyCookieString; - List result = CookieConverter.getCookies(cookiesStrings, getUrl(unsecuredUrl)); + List 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 @@ void testNotExpiredCookie() { null, null); cookiesStrings[0] = dummyCookieString; - List result = CookieConverter.getCookies(cookiesStrings, getUrl(unsecuredUrl)); + List 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 @@ void testExpiredCookie() { null, null); cookiesStrings[0] = dummyCookieString; - List result = CookieConverter.getCookies(cookiesStrings, getUrl(unsecuredUrl)); + List 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 @@ void testValidPath() { buildCookieString(dummyCookieHeader, dummyCookieValue, null, null, "/", null); cookiesStrings[0] = dummyCookieString; List 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 @@ void testValidPath2() { String dummyCookieString = buildCookieString(dummyCookieHeader, dummyCookieValue, null, null, "/", null); cookiesStrings[0] = dummyCookieString; - List result = CookieConverter.getCookies(cookiesStrings, getUrl(unsecuredUrl)); + List 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 @@ void testValidPath3() { dummyCookieHeader, dummyCookieValue, null, null, "/someFolder", null); cookiesStrings[0] = dummyCookieString; List 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 @@ void testValidPath4() { cookiesStrings[0] = dummyCookieString; List 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 @@ void testInvalidPath() { cookiesStrings[0] = dummyCookieString; List 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 @@ void testValidDomain() { cookiesStrings[0] = dummyCookieString; List 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 @@ void testInvalidDomain() { cookiesStrings[0] = dummyCookieString; List 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 @@ void testSecurFlagHttp() { cookiesStrings[0] = dummyCookieString; List 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 @@ void testSecurFlagHttpS() { cookiesStrings[0] = dummyCookieString; List 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 @@ void testFullCookie() { cookiesStrings[0] = dummyCookieString; List 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 @@ void test2Cookies() { cookiesStrings[1] = dummyCookieString2; List 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 @@ void test2Cookies() { "Cookie value should be as defined"); } + @Test + void cookieWithoutDomainIsSentToSameHost() { + String[] cookiesStrings = new String[1]; + cookiesStrings[0] = + buildCookieString(dummyCookieHeader, dummyCookieValue, null, null, null, null); + List 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 @@ void testDomainsChecker4() { 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 b2b297ad1..43368e97a 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 @@ void testTransfer() throws MalformedURLException { Assertions.assertEquals(1, urlpath.length); } + @Test + void testCookieTransfer() throws MalformedURLException { + Map 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 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 conf = new HashMap<>(); diff --git a/docs/src/main/asciidoc/configuration.adoc b/docs/src/main/asciidoc/configuration.adoc index 7aaa0ccf7..9f272d8ef 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 ff74e9a4d..7c11e40e9 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 3a9561d0b..3e947e917 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 ProtocolResponse getProtocolOutput(String url, Metadata md) throws Except (k, v) -> { responseMetaData.addValue(k, v); }); + recordCookieOrigin(responseMetaData, url); storeVerbatimHeaders(response, responseMetaData); } } @@ -271,6 +272,7 @@ public ProtocolResponse getProtocolOutput(String url, Metadata md) throws Except (k, v) -> { responseMetaData.addValue(k, v); }); + recordCookieOrigin(responseMetaData, url); storeVerbatimHeaders(response, responseMetaData); int httpStatus = response.status(); @@ -330,6 +332,22 @@ public ProtocolResponse getProtocolOutput(String url, Metadata md) throws Except } } + /** + * 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.