Skip to content

Commit 3e32abc

Browse files
cswaremikesamuel
csware
and
mikesamuel
authored
Avoid unnecessary copies (#302)
* Avoid unnecessary copies Signed-off-by: Sven Strickroth <email@cs-ware.de> * Update CssSchema.java --------- Signed-off-by: Sven Strickroth <email@cs-ware.de> Co-authored-by: Mike Samuel <mikesamuel@gmail.com>
1 parent 2901ef0 commit 3e32abc

File tree

9 files changed

+41
-33
lines changed

9 files changed

+41
-33
lines changed

‎src/main/java/org/owasp/html/CssSchema.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ public static CssSchema withProperties(
151151
if (prop == null) { throw new IllegalArgumentException(propertyName); }
152152
propertiesBuilder.put(propertyName, prop);
153153
}
154-
return new CssSchema(Map.copyOf(propertiesBuilder));
154+
return new CssSchema(Collections.unmodifiableMap(propertiesBuilder));
155155
}
156156

157157
/**
@@ -161,7 +161,7 @@ public static CssSchema withProperties(
161161
*/
162162
public static CssSchema withProperties(
163163
Map<? extends String, ? extends Property> properties) {
164-
Map<String, Property> propertyMap =
164+
Map<String, Property> propertyMapBuilder =
165165
new HashMap<>();
166166
// check that all fnKeys are defined in properties.
167167
for (Map.Entry<? extends String, ? extends Property> e : properties.entrySet()) {
@@ -173,9 +173,9 @@ public static CssSchema withProperties(
173173
+ " depends on undefined function key " + fnKey);
174174
}
175175
}
176-
propertyMap.put(e.getKey(), e.getValue());
176+
propertyMapBuilder.put(e.getKey(), e.getValue());
177177
}
178-
return new CssSchema(Map.copyOf(propertyMap));
178+
return new CssSchema(Collections.unmodifiableMap(propertyMapBuilder));
179179
}
180180

181181
/**
@@ -188,7 +188,7 @@ public static CssSchema withProperties(
188188
*/
189189
public static CssSchema union(CssSchema... cssSchemas) {
190190
if (cssSchemas.length == 1) { return cssSchemas[0]; }
191-
Map<String, Property> properties = new LinkedHashMap<>();
191+
Map<String, Property> propertyMapBuilder = new LinkedHashMap<>();
192192
for (CssSchema cssSchema : cssSchemas) {
193193
for (Map.Entry<String, Property> e : cssSchema.properties.entrySet()) {
194194
String name = e.getKey();
@@ -199,14 +199,14 @@ public static CssSchema union(CssSchema... cssSchemas) {
199199
if (Objects.isNull(newProp)) {
200200
throw new NullPointerException("An entry was returned with null value from cssSchema.properties");
201201
}
202-
Property oldProp = properties.put(name, newProp);
202+
Property oldProp = propertyMapBuilder.put(name, newProp);
203203
if (oldProp != null && !oldProp.equals(newProp)) {
204204
throw new IllegalArgumentException(
205205
"Duplicate irreconcilable definitions for " + name);
206206
}
207207
}
208208
}
209-
return new CssSchema(Map.copyOf(properties));
209+
return new CssSchema(Collections.unmodifiableMap(propertyMapBuilder));
210210
}
211211

212212
/**
@@ -847,15 +847,15 @@ Property forKey(String propertyName) {
847847
builder.put("z-index", bottom);
848848
builder.put("repeating-linear-gradient()", linearGradient$Fun);
849849
builder.put("repeating-radial-gradient()", radialGradient$Fun);
850-
DEFINITIONS = Map.copyOf(builder);
850+
DEFINITIONS = Collections.unmodifiableMap(builder);
851851
}
852852

853853
private static <T> Set<T> union(Set<T>... subsets) {
854854
Set<T> all = new HashSet<>();
855855
for (Set<T> subset : subsets) {
856856
all.addAll(subset);
857857
}
858-
return Set.copyOf(all);
858+
return Collections.unmodifiableSet(all);
859859
}
860860

861861
static final Set<String> DEFAULT_WHITELIST = Set.of(

‎src/main/java/org/owasp/html/ElementAndAttributePolicies.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
package org.owasp.html;
3030

31+
import java.util.Collections;
3132
import java.util.HashMap;
3233
import java.util.LinkedHashMap;
3334
import java.util.Map;
@@ -81,7 +82,7 @@ ElementAndAttributePolicies and(ElementAndAttributePolicies p) {
8182
return new ElementAndAttributePolicies(
8283
elementName,
8384
ElementPolicy.Util.join(elPolicy, p.elPolicy),
84-
Map.copyOf(joinedAttrPolicies),
85+
Collections.unmodifiableMap(joinedAttrPolicies),
8586
this.htmlTagSkipType.and(p.htmlTagSkipType));
8687
}
8788

‎src/main/java/org/owasp/html/ElementPolicy.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
package org.owasp.html;
3030

3131
import java.util.ArrayList;
32+
import java.util.Collections;
3233
import java.util.List;
3334
import java.util.Optional;
3435
import java.util.Set;
@@ -133,7 +134,7 @@ final class JoinedElementPolicy implements ElementPolicy {
133134
JoinedElementPolicy(Iterable<? extends ElementPolicy> policies) {
134135
List<ElementPolicy> builder = new ArrayList<>();
135136
policies.forEach(builder::add);
136-
this.policies = List.copyOf(builder);
137+
this.policies = Collections.unmodifiableList(builder);
137138
}
138139

139140
public @Nullable String apply(String elementName, List<String> attrs) {

‎src/main/java/org/owasp/html/FilterUrlByProtocolAttributePolicy.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
package org.owasp.html;
3030

31+
import java.util.Collections;
3132
import java.util.HashSet;
3233
import java.util.Set;
3334

@@ -66,7 +67,7 @@ public FilterUrlByProtocolAttributePolicy(
6667
Iterable<? extends String> protocols) {
6768
Set<String> builder = new HashSet<>();
6869
protocols.forEach(builder::add);
69-
this.protocols = Set.copyOf(builder);
70+
this.protocols = Collections.unmodifiableSet(builder);
7071
}
7172

7273
public @Nullable String apply(

‎src/main/java/org/owasp/html/HtmlElementTables.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.owasp.html;
22

33
import java.util.Arrays;
4+
import java.util.Collections;
45
import java.util.Comparator;
56
import java.util.HashMap;
67
import java.util.List;
@@ -418,7 +419,7 @@ public int getElementNameIndex(String canonName) {
418419
for (int i = 0, n = this.canonNames.size(); i < n; ++i) {
419420
builder.put(this.canonNames.get(i), i);
420421
}
421-
canonNameToIndex = Map.copyOf(builder);
422+
canonNameToIndex = Collections.unmodifiableMap(builder);
422423
this.customElementIndex = canonNames.indexOf(CUSTOM_ELEMENT_NAME);
423424
if (this.customElementIndex < 0) {
424425
throw new IllegalStateException("Negative element index");

‎src/main/java/org/owasp/html/HtmlEntities.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
package org.owasp.html;
3030

31+
import java.util.Collections;
3132
import java.util.HashMap;
3233
import java.util.Map;
3334

@@ -2290,7 +2291,7 @@ final class HtmlEntities {
22902291
}
22912292
}
22922293

2293-
final Map<String, String> entityNameToCodePointMap = Map.copyOf(builder);
2294+
final Map<String, String> entityNameToCodePointMap = Collections.unmodifiableMap(builder);
22942295

22952296
ENTITY_TRIE = new Trie<String>(entityNameToCodePointMap);
22962297
LONGEST_ENTITY_NAME = longestEntityName;

‎src/main/java/org/owasp/html/HtmlPolicyBuilder.java

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
package org.owasp.html;
3030

3131
import java.util.ArrayList;
32+
import java.util.Collections;
3233
import java.util.HashMap;
3334
import java.util.HashSet;
3435
import java.util.LinkedHashMap;
@@ -170,7 +171,7 @@ public class HtmlPolicyBuilder {
170171
for (String elementName : DEFAULT_SKIP_IF_EMPTY) {
171172
builder.put(elementName, HtmlTagSkipType.SKIP_BY_DEFAULT);
172173
}
173-
DEFAULT_SKIP_TAG_MAP_IF_EMPTY_ATTR = Map.copyOf(builder);
174+
DEFAULT_SKIP_TAG_MAP_IF_EMPTY_ATTR = Collections.unmodifiableMap(builder);
174175
}
175176

176177
/**
@@ -347,7 +348,7 @@ public AttributeBuilder allowAttributes(String... attributeNames) {
347348
for (String attributeName : attributeNames) {
348349
builder.add(HtmlLexer.canonicalAttributeName(attributeName));
349350
}
350-
return new AttributeBuilder(List.copyOf(builder));
351+
return new AttributeBuilder(Collections.unmodifiableList(builder));
351352
}
352353

353354
/**
@@ -647,7 +648,7 @@ AttributePolicy makeGuard(AttributeGuardIntermediates intermediates) {
647648
}
648649

649650
});
650-
ATTRIBUTE_GUARDS = Map.copyOf(builder);
651+
ATTRIBUTE_GUARDS = Collections.unmodifiableMap(builder);
651652
}
652653

653654
/**
@@ -686,17 +687,17 @@ public <CTX> HtmlSanitizer.Policy build(
686687
* each backed by a different output channel.
687688
*/
688689
public PolicyFactory toFactory() {
689-
Set<String> textContainerSet = new HashSet<>();
690+
Set<String> textContainerSetBuilder = new HashSet<>();
690691
for (Map.Entry<String, Boolean> textContainer
691692
: this.textContainers.entrySet()) {
692693
if (Boolean.TRUE.equals(textContainer.getValue())) {
693-
textContainerSet.add(textContainer.getKey());
694+
textContainerSetBuilder.add(textContainer.getKey());
694695
}
695696
}
696697
CompiledState compiled = compilePolicies();
697698

698699
return new PolicyFactory(
699-
compiled.compiledPolicies, Set.copyOf(textContainerSet),
700+
compiled.compiledPolicies, Collections.unmodifiableSet(textContainerSetBuilder),
700701
Map.copyOf(compiled.globalAttrPolicies),
701702
preprocessor, postprocessor);
702703
}
@@ -812,7 +813,7 @@ private CompiledState compilePolicies() {
812813
elAttrPolicies = Map.of();
813814
}
814815

815-
Map<String, AttributePolicy> attrs
816+
Map<String, AttributePolicy> attrsBuilder
816817
= new HashMap<>();
817818

818819
for (Map.Entry<String, AttributePolicy> ape : elAttrPolicies.entrySet()) {
@@ -822,7 +823,7 @@ private CompiledState compilePolicies() {
822823
if (globalAttrPolicies.containsKey(attributeName)) { continue; }
823824
AttributePolicy policy = ape.getValue();
824825
if (!AttributePolicy.REJECT_ALL_ATTRIBUTE_POLICY.equals(policy)) {
825-
attrs.put(attributeName, policy);
826+
attrsBuilder.put(attributeName, policy);
826827
}
827828
}
828829
for (Map.Entry<String, AttributePolicy> ape
@@ -831,21 +832,21 @@ private CompiledState compilePolicies() {
831832
AttributePolicy policy = AttributePolicy.Util.join(
832833
elAttrPolicies.get(attributeName), ape.getValue());
833834
if (!AttributePolicy.REJECT_ALL_ATTRIBUTE_POLICY.equals(policy)) {
834-
attrs.put(attributeName, policy);
835+
attrsBuilder.put(attributeName, policy);
835836
}
836837
}
837838

838839
policiesBuilder.put(
839840
elementName,
840841
new ElementAndAttributePolicies(
841842
elementName,
842-
elPolicy, Map.copyOf(attrs),
843+
elPolicy, Collections.unmodifiableMap(attrsBuilder),
843844
getHtmlTagSkipType(elementName)
844845
)
845846
);
846847
}
847848
compiledState = new CompiledState(
848-
globalAttrPolicies, Map.copyOf(policiesBuilder));
849+
globalAttrPolicies, Collections.unmodifiableMap(policiesBuilder));
849850
return compiledState;
850851
}
851852

@@ -982,7 +983,7 @@ public HtmlPolicyBuilder onElements(String... elementNames) {
982983
builder.add(HtmlLexer.canonicalElementName(elementName));
983984
}
984985
return HtmlPolicyBuilder.this.allowAttributesOnElements(
985-
policy, attributeNames, List.copyOf(builder));
986+
policy, attributeNames, Collections.unmodifiableList(builder));
986987
}
987988
}
988989

‎src/main/java/org/owasp/html/PolicyFactory.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
package org.owasp.html;
3030

31+
import java.util.Collections;
3132
import java.util.HashMap;
3233
import java.util.HashSet;
3334
import java.util.Map;
@@ -173,10 +174,10 @@ public PolicyFactory and(PolicyFactory f) {
173174
} else if (f.textContainers.containsAll(this.textContainers)) {
174175
allTextContainers = f.textContainers;
175176
} else {
176-
Set<String> containers = new HashSet<>();
177-
this.textContainers.forEach(containers::add);
178-
f.textContainers.forEach(containers::add);
179-
allTextContainers = Set.copyOf(containers);
177+
Set<String> containerBuilder = new HashSet<>();
178+
this.textContainers.forEach(containerBuilder::add);
179+
f.textContainers.forEach(containerBuilder::add);
180+
allTextContainers = Collections.unmodifiableSet(containerBuilder);
180181
}
181182
Map<String, AttributePolicy> allGlobalAttrPolicies;
182183
if (f.globalAttrPolicies.isEmpty()) {
@@ -200,7 +201,7 @@ public PolicyFactory and(PolicyFactory f) {
200201
ab.put(attrName, e.getValue());
201202
}
202203
}
203-
allGlobalAttrPolicies = Map.copyOf(ab);
204+
allGlobalAttrPolicies = Collections.unmodifiableMap(ab);
204205
}
205206
HtmlStreamEventProcessor compositionOfPreprocessors
206207
= HtmlStreamEventProcessor.Processors.compose(
@@ -209,7 +210,7 @@ public PolicyFactory and(PolicyFactory f) {
209210
= HtmlStreamEventProcessor.Processors.compose(
210211
this.postprocessor, f.postprocessor);
211212
return new PolicyFactory(
212-
Map.copyOf(builder), allTextContainers, allGlobalAttrPolicies,
213+
Collections.unmodifiableMap(builder), allTextContainers, allGlobalAttrPolicies,
213214
compositionOfPreprocessors, compositionOfPostprocessors);
214215
}
215216
}

‎src/test/java/org/owasp/html/SanitizersTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.ArrayList;
3434
import java.util.Arrays;
3535
import java.util.BitSet;
36+
import java.util.Collections;
3637
import java.util.Iterator;
3738
import java.util.List;
3839
import java.util.NoSuchElementException;
@@ -579,7 +580,7 @@ private static class Permutations<T> implements Iterable<List<T>> {
579580
this.k = k;
580581
List<T> builder = new ArrayList<>();
581582
Arrays.stream(elements).forEach(builder::add);
582-
this.elements = List.copyOf(builder);
583+
this.elements = Collections.unmodifiableList(builder);
583584
}
584585

585586
public Iterator<List<T>> iterator() {

0 commit comments

Comments
 (0)