Fix NPE in escaping if output settings cloned twice (#1964)

* Fix NPE in escaping if output settings cloned twice

* Refactored coreCharset to be set in charset()

---------

Co-authored-by: Jonathan Hedley <jonathan@hedley.net>

CHANGES

@@ -31,6 +31,9 @@ Release 1.16.2 [PENDING]
   * Bugfix: `form` elements and empty elements (such as `img`) did not have their attributes de-duplicated.
     <https://github.com/jhy/jsoup/pull/1950>
 
+  * Bugfix: if Document.OutputSettings was cloned from a clone, an NPE would be thrown when used.
+    <https://github.com/jhy/jsoup/pull/1964>
+
   * Bugfix: in Jsoup.connect(url), URL paths containing a %2B were incorrectly recoded to a '+', or a '+' was recoded
     to a ' '. Fixed by reverting to the previous behavior of not encoding supplied paths, other than normalizing to
     ASCII.

src/main/java/org/jsoup/nodes/Document.java

@@ -385,17 +385,19 @@ public static class OutputSettings implements Cloneable {
         public enum Syntax {html, xml}
 
         private Entities.EscapeMode escapeMode = Entities.EscapeMode.base;
-        private Charset charset = DataUtil.UTF_8;
+        private Charset charset;
+        Entities.CoreCharset coreCharset; // fast encoders for ascii and utf8
         private final ThreadLocal<CharsetEncoder> encoderThreadLocal = new ThreadLocal<>(); // initialized by start of OuterHtmlVisitor
-        @Nullable Entities.CoreCharset coreCharset; // fast encoders for ascii and utf8
 
         private boolean prettyPrint = true;
         private boolean outline = false;
         private int indentAmount = 1;
         private int maxPaddingWidth = 30;
         private Syntax syntax = Syntax.html;
 
-        public OutputSettings() {}
+        public OutputSettings() {
+            charset(DataUtil.UTF_8);
+        }
 
         /**
          * Get the document's current HTML escape mode: <code>base</code>, which provides a limited set of named HTML
@@ -439,6 +441,7 @@ public Charset charset() {
          */
         public OutputSettings charset(Charset charset) {
             this.charset = charset;
+            coreCharset = Entities.CoreCharset.byName(charset.name());
             return this;
         }
 
@@ -456,7 +459,6 @@ CharsetEncoder prepareEncoder() {
             // created at start of OuterHtmlVisitor so each pass has own encoder, so OutputSettings can be shared among threads
             CharsetEncoder encoder = charset.newEncoder();
             encoderThreadLocal.set(encoder);
-            coreCharset = Entities.CoreCharset.byName(encoder.charset().name());
             return encoder;
         }
 
@@ -570,7 +572,7 @@ public OutputSettings clone() {
             } catch (CloneNotSupportedException e) {
                 throw new RuntimeException(e);
             }
-            clone.charset(charset.name()); // new charset and charset encoder
+            clone.charset(charset.name()); // new charset, coreCharset, and charset encoder
             clone.escapeMode = Entities.EscapeMode.valueOf(escapeMode.name());
             // indentAmount, maxPaddingWidth, and prettyPrint are primitives so object.clone() will handle
             return clone;

src/test/java/org/jsoup/nodes/EntitiesTest.java

@@ -6,6 +6,7 @@
 
 import static org.jsoup.nodes.Document.OutputSettings;
 import static org.jsoup.nodes.Entities.EscapeMode.*;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 public class EntitiesTest {
@@ -163,4 +164,15 @@ public class EntitiesTest {
         Document xml = Jsoup.parse(input, "", Parser.xmlParser());
         assertEquals(input, xml.html());
     }
+
+    @Test public void escapeByClonedOutputSettings() {
+        OutputSettings outputSettings = new OutputSettings();
+        String text = "Hello &<> Å å π 新 there ¾ © »";
+        OutputSettings clone1 = outputSettings.clone();
+        OutputSettings clone2 = outputSettings.clone();
+
+        String escaped1 = assertDoesNotThrow(() -> Entities.escape(text, clone1));
+        String escaped2 = assertDoesNotThrow(() -> Entities.escape(text, clone2));
+        assertEquals(escaped1, escaped2);
+    }
 }