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.