Document Cleaner and Safelist thread safety

And add a guard test

Fixes #2473

Author: jhy

src/main/java/org/jsoup/safety/Cleaner.java

@@ -17,19 +17,24 @@
 import static org.jsoup.internal.SharedConstants.DummyUri;
 
 /**
- The safelist based HTML cleaner. Use to ensure that end-user provided HTML contains only the elements and attributes
+ The {@link Safelist}-based HTML cleaner. Use to ensure that end-user provided HTML contains only the elements and attributes
  that you are expecting; no junk, and no cross-site scripting attacks!
  <p>
- The HTML cleaner parses the input as HTML and then runs it through a safe-list, so the output HTML can only contain
+ The HTML cleaner parses the input as HTML and then runs it through a safelist, so the output HTML can only contain
  HTML that is allowed by the safelist.
  </p>
  <p>
  It is assumed that the input HTML is a body fragment; the clean methods only pull from the source's body, and the
- canned safe-lists only allow body contained tags.
+ canned safelists only allow body-contained tags.
  </p>
  <p>
  Rather than interacting directly with a Cleaner object, generally see the {@code clean} methods in {@link org.jsoup.Jsoup}.
  </p>
+ <p>
+ A Cleaner may be reused across multiple documents and shared across concurrent threads once its {@link Safelist} has
+ been configured. The cleaner uses the supplied safelist directly, so later safelist changes affect later cleaning
+ calls. If you need a variant of an existing configuration, use {@link Safelist#Safelist(Safelist)} to make a copy.
+ </p>
  */
 public class Cleaner {
     private final Safelist safelist;

src/main/java/org/jsoup/safety/Safelist.java

@@ -22,7 +22,7 @@ Thank you to Ryan Grove (wonko.com) for the Ruby HTML cleaner http://github.com/
 
 
 /**
- Safe-lists define what HTML (elements and attributes) to allow through the cleaner. Everything else is removed.
+ Safelists define what HTML (elements and attributes) to allow through a {@link Cleaner}. Everything else is removed.
  <p>
  Start with one of the defaults:
  </p>
@@ -53,15 +53,20 @@ If you need to allow more through (please be careful!), tweak a base safelist wi
  </ul>
 
  <p>
- The cleaner and these safelists assume that you want to clean a <code>body</code> fragment of HTML (to add user
+ The {@link Cleaner} and these safelists assume that you want to clean a <code>body</code> fragment of HTML (to add user
  supplied HTML into a templated page), and not to clean a full HTML document. If the latter is the case, you could wrap
  the templated document HTML around the cleaned body HTML.
  </p>
  <p>
+ Safelists are mutable. A {@link Cleaner} uses the supplied safelist directly, so later changes affect later cleaning
+ calls. If you want to share a safelist across threads, finish configuring it first and do not mutate it while it is in
+ use. To build a variant from an existing configuration, use {@link #Safelist(Safelist)} to make a copy.
+ </p>
+ <p>
  If you are going to extend a safelist, please be very careful. Make sure you understand what attributes may lead to
  XSS attack vectors. URL attributes are particularly vulnerable and require careful validation. See 
  the <a href="https://owasp.org/www-community/xss-filter-evasion-cheatsheet">XSS Filter Evasion Cheat Sheet</a> for some
- XSS attack examples (that jsoup will safegaurd against the default Cleaner and Safelist configuration).
+ XSS attack examples (that jsoup will safeguard against with the default Cleaner and Safelist configuration).
  </p>
  */
 public class Safelist {

src/test/java/org/jsoup/safety/CleanerTest.java

@@ -16,6 +16,9 @@
 
 import java.util.Arrays;
 import java.util.Locale;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -226,6 +229,53 @@ public void safeListedProtocolShouldBeRetained(Locale locale) {
         assertFalse(new Cleaner(Safelist.none()).isValid(okDoc));
     }
 
+    @Test void configuredCleanerMayBeSharedAcrossThreads() throws InterruptedException {
+        // https://github.com/jhy/jsoup/issues/2473
+        String html = "<a href='/foo'>Link</a><img src='/bar' alt='Q'>";
+        String baseUri = "https://example.com/";
+        String expected = "<a href=\"https://example.com/foo\">Link</a><img src=\"https://example.com/bar\" alt=\"Q\">";
+        Cleaner cleaner = new Cleaner(Safelist.basicWithImages());
+
+        int numThreads = 10;
+        int numLoops = 20;
+        String[] cleaned = new String[numThreads * numLoops];
+        AtomicInteger next = new AtomicInteger();
+        AtomicReference<Throwable> failure = new AtomicReference<>();
+        CountDownLatch start = new CountDownLatch(1);
+        CountDownLatch done = new CountDownLatch(numThreads);
+        Thread[] threads = new Thread[numThreads];
+
+        for (int i = 0; i < numThreads; i++) {
+            Thread thread = new Thread(() -> {
+                try {
+                    start.await();
+                    for (int j = 0; j < numLoops; j++) {
+                        Document dirty = Jsoup.parseBodyFragment(html, baseUri);
+                        cleaned[next.getAndIncrement()] = cleaner.clean(dirty).body().html();
+                    }
+                } catch (Throwable t) {
+                    failure.compareAndSet(null, t);
+                    if (t instanceof InterruptedException) Thread.currentThread().interrupt();
+                } finally {
+                    done.countDown();
+                }
+            });
+            threads[i] = thread;
+            thread.start();
+        }
+
+        start.countDown();
+        done.await();
+
+        if (failure.get() != null)
+            throw new AssertionError("Concurrent cleaner use failed", failure.get());
+
+        assertEquals(cleaned.length, next.get());
+        for (String clean : cleaned) {
+            assertEquals(expected, clean);
+        }
+    }
+
     @Test public void resolvesRelativeLinks() {
         String html = "<a href='/foo'>Link</a><img src='/bar'>";
         String clean = Jsoup.clean(html, "http://example.com/", Safelist.basicWithImages());