Avoid unnecessary copies (#302)

* Avoid unnecessary copies Signed-off-by: Sven Strickroth <[email protected]> * Update CssSchema.java --------- Signed-off-by: Sven Strickroth <[email protected]> Co-authored-by: Mike Samuel <[email protected]>
OWASP · Feb 2, 2024 · 3e32abc · 3e32abc
1 parent 2901ef0
commit 3e32abc
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 33 deletions.
diff --git a/src/main/java/org/owasp/html/CssSchema.java b/src/main/java/org/owasp/html/CssSchema.java
@@ -151,7 +151,7 @@ public static CssSchema withProperties(
       if (prop == null) { throw new IllegalArgumentException(propertyName); }
       propertiesBuilder.put(propertyName, prop);
     }
-    return new CssSchema(Map.copyOf(propertiesBuilder));
+    return new CssSchema(Collections.unmodifiableMap(propertiesBuilder));
   }
 
   /**
@@ -161,7 +161,7 @@ public static CssSchema withProperties(
    */
   public static CssSchema withProperties(
       Map<? extends String, ? extends Property> properties) {
-    Map<String, Property> propertyMap =
+    Map<String, Property> propertyMapBuilder =
         new HashMap<>();
     // check that all fnKeys are defined in properties.
     for (Map.Entry<? extends String, ? extends Property> e : properties.entrySet()) {
@@ -173,9 +173,9 @@ public static CssSchema withProperties(
               + " depends on undefined function key " + fnKey);
         }
       }
-      propertyMap.put(e.getKey(), e.getValue());
+      propertyMapBuilder.put(e.getKey(), e.getValue());
     }
-    return new CssSchema(Map.copyOf(propertyMap));
+    return new CssSchema(Collections.unmodifiableMap(propertyMapBuilder));
   }
 
   /**
@@ -188,7 +188,7 @@ public static CssSchema withProperties(
    */
   public static CssSchema union(CssSchema... cssSchemas) {
     if (cssSchemas.length == 1) { return cssSchemas[0]; }
-    Map<String, Property> properties = new LinkedHashMap<>();
+    Map<String, Property> propertyMapBuilder = new LinkedHashMap<>();
     for (CssSchema cssSchema : cssSchemas) {
       for (Map.Entry<String, Property> e : cssSchema.properties.entrySet()) {
         String name = e.getKey();
@@ -199,14 +199,14 @@ public static CssSchema union(CssSchema... cssSchemas) {
         if (Objects.isNull(newProp)) {
           throw new NullPointerException("An entry was returned with null value from cssSchema.properties");
         }
-        Property oldProp = properties.put(name, newProp);
+        Property oldProp = propertyMapBuilder.put(name, newProp);
         if (oldProp != null && !oldProp.equals(newProp)) {
           throw new IllegalArgumentException(
               "Duplicate irreconcilable definitions for " + name);
         }
       }
     }
-    return new CssSchema(Map.copyOf(properties));
+    return new CssSchema(Collections.unmodifiableMap(propertyMapBuilder));
   }
 
   /**
@@ -847,15 +847,15 @@ Property forKey(String propertyName) {
     builder.put("z-index", bottom);
     builder.put("repeating-linear-gradient()", linearGradient$Fun);
     builder.put("repeating-radial-gradient()", radialGradient$Fun);
-	DEFINITIONS = Map.copyOf(builder);
+    DEFINITIONS = Collections.unmodifiableMap(builder);
   }
 
   private static <T> Set<T> union(Set<T>... subsets) {
     Set<T> all = new HashSet<>();
     for (Set<T> subset : subsets) {
       all.addAll(subset);
     }
-    return Set.copyOf(all);
+    return Collections.unmodifiableSet(all);
   }
 
   static final Set<String> DEFAULT_WHITELIST = Set.of(

diff --git a/src/main/java/org/owasp/html/ElementAndAttributePolicies.java b/src/main/java/org/owasp/html/ElementAndAttributePolicies.java
@@ -28,6 +28,7 @@
 
 package org.owasp.html;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -81,7 +82,7 @@ ElementAndAttributePolicies and(ElementAndAttributePolicies p) {
     return new ElementAndAttributePolicies(
         elementName,
         ElementPolicy.Util.join(elPolicy, p.elPolicy),
-        Map.copyOf(joinedAttrPolicies),
+        Collections.unmodifiableMap(joinedAttrPolicies),
         this.htmlTagSkipType.and(p.htmlTagSkipType));
   }
 

diff --git a/src/main/java/org/owasp/html/ElementPolicy.java b/src/main/java/org/owasp/html/ElementPolicy.java
@@ -29,6 +29,7 @@
 package org.owasp.html;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -133,7 +134,7 @@ final class JoinedElementPolicy implements ElementPolicy {
   JoinedElementPolicy(Iterable<? extends ElementPolicy> policies) {
     List<ElementPolicy> builder = new ArrayList<>();
 	policies.forEach(builder::add);
-    this.policies = List.copyOf(builder);
+    this.policies = Collections.unmodifiableList(builder);
   }
 
   public @Nullable String apply(String elementName, List<String> attrs) {

diff --git a/src/main/java/org/owasp/html/FilterUrlByProtocolAttributePolicy.java b/src/main/java/org/owasp/html/FilterUrlByProtocolAttributePolicy.java
@@ -28,6 +28,7 @@
 
 package org.owasp.html;
 
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -66,7 +67,7 @@ public FilterUrlByProtocolAttributePolicy(
       Iterable<? extends String> protocols) {
     Set<String> builder = new HashSet<>();
     protocols.forEach(builder::add);
-    this.protocols = Set.copyOf(builder);
+    this.protocols = Collections.unmodifiableSet(builder);
   }
 
   public @Nullable String apply(

diff --git a/src/main/java/org/owasp/html/HtmlElementTables.java b/src/main/java/org/owasp/html/HtmlElementTables.java
@@ -1,6 +1,7 @@
 package org.owasp.html;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
@@ -418,7 +419,7 @@ public int getElementNameIndex(String canonName) {
         for (int i = 0, n = this.canonNames.size(); i < n; ++i) {
           builder.put(this.canonNames.get(i), i);
         }
-        canonNameToIndex = Map.copyOf(builder);
+        canonNameToIndex = Collections.unmodifiableMap(builder);
         this.customElementIndex = canonNames.indexOf(CUSTOM_ELEMENT_NAME);
         if (this.customElementIndex < 0) {
           throw new IllegalStateException("Negative element index");

diff --git a/src/main/java/org/owasp/html/HtmlEntities.java b/src/main/java/org/owasp/html/HtmlEntities.java
@@ -28,6 +28,7 @@
 
 package org.owasp.html;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -2290,7 +2291,7 @@ final class HtmlEntities {
       }
     }
 
-    final Map<String, String> entityNameToCodePointMap = Map.copyOf(builder);
+    final Map<String, String> entityNameToCodePointMap = Collections.unmodifiableMap(builder);
 
     ENTITY_TRIE = new Trie<String>(entityNameToCodePointMap);
     LONGEST_ENTITY_NAME = longestEntityName;

diff --git a/src/main/java/org/owasp/html/HtmlPolicyBuilder.java b/src/main/java/org/owasp/html/HtmlPolicyBuilder.java
@@ -29,6 +29,7 @@
 package org.owasp.html;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
@@ -170,7 +171,7 @@ public class HtmlPolicyBuilder {
     for (String elementName : DEFAULT_SKIP_IF_EMPTY) {
       builder.put(elementName, HtmlTagSkipType.SKIP_BY_DEFAULT);
     }
-    DEFAULT_SKIP_TAG_MAP_IF_EMPTY_ATTR = Map.copyOf(builder);
+    DEFAULT_SKIP_TAG_MAP_IF_EMPTY_ATTR = Collections.unmodifiableMap(builder);
   }
 
   /**
@@ -347,7 +348,7 @@ public AttributeBuilder allowAttributes(String... attributeNames) {
     for (String attributeName : attributeNames) {
       builder.add(HtmlLexer.canonicalAttributeName(attributeName));
     }
-    return new AttributeBuilder(List.copyOf(builder));
+    return new AttributeBuilder(Collections.unmodifiableList(builder));
   }
 
   /**
@@ -647,7 +648,7 @@ AttributePolicy makeGuard(AttributeGuardIntermediates intermediates) {
       }
 
     });
-    ATTRIBUTE_GUARDS = Map.copyOf(builder);
+    ATTRIBUTE_GUARDS = Collections.unmodifiableMap(builder);
   }
 
   /**
@@ -686,17 +687,17 @@ public <CTX> HtmlSanitizer.Policy build(
    * each backed by a different output channel.
    */
   public PolicyFactory toFactory() {
-    Set<String> textContainerSet = new HashSet<>();
+    Set<String> textContainerSetBuilder = new HashSet<>();
     for (Map.Entry<String, Boolean> textContainer
          : this.textContainers.entrySet()) {
       if (Boolean.TRUE.equals(textContainer.getValue())) {
-        textContainerSet.add(textContainer.getKey());
+        textContainerSetBuilder.add(textContainer.getKey());
       }
     }
     CompiledState compiled = compilePolicies();
 
     return new PolicyFactory(
-        compiled.compiledPolicies, Set.copyOf(textContainerSet),
+        compiled.compiledPolicies, Collections.unmodifiableSet(textContainerSetBuilder),
         Map.copyOf(compiled.globalAttrPolicies),
         preprocessor, postprocessor);
   }
@@ -812,7 +813,7 @@ private CompiledState compilePolicies() {
         elAttrPolicies = Map.of();
       }
 
-      Map<String, AttributePolicy> attrs
+      Map<String, AttributePolicy> attrsBuilder
           = new HashMap<>();
 
       for (Map.Entry<String, AttributePolicy> ape : elAttrPolicies.entrySet()) {
@@ -822,7 +823,7 @@ private CompiledState compilePolicies() {
         if (globalAttrPolicies.containsKey(attributeName)) { continue; }
         AttributePolicy policy = ape.getValue();
         if (!AttributePolicy.REJECT_ALL_ATTRIBUTE_POLICY.equals(policy)) {
-          attrs.put(attributeName, policy);
+          attrsBuilder.put(attributeName, policy);
         }
       }
       for (Map.Entry<String, AttributePolicy> ape
@@ -831,21 +832,21 @@ private CompiledState compilePolicies() {
         AttributePolicy policy = AttributePolicy.Util.join(
             elAttrPolicies.get(attributeName), ape.getValue());
         if (!AttributePolicy.REJECT_ALL_ATTRIBUTE_POLICY.equals(policy)) {
-          attrs.put(attributeName, policy);
+          attrsBuilder.put(attributeName, policy);
         }
       }
 
       policiesBuilder.put(
           elementName,
           new ElementAndAttributePolicies(
               elementName,
-              elPolicy, Map.copyOf(attrs),
+              elPolicy, Collections.unmodifiableMap(attrsBuilder),
               getHtmlTagSkipType(elementName)
           )
       );
     }
     compiledState = new CompiledState(
-        globalAttrPolicies, Map.copyOf(policiesBuilder));
+        globalAttrPolicies, Collections.unmodifiableMap(policiesBuilder));
     return compiledState;
   }
 
@@ -982,7 +983,7 @@ public HtmlPolicyBuilder onElements(String... elementNames) {
         builder.add(HtmlLexer.canonicalElementName(elementName));
       }
       return HtmlPolicyBuilder.this.allowAttributesOnElements(
-          policy, attributeNames, List.copyOf(builder));
+          policy, attributeNames, Collections.unmodifiableList(builder));
     }
   }
 

diff --git a/src/main/java/org/owasp/html/PolicyFactory.java b/src/main/java/org/owasp/html/PolicyFactory.java
@@ -28,6 +28,7 @@
 
 package org.owasp.html;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -173,10 +174,10 @@ public PolicyFactory and(PolicyFactory f) {
     } else if (f.textContainers.containsAll(this.textContainers)) {
       allTextContainers = f.textContainers;
     } else {
-      Set<String> containers = new HashSet<>();
-      this.textContainers.forEach(containers::add);
-      f.textContainers.forEach(containers::add);
-      allTextContainers = Set.copyOf(containers);
+      Set<String> containerBuilder = new HashSet<>();
+      this.textContainers.forEach(containerBuilder::add);
+      f.textContainers.forEach(containerBuilder::add);
+      allTextContainers = Collections.unmodifiableSet(containerBuilder);
     }
     Map<String, AttributePolicy> allGlobalAttrPolicies;
     if (f.globalAttrPolicies.isEmpty()) {
@@ -200,7 +201,7 @@ public PolicyFactory and(PolicyFactory f) {
           ab.put(attrName, e.getValue());
         }
       }
-      allGlobalAttrPolicies = Map.copyOf(ab);
+      allGlobalAttrPolicies = Collections.unmodifiableMap(ab);
     }
     HtmlStreamEventProcessor compositionOfPreprocessors
         = HtmlStreamEventProcessor.Processors.compose(
@@ -209,7 +210,7 @@ public PolicyFactory and(PolicyFactory f) {
         = HtmlStreamEventProcessor.Processors.compose(
             this.postprocessor, f.postprocessor);
     return new PolicyFactory(
-        Map.copyOf(builder), allTextContainers, allGlobalAttrPolicies,
+        Collections.unmodifiableMap(builder), allTextContainers, allGlobalAttrPolicies,
         compositionOfPreprocessors, compositionOfPostprocessors);
   }
 }
diff --git a/src/test/java/org/owasp/html/SanitizersTest.java b/src/test/java/org/owasp/html/SanitizersTest.java
@@ -33,6 +33,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.BitSet;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
@@ -579,7 +580,7 @@ private static class Permutations<T> implements Iterable<List<T>> {
       this.k = k;
       List<T> builder = new ArrayList<>();
       Arrays.stream(elements).forEach(builder::add);
-      this.elements = List.copyOf(builder);
+      this.elements = Collections.unmodifiableList(builder);
     }
 
     public Iterator<List<T>> iterator() {