#2092 Bound the meta charset look-ahead and catch charset detection failures in JSoupParserBolt

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

@@ -269,22 +269,6 @@ public void execute(Tuple tuple) {
 
         long start = System.currentTimeMillis();
 
-        String charset;
-
-        if (fastCharsetDetection) {
-            charset =
-                    CharsetIdentification.getCharsetFast(
-                            metadata, content, maxLengthCharsetDetection);
-        } else {
-            charset =
-                    CharsetIdentification.getCharset(metadata, content, maxLengthCharsetDetection);
-        }
-
-        LOG.debug(
-                "Charset identified as {} in {} msec",
-                charset,
-                (System.currentTimeMillis() - start));
-
         RobotsTags robotsTags = new RobotsTags();
 
         // get the robots tags from the fetch metadata
@@ -295,8 +279,25 @@ public void execute(Tuple tuple) {
         Map<String, List<String>> slinks;
         String text;
         final org.jsoup.nodes.Document jsoupDoc;
+        String charset;
 
         try {
+            // inside the try: a failure here is a parse error of this URL, not a dead worker
+            if (fastCharsetDetection) {
+                charset =
+                        CharsetIdentification.getCharsetFast(
+                                metadata, content, maxLengthCharsetDetection);
+            } else {
+                charset =
+                        CharsetIdentification.getCharset(
+                                metadata, content, maxLengthCharsetDetection);
+            }
+
+            LOG.debug(
+                    "Charset identified as {} in {} msec",
+                    charset,
+                    (System.currentTimeMillis() - start));
+
             String html = Charset.forName(charset).decode(ByteBuffer.wrap(content)).toString();
 
             if (isPlainText) {

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

@@ -44,6 +44,12 @@ public class CharsetIdentification {
     private static final Pattern charsetPattern =
             Pattern.compile("(?i)\\bcharset=\\s*(?:[\"'])?([^\\s,;\"']*)");
 
+    /**
+     * Bytes read beyond the detection window when a {@code <meta charset="} declaration is cut by
+     * it: room for any registered charset name and its closing quote.
+     */
+    private static final int META_CHARSET_LOOKAHEAD = 64;
+
     /**
      * Identifies the charset of a document based on the following logic: guess from the
      * ByteOrderMark - else return any charset specified in the http headers if any, otherwise
@@ -190,9 +196,11 @@ private static String getCharsetFromMeta(byte[] buffer, int maxlength) {
         if (start != -1) {
             int end = html.indexOf('"', start + 15);
             // https://github.com/apache/stormcrawler/issues/870
-            // try on a slightly larger section of text if it is trimmed
-            if (end == -1 && ((maxlength + 10) < buffer.length)) {
-                return getCharsetFromMeta(buffer, maxlength + 10);
+            // the declaration may be cut by the window: look a bounded distance further
+            if (end == -1 && len < buffer.length) {
+                int extended = Math.min(buffer.length, len + META_CHARSET_LOOKAHEAD);
+                html = new String(buffer, 0, extended, DEFAULT_CHARSET);
+                end = html.indexOf('"', start + 15);
             }
             if (end == -1) {
                 // there is an open tag meta but not closed = we have broken content!

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

@@ -28,10 +28,13 @@
 import org.apache.stormcrawler.TestUtil;
 import org.apache.stormcrawler.parse.ParsingTester;
 import org.apache.stormcrawler.persistence.Status;
+import org.apache.stormcrawler.util.CharsetIdentification;
 import org.apache.stormcrawler.util.RobotsTags;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
 
 class JSoupParserBoltTest extends ParsingTester {
 
@@ -115,6 +118,31 @@ void setupParserBolt() {
         setupParserBolt(bolt);
     }
 
+    /** A failure inside charset detection is a parse error of that URL, not a dead worker. */
+    @Test
+    void charsetDetectionFailureIsReportedAsParseError() throws IOException {
+        bolt.prepare(
+                new HashMap<>(), TestUtil.getMockedTopologyContext(), new OutputCollector(output));
+        try (MockedStatic<CharsetIdentification> detection =
+                Mockito.mockStatic(CharsetIdentification.class)) {
+            detection
+                    .when(
+                            () ->
+                                    CharsetIdentification.getCharset(
+                                            Mockito.any(), Mockito.any(), Mockito.anyInt()))
+                    .thenThrow(new StackOverflowError());
+            parse("https://stormcrawler.apache.org", "stormcrawler.apache.org.html");
+        }
+        List<List<Object>> statusTuples = output.getEmitted(Constants.StatusStreamName);
+        Assertions.assertEquals(1, statusTuples.size());
+        Assertions.assertEquals(Status.ERROR, statusTuples.get(0).get(2));
+        Metadata metadata = (Metadata) statusTuples.get(0).get(1);
+        Assertions.assertEquals(
+                "content parsing", metadata.getFirstValue(Constants.STATUS_ERROR_SOURCE));
+        Assertions.assertEquals(1, output.getAckedTuples().size());
+        Assertions.assertTrue(output.getEmitted().isEmpty(), "no document must be emitted");
+    }
+
     /** Checks that content in script is not included in the text representation. */
     @Test
     void testNoScriptInText() throws IOException {

core/src/test/java/org/apache/stormcrawler/util/CharsetIdentificationTest.java

@@ -0,0 +1,103 @@
+/*
+ * 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.util;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.stormcrawler.Metadata;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class CharsetIdentificationTest {
+
+    /** detect.charset.maxlength as set in crawler-default.yaml. */
+    private static final int MAXLENGTH = 10000;
+
+    /** A body which opens a meta charset declaration and never closes it. */
+    private static byte[] unterminatedMetaCharset(int size) {
+        byte[] content = new byte[size];
+        Arrays.fill(content, (byte) 'A');
+        byte[] prefix = "<meta charset=\"".getBytes(StandardCharsets.US_ASCII);
+        System.arraycopy(prefix, 0, content, 0, prefix.length);
+        return content;
+    }
+
+    /**
+     * Runs the detection on a thread with a fixed stack so that the outcome does not depend on the
+     * JVM defaults, and returns what it threw, if anything.
+     */
+    private static Throwable detectOnSmallStack(byte[] content, int maxlength)
+            throws InterruptedException {
+        AtomicReference<Throwable> thrown = new AtomicReference<>();
+        Thread thread =
+                new Thread(
+                        null,
+                        () -> {
+                            try {
+                                CharsetIdentification.getCharset(
+                                        new Metadata(), content, maxlength);
+                            } catch (Throwable t) {
+                                thrown.set(t);
+                            }
+                        },
+                        "charset-detection",
+                        1024 * 1024);
+        thread.start();
+        thread.join();
+        return thrown.get();
+    }
+
+    @Test
+    void largeDocumentWithUnterminatedMetaCharsetDoesNotOverflowTheStack() throws Exception {
+        // 400 KB is an ordinary page size, fetched whole under the default http.content.limit
+        Throwable thrown = detectOnSmallStack(unterminatedMetaCharset(400_000), MAXLENGTH);
+        Assertions.assertNull(thrown, "charset detection threw " + thrown);
+    }
+
+    @Test
+    void smallDocumentWithUnterminatedMetaCharsetIsHandled() throws Exception {
+        Throwable thrown = detectOnSmallStack(unterminatedMetaCharset(20_000), MAXLENGTH);
+        Assertions.assertNull(thrown, "charset detection threw " + thrown);
+    }
+
+    @Test
+    void unterminatedMetaCharsetIsHandledWithFullContentDetection() throws Exception {
+        Throwable thrown = detectOnSmallStack(unterminatedMetaCharset(400_000), -1);
+        Assertions.assertNull(thrown, "charset detection threw " + thrown);
+    }
+
+    /** A declaration cut by the detection window is still read, see #870. */
+    @Test
+    void metaCharsetCutByTheDetectionWindowIsStillRead() {
+        // ASCII content, no BOM and no HTTP header: only the meta tag can yield this charset
+        String declaration = "<meta charset=\"windows-1251\">";
+        StringBuilder page = new StringBuilder("<html><head>");
+        while (page.length() < MAXLENGTH) {
+            page.append("<!-- padding -->");
+        }
+        // the window ends inside the charset name, the buffer ends right after the tag
+        int cut = page.length() + "<meta charset=\"win".length();
+        page.append(declaration).append("</head><body></body></html>");
+        byte[] content = page.toString().getBytes(StandardCharsets.US_ASCII);
+
+        String charset = CharsetIdentification.getCharsetFast(new Metadata(), content, cut);
+
+        Assertions.assertEquals("windows-1251", charset);
+    }
+}