diff --git a/core/src/main/java/org/apache/stormcrawler/bolt/JSoupParserBolt.java b/core/src/main/java/org/apache/stormcrawler/bolt/JSoupParserBolt.java index 43a591c74..dc8c32c92 100644 --- a/core/src/main/java/org/apache/stormcrawler/bolt/JSoupParserBolt.java +++ b/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> 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) { diff --git a/core/src/main/java/org/apache/stormcrawler/util/CharsetIdentification.java b/core/src/main/java/org/apache/stormcrawler/util/CharsetIdentification.java index c9ebf21d5..9fc778dd3 100644 --- a/core/src/main/java/org/apache/stormcrawler/util/CharsetIdentification.java +++ b/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 (), TestUtil.getMockedTopologyContext(), new OutputCollector(output)); + try (MockedStatic 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> 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 { diff --git a/core/src/test/java/org/apache/stormcrawler/util/CharsetIdentificationTest.java b/core/src/test/java/org/apache/stormcrawler/util/CharsetIdentificationTest.java new file mode 100644 index 000000000..0a22c7a5d --- /dev/null +++ b/core/src/test/java/org/apache/stormcrawler/util/CharsetIdentificationTest.java @@ -0,0 +1,145 @@ +/* + * 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; + + /** + * META_CHARSET_LOOKAHEAD in CharsetIdentification: bytes read past the window for the quote. + */ + private static final int LOOKAHEAD = 64; + + /** 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 = " 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); + } + + /** ASCII content, no BOM and no HTTP header: only the meta tag can yield this charset. */ + private static final String DECLARED = "windows-1251"; + + /** More than the look-ahead of body behind the tag, so that the look-ahead bounds the read. */ + private static final String BODY = + "" + "

text

".repeat(20) + ""; + + /** A declaration cut by the detection window is still read, see #870. */ + @Test + void metaCharsetCutByTheDetectionWindowIsStillRead() { + StringBuilder page = new StringBuilder(""); + while (page.length() < MAXLENGTH) { + page.append(""); + } + // the window ends inside the charset name + int cut = page.length() + "").append(BODY); + byte[] content = page.toString().getBytes(StandardCharsets.US_ASCII); + + String charset = CharsetIdentification.getCharsetFast(new Metadata(), content, cut); + + Assertions.assertEquals(DECLARED, charset); + } + + /** + * A page whose declaration opens on the last bytes of a window of {@link #MAXLENGTH} and closes + * {@code quoteOffset} bytes after it. The name is padded with spaces, which the validation + * trims, so the charset is used whenever the closing quote is read. + */ + private static byte[] metaCharsetClosingAfterTheWindow(int quoteOffset) { + StringBuilder page = new StringBuilder(""); + while (page.length() < MAXLENGTH) { + page.append(""); + } + page.setLength(MAXLENGTH - ""); + page.append(BODY); + return page.toString().getBytes(StandardCharsets.US_ASCII); + } + + /** The look-ahead is bounded: a quote on its last byte is read, one byte further is not. */ + @Test + void lookAheadForTheClosingQuoteIsBounded() { + String within = + CharsetIdentification.getCharsetFast( + new Metadata(), metaCharsetClosingAfterTheWindow(LOOKAHEAD - 1), MAXLENGTH); + String beyond = + CharsetIdentification.getCharsetFast( + new Metadata(), metaCharsetClosingAfterTheWindow(LOOKAHEAD), MAXLENGTH); + + Assertions.assertEquals(DECLARED, within); + Assertions.assertNotEquals( + DECLARED, beyond, "a declaration closing beyond the look-ahead is not used"); + } +}