Clone the Parser when cloning a Document

jhy · jhy · commit d55469aa637a · 2025-03-03T07:12:18.000+11:00
This ensures that concurrent calls to `doc.append(html)`, which uses the document's parser, are supported. Fixes #2281
diff --git a/src/main/java/org/jsoup/nodes/Document.java b/src/main/java/org/jsoup/nodes/Document.java
@@ -294,6 +294,7 @@ public boolean updateMetaCharsetElement() {
     public Document clone() {
         Document clone = (Document) super.clone();
         clone.outputSettings = this.outputSettings.clone();
+        clone.parser = this.parser.clone();
         return clone;
     }
 
diff --git a/src/main/java/org/jsoup/parser/Parser.java b/src/main/java/org/jsoup/parser/Parser.java
@@ -14,7 +14,7 @@
  {@link org.jsoup.Jsoup}.
  <p>Note that a Parser instance object is not threadsafe. To reuse a Parser configuration in a multi-threaded
  environment, use {@link #newInstance()} to make copies. */
-public class Parser {
+public class Parser implements Cloneable {
     public static final String NamespaceHtml = "http://www.w3.org/1999/xhtml";
     public static final String NamespaceXml = "http://www.w3.org/XML/1998/namespace";
     public static final String NamespaceMathml = "http://www.w3.org/1998/Math/MathML";
@@ -43,6 +43,12 @@ public Parser newInstance() {
         return new Parser(this);
     }
 
+    @SuppressWarnings("MethodDoesntCallSuperMethod") // because we use the copy constructor instead
+    @Override
+    public Parser clone() {
+        return new Parser(this);
+    }
+
     private Parser(Parser copy) {
         treeBuilder = copy.treeBuilder.newInstance(); // because extended
         errors = new ParseErrorList(copy.errors); // only copies size, not contents
diff --git a/src/test/java/org/jsoup/nodes/DocumentTest.java b/src/test/java/org/jsoup/nodes/DocumentTest.java
@@ -109,9 +109,14 @@ public class DocumentTest {
     @Test public void testClone() {
         Document doc = Jsoup.parse("<title>Hello</title> <p>One<p>Two");
         Document clone = doc.clone();
+        assertNotSame(doc, clone);
+        assertTrue(doc.hasSameValue(clone));
+        assertNotSame(doc.parser(), clone.parser());
+        assertNotSame(doc.outputSettings(), clone.outputSettings());
 
         assertEquals("<html><head><title>Hello</title></head><body><p>One</p><p>Two</p></body></html>", TextUtil.stripNewlines(clone.html()));
         clone.title("Hello there");
+        assertFalse(doc.hasSameValue(clone));
         clone.expectFirst("p").text("One more").attr("id", "1");
         assertEquals("<html><head><title>Hello there</title></head><body><p id=\"1\">One more</p><p>Two</p></body></html>", TextUtil.stripNewlines(clone.html()));
         assertEquals("<html><head><title>Hello</title></head><body><p>One</p><p>Two</p></body></html>", TextUtil.stripNewlines(doc.html()));
diff --git a/src/test/java/org/jsoup/nodes/ElementIT.java b/src/test/java/org/jsoup/nodes/ElementIT.java
@@ -134,4 +134,53 @@ public void testFastReparentExistingContent() {
         assertTrue(html.startsWith("<div>"));
         assertEquals(num + 3, el.parents().size());
     }
+
+    @Test
+    public void testConcurrentMerge() throws Exception {
+        // https://github.com/jhy/jsoup/discussions/2280 / https://github.com/jhy/jsoup/issues/2281
+        // This was failing because the template.clone().append(html) was reusing the same underlying parser object.
+        // Document#clone now correctly clones its parser.
+
+        class TemplateMerger {
+            private final Document templateDoc;
+
+            public TemplateMerger(Document template) {
+                this.templateDoc = template;
+            }
+
+            public Document mergeWith(Document inputDoc) {
+                Document merged = templateDoc.clone();
+                Element content = merged.getElementById("content");
+                content.append(inputDoc.html());
+                return merged;
+            }
+        }
+
+        String templateHtml = "<html><body><div id='content'></div></body></html>";
+        Document templateDoc = Jsoup.parse(templateHtml);
+        TemplateMerger merger = new TemplateMerger(templateDoc);
+
+        Runnable mergeTask = () -> {
+            try {
+                for (int i = 0; i < 1000; i++) {
+                    String inputHtml = "<html><body><p>Some content</p></body></html>";
+                    Document inputDoc = Jsoup.parse(inputHtml);
+                    Document merged = merger.mergeWith(inputDoc);
+                    assertNotNull(merged);
+                }
+            } catch (Exception e) {
+                fail(e);
+            }
+        };
+
+        int threadCount = 10;
+        Thread[] threads = new Thread[threadCount];
+        for (int i = 0; i < threadCount; i++) {
+            threads[i] = new Thread(mergeTask);
+            threads[i].start();
+        }
+        for (Thread t : threads) {
+            t.join();
+        }
+    }
 }
diff --git a/src/test/java/org/jsoup/parser/ParserTest.java b/src/test/java/org/jsoup/parser/ParserTest.java
@@ -9,6 +9,7 @@
 import java.nio.charset.StandardCharsets;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
 
 public class ParserTest {
 
@@ -36,4 +37,25 @@ public void testUtf8() throws IOException {
         String text = parsed.selectFirst("p").wholeText();
         assertEquals(text, "H\u00E9llo, w\u00F6rld!");
     }
+
+    @Test
+    public void testClone() {
+        // Test HTML parser cloning
+        Parser htmlParser = Parser.htmlParser();
+        Parser htmlClone = htmlParser.clone();
+        assertNotSame(htmlParser, htmlClone);
+        // Ensure the tree builder instances are different
+        assertNotSame(htmlParser.getTreeBuilder(), htmlClone.getTreeBuilder());
+        // Check that settings are cloned properly (for example, tag case settings)
+        assertEquals(htmlParser.settings().preserveTagCase(), htmlClone.settings().preserveTagCase());
+        assertEquals(htmlParser.settings().preserveAttributeCase(), htmlClone.settings().preserveAttributeCase());
+
+        // Test XML parser cloning
+        Parser xmlParser = Parser.xmlParser();
+        Parser xmlClone = xmlParser.clone();
+        assertNotSame(xmlParser, xmlClone);
+        assertNotSame(xmlParser.getTreeBuilder(), xmlClone.getTreeBuilder());
+        assertEquals(xmlParser.settings().preserveTagCase(), xmlClone.settings().preserveTagCase());
+        assertEquals(xmlParser.settings().preserveAttributeCase(), xmlClone.settings().preserveAttributeCase());
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -294,6 +294,7 @@ public boolean updateMetaCharsetElement() {`
`294`	`294`	`public Document clone() {`
`295`	`295`	`Document clone = (Document) super.clone();`
`296`	`296`	`clone.outputSettings = this.outputSettings.clone();`
	`297`	`+ clone.parser = this.parser.clone();`
`297`	`298`	`return clone;`
`298`	`299`	`}`
`299`	`300`