#2211 Migrate core to HttpClient 5 and preserve parsing behavior

THIRD-PARTY.txt

@@ -31,6 +31,7 @@ List of third-party dependencies grouped by their license type.
         * Apache Commons IO (commons-io:commons-io:2.22.0 - https://commons.apache.org/proper/commons-io/)
         * Apache Commons Lang (org.apache.commons:commons-lang3:3.21.0 - https://commons.apache.org/proper/commons-lang/)
         * Apache Commons Logging (commons-logging:commons-logging:1.2 - http://commons.apache.org/proper/commons-logging/)
+        * Apache Commons Logging (commons-logging:commons-logging:1.3.0 - https://commons.apache.org/proper/commons-logging/)
         * Apache Commons Logging (commons-logging:commons-logging:1.3.6 - https://commons.apache.org/proper/commons-logging/)
         * Apache Commons Logging (commons-logging:commons-logging:1.4.0 - https://commons.apache.org/proper/commons-logging/)
         * Apache Commons Math (org.apache.commons:commons-math3:3.6.1 - http://commons.apache.org/proper/commons-math/)

core/pom.xml

@@ -41,7 +41,6 @@ under the License.
         <jsoup.version>1.23.2</jsoup.version>
         <icu4j.version>78.3</icu4j.version>
         <xerces.version>2.12.2</xerces.version>
-        <httpclient.version>4.5.14</httpclient.version>
         <snakeyaml.version>2.7</snakeyaml.version>
         <commons.lang.version>3.21.0</commons.lang.version>
         <wiremock.version>3.13.2</wiremock.version>
@@ -215,9 +214,9 @@ under the License.
         </dependency>
 
         <dependency>
-            <groupId>org.apache.httpcomponents</groupId>
-            <artifactId>httpclient</artifactId>
-            <version>${httpclient.version}</version>
+            <groupId>org.apache.httpcomponents.core5</groupId>
+            <artifactId>httpcore5</artifactId>
+            <version>${httpcore5.version}</version>
         </dependency>
 
         <dependency>

core/src/main/java/org/apache/stormcrawler/bolt/FeedParserBolt.java

@@ -36,7 +36,7 @@
 import java.util.Map;
 import java.util.TimeZone;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.storm.task.OutputCollector;
 import org.apache.storm.task.TopologyContext;
 import org.apache.storm.topology.OutputFieldsDeclarer;

core/src/main/java/org/apache/stormcrawler/bolt/JSoupParserBolt.java

@@ -32,7 +32,7 @@
 import java.util.Map;
 import java.util.stream.Stream;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.storm.task.OutputCollector;
 import org.apache.storm.task.TopologyContext;
 import org.apache.storm.topology.OutputFieldsDeclarer;

core/src/main/java/org/apache/stormcrawler/bolt/SimpleFetcherBolt.java

@@ -31,7 +31,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.storm.Config;
 import org.apache.storm.task.OutputCollector;
 import org.apache.storm.task.TopologyContext;

core/src/main/java/org/apache/stormcrawler/bolt/SiteMapParserBolt.java

@@ -44,7 +44,7 @@
 import java.util.TimeZone;
 import java.util.function.Consumer;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.storm.task.OutputCollector;
 import org.apache.storm.task.TopologyContext;
 import org.apache.storm.topology.OutputFieldsDeclarer;

core/src/main/java/org/apache/stormcrawler/fetcher/FetchOutcomes.java

@@ -20,7 +20,7 @@
 import java.io.InterruptedIOException;
 import java.net.UnknownHostException;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.stormcrawler.Constants;
 import org.apache.stormcrawler.Metadata;
 import org.apache.stormcrawler.bolt.FetcherBolt;

core/src/main/java/org/apache/stormcrawler/filtering/basic/BasicURLNormalizer.java

@@ -37,8 +37,8 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.http.NameValuePair;
-import org.apache.http.client.utils.URLEncodedUtils;
+import org.apache.hc.core5.http.NameValuePair;
+import org.apache.hc.core5.net.URLEncodedUtils;
 import org.apache.stormcrawler.Metadata;
 import org.apache.stormcrawler.filtering.URLFilter;
 import org.apache.stormcrawler.util.URLUtil;
@@ -289,7 +289,10 @@ private String processQueryElements(String urlToFilter) {
                 return urlToFilter;
             }
 
-            List<NameValuePair> pairs = URLEncodedUtils.parse(query, StandardCharsets.UTF_8);
+            // Keep parse and format: WWWFormCodec changes semicolon splitting and tilde/star
+            // encoding.
+            List<NameValuePair> pairs =
+                    URLEncodedUtils.parse(query, StandardCharsets.UTF_8, '&', ';');
             Iterator<NameValuePair> pairsIterator = pairs.iterator();
             while (pairsIterator.hasNext()) {
                 NameValuePair param = pairsIterator.next();

core/src/main/java/org/apache/stormcrawler/persistence/AdaptiveScheduler.java

@@ -24,7 +24,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.TimeZone;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.stormcrawler.Constants;
 import org.apache.stormcrawler.Metadata;
 import org.apache.stormcrawler.parse.filter.MD5SignatureParseFilter;

core/src/main/java/org/apache/stormcrawler/protocol/HttpRobotRulesParser.java

@@ -28,7 +28,7 @@
 import java.util.Locale;
 import java.util.Set;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.storm.Config;
 import org.apache.stormcrawler.Metadata;
 import org.apache.stormcrawler.util.ConfUtils;

core/src/main/java/org/apache/stormcrawler/protocol/file/FileResponse.java

@@ -26,8 +26,8 @@
 import java.nio.charset.StandardCharsets;
 import java.util.Locale;
 import org.apache.commons.io.IOUtils;
-import org.apache.http.HttpHeaders;
-import org.apache.http.HttpStatus;
+import org.apache.hc.core5.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpStatus;
 import org.apache.stormcrawler.Metadata;
 import org.apache.stormcrawler.protocol.ProtocolResponse;
 import org.apache.stormcrawler.util.URLUtil;

core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java

@@ -71,8 +71,7 @@
 import okio.BufferedSource;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.mutable.MutableObject;
-import org.apache.http.HttpHeaders;
-import org.apache.http.cookie.Cookie;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.storm.Config;
 import org.apache.stormcrawler.Constants;
 import org.apache.stormcrawler.Metadata;
@@ -86,6 +85,7 @@
 import org.apache.stormcrawler.proxy.SCProxy;
 import org.apache.stormcrawler.util.ConfUtils;
 import org.apache.stormcrawler.util.CookieConverter;
+import org.apache.stormcrawler.util.CookieConverter.Cookie;
 import org.apache.stormcrawler.util.URLUtil;
 import org.jetbrains.annotations.NotNull;
 import org.slf4j.LoggerFactory;
@@ -506,7 +506,7 @@ private void addCookiesToRequest(Builder rb, String url, Metadata md, boolean se
                     CookieConverter.getCookies(
                             cookieStrings, getCookieOrigin(md, url), URLUtil.toURL(url));
             for (Cookie c : cookies) {
-                rb.addHeader("Cookie", c.getName() + "=" + c.getValue());
+                rb.addHeader("Cookie", c.name() + "=" + c.value());
             }
         } catch (MalformedURLException e) { // Bad url , nothing to do
         }

core/src/main/java/org/apache/stormcrawler/util/CharsetIdentification.java

@@ -30,7 +30,7 @@
 import java.util.regex.Pattern;
 import org.apache.commons.io.ByteOrderMark;
 import org.apache.commons.io.input.BOMInputStream;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.stormcrawler.Metadata;
 import org.jsoup.nodes.Document;
 import org.jsoup.nodes.Element;

core/src/main/java/org/apache/stormcrawler/util/CookieConverter.java

@@ -20,22 +20,52 @@
 import crawlercommons.domains.EffectiveTldFinder;
 import java.net.IDN;
 import java.net.URL;
+import java.text.ParsePosition;
+import java.text.SimpleDateFormat;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 import java.util.Locale;
-import org.apache.http.cookie.Cookie;
-import org.apache.http.impl.cookie.BasicClientCookie;
+import java.util.TimeZone;
 
 /** Helper to extract cookies from cookies string. */
 public class CookieConverter {
 
+    /** An immutable cookie accepted for a target URL. */
+    public record Cookie(
+            String name,
+            String value,
+            String domain,
+            String path,
+            boolean secure,
+            Instant expiry) {}
+
     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;
 
+    private static final String[] COOKIE_DATE_PATTERNS = {
+        "EEE, dd MMM yyyy HH:mm:ss zzz", "EEE, dd-MMM-yy HH:mm:ss zzz", "EEE MMM d HH:mm:ss yyyy"
+    };
+
+    private static final Date COOKIE_DATE_PIVOT = Date.from(Instant.parse("2000-01-01T00:00:00Z"));
+
+    private static final ThreadLocal<SimpleDateFormat[]> COOKIE_DATE_FORMATS =
+            ThreadLocal.withInitial(
+                    () -> {
+                        SimpleDateFormat[] formats =
+                                new SimpleDateFormat[COOKIE_DATE_PATTERNS.length];
+                        for (int i = 0; i < COOKIE_DATE_PATTERNS.length; i++) {
+                            formats[i] = new SimpleDateFormat(COOKIE_DATE_PATTERNS[i], Locale.US);
+                            formats[i].setTimeZone(TimeZone.getTimeZone("GMT"));
+                            formats[i].set2DigitYearStart(COOKIE_DATE_PIVOT);
+                        }
+                        return formats;
+                    });
+
     /**
      * Get a list of cookies based on the cookies string taken from response header and the target
      * url. As the host which set the cookies is unknown, cookies without a domain attribute are
@@ -115,8 +145,6 @@ public static List<Cookie> getCookies(String[] cookiesStrings, URL originURL, UR
                 }
             }
 
-            BasicClientCookie cookie = new BasicClientCookie(name, value);
-
             // check domain
             if (domain != null && !domain.isBlank()) {
                 final String scope = normaliseDomain(domain);
@@ -125,8 +153,6 @@ public static List<Cookie> getCookies(String[] cookiesStrings, URL originURL, UR
                     continue;
                 }
 
-                cookie.setDomain(domain);
-
                 // 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);
@@ -167,8 +193,6 @@ public static List<Cookie> getCookies(String[] cookiesStrings, URL originURL, UR
 
             // check path
             if (path != null) {
-                cookie.setPath(path);
-
                 if (!path.equals("")
                         && !path.equals("/")
                         && !targetURL.getPath().startsWith(path)) {
@@ -183,23 +207,20 @@ public static List<Cookie> getCookies(String[] cookiesStrings, URL originURL, UR
 
             // check secure
             if (secure) {
-                cookie.setSecure(secure);
-
                 if (!targetURL.getProtocol().equalsIgnoreCase("https")) {
                     LOG.debug("Skipping cookie {}: secure and {} is not https", name, targetURL);
                     continue;
                 }
             }
 
             // check expiration
+            Instant expirationDate = null;
             if (expires != null) {
                 try {
-                    Date expirationDate = org.apache.http.client.utils.DateUtils.parseDate(expires);
+                    expirationDate = parseCookieExpiry(expires);
                     if (expirationDate != null) {
-                        cookie.setExpiryDate(expirationDate);
-
                         // check that it hasn't expired?
-                        if (cookie.isExpired(new Date())) {
+                        if (!expirationDate.isAfter(Instant.now())) {
                             LOG.debug("Skipping cookie {}: expired on {}", name, expires);
                             continue;
                         }
@@ -210,12 +231,36 @@ public static List<Cookie> getCookies(String[] cookiesStrings, URL originURL, UR
             }
 
             // attach additional infos to cookie
-            list.add(cookie);
+            list.add(
+                    new Cookie(
+                            name,
+                            value,
+                            domain == null || domain.isBlank()
+                                    ? null
+                                    : domain.toLowerCase(Locale.ROOT),
+                            path,
+                            secure,
+                            expirationDate));
         }
 
         return list;
     }
 
+    static Instant parseCookieExpiry(String expires) {
+        // Preserve legacy date parsing so expired cookies do not become session cookies.
+        String value = expires;
+        if (value.length() > 1 && value.startsWith("'") && value.endsWith("'")) {
+            value = value.substring(1, value.length() - 1);
+        }
+        for (SimpleDateFormat format : COOKIE_DATE_FORMATS.get()) {
+            Date legacyDate = format.parse(value, new ParsePosition(0));
+            if (legacyDate != null) {
+                return legacyDate.toInstant();
+            }
+        }
+        return null;
+    }
+
     /**
      * Normalises the value of a cookie domain attribute into the domain it scopes the cookie to.
      *

core/src/test/java/org/apache/stormcrawler/bolt/FeedParserBoltTest.java

@@ -21,7 +21,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.storm.task.OutputCollector;
 import org.apache.stormcrawler.Constants;
 import org.apache.stormcrawler.Metadata;

core/src/test/java/org/apache/stormcrawler/bolt/SiteMapParserBoltTest.java

@@ -26,7 +26,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.storm.task.OutputCollector;
 import org.apache.storm.tuple.Values;
 import org.apache.stormcrawler.Constants;

core/src/test/java/org/apache/stormcrawler/filtering/BasicURLNormalizerTest.java

@@ -44,6 +44,37 @@ class BasicURLNormalizerTest {
 
     List<String> queryParamsToFilter = Arrays.asList("a", "foo");
 
+    @Test
+    void testMixedQuerySeparatorsAndEncoding() {
+        URLFilter filter = createFilter(List.of("tracking"));
+        assertEquals(
+                "http://example.com/?a=2&b=1&c=x+y+z&d",
+                filter.filter(null, new Metadata(), "http://example.com/?b=1;a=2&c=x+y%20z&d"));
+        assertEquals(
+                "http://example.com/?a=&b=x%3By%26z&b=%2B&flag",
+                filter.filter(
+                        null, new Metadata(), "http://example.com/?flag;b=x%3By%26z&b=%2B;a="));
+        assertEquals(
+                "http://example.com/?a=%C3%A9&b=%E2%82%AC",
+                filter.filter(null, new Metadata(), "http://example.com/?b=%E2%82%AC;a=%C3%A9"));
+    }
+
+    @Test
+    void testQueryFormattingKeepsLegacyEncoding() {
+        URLFilter filter = createFilter(List.of("tracking"));
+        assertEquals(
+                "http://example.com/?a=~%21%27%28%29*&b=2",
+                filter.filter(null, new Metadata(), "http://example.com/?b=2&a=~!'()*"));
+    }
+
+    @Test
+    void testRemoveSemicolonSeparatedQueryParameters() {
+        URLFilter filter = createFilter(queryParamsToFilter);
+        assertEquals(
+                "http://example.com/?keep=yes",
+                filter.filter(null, new Metadata(), "http://example.com/?foo=bar;keep=yes&a=1"));
+    }
+
     private URLFilter createFilter(boolean removeAnchor, boolean checkValidURI) {
         ObjectNode filterParams = new ObjectNode(JsonNodeFactory.instance);
         filterParams.put("removeAnchorPart", Boolean.valueOf(removeAnchor));

core/src/test/java/org/apache/stormcrawler/persistence/AdaptiveSchedulerTest.java

@@ -31,7 +31,7 @@
 import java.util.Optional;
 import java.util.TimeZone;
 import org.apache.commons.lang3.time.DateUtils;
-import org.apache.http.HttpHeaders;
+import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.stormcrawler.Metadata;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;