You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2022/10/19 09:22:01 UTC
[GitHub] [sling-org-apache-sling-xss] nonanalou opened a new pull request, #30: Add tests for the dynamic and global attribute
nonanalou opened a new pull request, #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30
* Correct the policy adapter so that the conditions are added with an "or" instead of an "and".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] rombert merged pull request #30: SLING-11592 - Fix policy adapter condition chaining and add tests
Posted by GitBox <gi...@apache.org>.
rombert merged PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] nonanalou commented on a diff in pull request #30: SLING-11592 - Fix policy adapter condition chaining and add tests
Posted by GitBox <gi...@apache.org>.
nonanalou commented on code in PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#discussion_r1000558760
##########
src/main/java/org/apache/sling/xss/impl/AntiSamyPolicyAdapter.java:
##########
@@ -222,43 +224,28 @@ public boolean apply(String s) {
private static Predicate<String> matchesPatternsOrLiterals(List<Pattern> patternList, boolean ignoreCase, List<String> literalList) {
return new Predicate<String>() {
public boolean apply(String s) {
- // check if the string matches to the pattern
- for (Pattern pattern : patternList) {
- if (pattern.matcher(s).matches()) {
- return true;
- }
- }
- // if the pattern does not match it goes through the literals
- for (String string : literalList) {
- s = ignoreCase
- ? s.toLowerCase()
- : s;
- if (string.equals(s)) {
- return true;
- }
- }
- // if it neither matches the patterns nor the literals it returns false
- return false;
+ // check if the string matches to the pattern or one of the literal
+ s = ignoreCase ? s.toLowerCase() : s;
Review Comment:
Resolved: [9f0dbc5](https://github.com/apache/sling-org-apache-sling-xss/pull/30/commits/9f0dbc5a30a90a3949c0dcd7f2bc3b2bac41b420)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] sonarcloud[bot] commented on pull request #30: SLING-11592 - Fix policy adapter condition chaining and add tests
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#issuecomment-1289184786
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-xss&pullRequest=30)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_coverage&view=list) [77.5% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] sonarcloud[bot] commented on pull request #30: Add tests for the dynamic and global attribute
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#issuecomment-1283894539
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-xss&pullRequest=30)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_coverage&view=list) [74.1% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] rombert commented on a diff in pull request #30: SLING-11592 - Fix policy adapter condition chaining and add tests
Posted by GitBox <gi...@apache.org>.
rombert commented on code in PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#discussion_r1000827100
##########
src/main/java/org/apache/sling/xss/impl/xml/AntiSamyRules.java:
##########
@@ -125,3 +126,10 @@ public Map<String, Pattern> getCommonPatternByName() {
.collect(Collectors.toMap(Regexp::getName, Regexp::getPattern));
}
}
+
+/**
+ * Attribute, Property and Tag names use the English locale, which may differ from the original config
+ */
+class AntiSamyConfigLocale {
Review Comment:
I find non-nested classes in the same file confusing, maybe make it a static inner class?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] rombert commented on pull request #30: Add tests for the dynamic and global attribute
Posted by GitBox <gi...@apache.org>.
rombert commented on PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#issuecomment-1283913227
> > @kwin - is this failure related to your recent changes? https://ci-builds.apache.org/blue/organizations/jenkins/Sling%2Fmodules%2Fsling-org-apache-sling-xss/detail/PR-30/1/pipeline fails with
> > ```
> > + mvn -U -B -e clean compile
> > ----- withMaven Wrapper script -----
> > The JAVA_HOME environment variable is not defined correctly,
> > this environment variable is needed to run this program.
> > script returned exit code 1
> > ```
>
> @rombert Thanks for the pointer, I accidentally removed a variable specifying the default node label. That made ASF Jenkins run certain stages on nodes not having Java installed. I fixed it (hopefully) with [apache/sling-tooling-jenkins@849fe59](https://github.com/apache/sling-tooling-jenkins/commit/849fe59626774cb8f8a336ec18ef04d4461738d7).
Looks good now, thanks @kwin!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] nonanalou commented on a diff in pull request #30: SLING-11592 - Fix policy adapter condition chaining and add tests
Posted by GitBox <gi...@apache.org>.
nonanalou commented on code in PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#discussion_r1003046153
##########
src/main/java/org/apache/sling/xss/impl/xml/AntiSamyRules.java:
##########
@@ -125,3 +126,10 @@ public Map<String, Pattern> getCommonPatternByName() {
.collect(Collectors.toMap(Regexp::getName, Regexp::getPattern));
}
}
+
+/**
+ * Attribute, Property and Tag names use the English locale, which may differ from the original config
+ */
+class AntiSamyConfigLocale {
Review Comment:
Resolved: [e763eef](https://github.com/apache/sling-org-apache-sling-xss/pull/30/commits/e763eef2cab9c2b3a43de53c2747aad62e7e62d6)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] nonanalou commented on a diff in pull request #30: SLING-11592 - Fix policy adapter condition chaining and add tests
Posted by GitBox <gi...@apache.org>.
nonanalou commented on code in PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#discussion_r1000559047
##########
src/main/java/org/apache/sling/xss/impl/AntiSamyPolicyAdapter.java:
##########
@@ -222,43 +224,28 @@ public boolean apply(String s) {
private static Predicate<String> matchesPatternsOrLiterals(List<Pattern> patternList, boolean ignoreCase, List<String> literalList) {
return new Predicate<String>() {
public boolean apply(String s) {
- // check if the string matches to the pattern
- for (Pattern pattern : patternList) {
- if (pattern.matcher(s).matches()) {
- return true;
- }
- }
- // if the pattern does not match it goes through the literals
- for (String string : literalList) {
- s = ignoreCase
- ? s.toLowerCase()
- : s;
- if (string.equals(s)) {
- return true;
- }
- }
- // if it neither matches the patterns nor the literals it returns false
- return false;
+ // check if the string matches to the pattern or one of the literal
+ s = ignoreCase ? s.toLowerCase() : s;
+ return matchesToPatterns(patternList).apply(s) || literalList.contains(s);
}
};
}
- public AttributePolicy newDynamicAttributePolicy(final Pattern pattern) {
+ public AttributePolicy newDynamicAttributePolicy(final List<Pattern> patternList, final boolean ignoreCase, final List<String> literalList) {
return new AttributePolicy() {
@Override
public @Nullable String apply(String elementName, String attributeName, String value) {
- return pattern.matcher(value).matches() ? value : null;
- }
- };
- }
+ if (!literalList.isEmpty() && !patternList.isEmpty()) {
+ return matchesPatternsOrLiterals(patternList,ignoreCase, literalList).apply(value) ? value : null;
- public AttributePolicy newDynamicAttributePolicy(boolean ignoreCase, String... allowedValues) {
- final List<String> allowed = Arrays.asList(allowedValues);
- return new AttributePolicy() {
- @Override
- public @Nullable String apply(String elementName, String attributeName, String uncanonValue) {
- String value = ignoreCase ? uncanonValue.toLowerCase() : uncanonValue;
- return allowed.contains(value) ? value : null;
+ } else if (!literalList.isEmpty()) {
+ value = ignoreCase ? value.toLowerCase() : value;
Review Comment:
Resolved: [9f0dbc5](https://github.com/apache/sling-org-apache-sling-xss/pull/30/commits/9f0dbc5a30a90a3949c0dcd7f2bc3b2bac41b420)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] kwin commented on pull request #30: Add tests for the dynamic and global attribute
Posted by GitBox <gi...@apache.org>.
kwin commented on PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#issuecomment-1283877213
> @kwin - is this failure related to your recent changes? https://ci-builds.apache.org/blue/organizations/jenkins/Sling%2Fmodules%2Fsling-org-apache-sling-xss/detail/PR-30/1/pipeline fails with
>
> ```
> + mvn -U -B -e clean compile
> ----- withMaven Wrapper script -----
> The JAVA_HOME environment variable is not defined correctly,
> this environment variable is needed to run this program.
> script returned exit code 1
> ```
@rombert Thanks for the pointer, I accidentally removed a variable specifying the default node label. That made ASF Jenkins run certain stages on nodes not having Java installed. I fixed it (hopefully) with https://github.com/apache/sling-tooling-jenkins/commit/849fe59626774cb8f8a336ec18ef04d4461738d7.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] sonarcloud[bot] commented on pull request #30: SLING-11592 - Fix policy adapter condition chaining and add tests
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#issuecomment-1285466183
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-xss&pullRequest=30)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL) [16 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_coverage&view=list) [77.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] rombert commented on pull request #30: Add tests for the dynamic and global attribute
Posted by GitBox <gi...@apache.org>.
rombert commented on PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#issuecomment-1283703470
@kwin - is this failure related to your recent changes? https://ci-builds.apache.org/blue/organizations/jenkins/Sling%2Fmodules%2Fsling-org-apache-sling-xss/detail/PR-30/1/pipeline fails with
```
+ mvn -U -B -e clean compile
----- withMaven Wrapper script -----
The JAVA_HOME environment variable is not defined correctly,
this environment variable is needed to run this program.
script returned exit code 1
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] nonanalou commented on a diff in pull request #30: SLING-11592 - Fix policy adapter condition chaining and add tests
Posted by GitBox <gi...@apache.org>.
nonanalou commented on code in PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#discussion_r1000559957
##########
src/test/java/org/apache/sling/xss/impl/AntiSamyPolicyWithTestConfigTest.java:
##########
@@ -0,0 +1,146 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
+package org.apache.sling.xss.impl;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+import java.util.regex.Pattern;
+
+import org.apache.sling.xss.impl.xml.AntiSamyPolicy;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class AntiSamyPolicyWithTestConfigTest {
+
+ public static final String POLICY_FILE = "./testConfig.xml";
Review Comment:
Resolved: [9f0dbc5](https://github.com/apache/sling-org-apache-sling-xss/pull/30/commits/9f0dbc5a30a90a3949c0dcd7f2bc3b2bac41b420)
##########
src/test/java/org/apache/sling/xss/impl/AntiSamyPolicyWithTestConfigTest.java:
##########
@@ -0,0 +1,146 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
+package org.apache.sling.xss.impl;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+import java.util.regex.Pattern;
+
+import org.apache.sling.xss.impl.xml.AntiSamyPolicy;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class AntiSamyPolicyWithTestConfigTest {
+
+ public static final String POLICY_FILE = "./testConfig.xml";
+ private static HtmlSanitizer antiSamy;
+
+ @BeforeAll
+ public static void setup() throws InvalidConfigException, XMLStreamException, IOException {
+ antiSamy = new HtmlSanitizer(new AntiSamyPolicy(AntiSamyPolicyWithTestConfigTest.class.getClassLoader().getResourceAsStream(POLICY_FILE)));
+ }
+
+ @ParameterizedTest
+ @MethodSource("dataForDynamicAttributes")
+ public void testDynamicAttributes(TestInput testInput) throws Exception {
+ testInput.skipComparingInputWithOutput = false;
+ testInput.runCheck();
+ }
+
+ @ParameterizedTest
+ @MethodSource("dataForGlobalAttributes")
+ public void testGlobalAttributes(TestInput testInput) throws Exception {
+ testInput.skipComparingInputWithOutput = false;
+ testInput.runCheck();
+ }
+
+
+ static TestInput[] dataForGlobalAttributes() {
+ return new TestInput[]{
+ new TestInput("<p draggable=\"wrong\">This is a paragraph.</p>",
+ "<p",false),
+ new TestInput("<p draggable=\"auto\">This is a paragraph.</p>",
+ "<p", true),
+ new TestInput("<p draggable=\"true\">This is a draggable paragraph.</p>",
+ "<p", true),
+ new TestInput("<p contenteditable=\"wrong\">This is a paragraph.</p>",
+ "<p",false),
+ new TestInput("<p contenteditable=\"true\">This is a paragraph.</p>",
+ "<p", true),
+ };
+ }
+
+ static TestInput[] dataForDynamicAttributes() {
+ return new TestInput[]{
+ new TestInput("<p data-test=\"test-purpose\">This is a paragraph.</p>",
+ "<p data-test=\"test-purpose\">This is a paragraph.</p>", true),
+ new TestInput("<p data-test=\"test\">This is a paragraph.</p>",
+ "<p data-test=\"test\">This is a paragraph.</p>", true),
+ };
+ }
+
+ private static class TestInput {
Review Comment:
Resolved: [9f0dbc5](https://github.com/apache/sling-org-apache-sling-xss/pull/30/commits/9f0dbc5a30a90a3949c0dcd7f2bc3b2bac41b420)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] rombert commented on a diff in pull request #30: Add tests for the dynamic and global attribute
Posted by GitBox <gi...@apache.org>.
rombert commented on code in PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#discussion_r999362695
##########
src/test/java/org/apache/sling/xss/impl/AntiSamyPolicyWithTestConfigTest.java:
##########
@@ -0,0 +1,146 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
+package org.apache.sling.xss.impl;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+import java.util.regex.Pattern;
+
+import org.apache.sling.xss.impl.xml.AntiSamyPolicy;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class AntiSamyPolicyWithTestConfigTest {
+
+ public static final String POLICY_FILE = "./testConfig.xml";
Review Comment:
Please set a self-explanatory name , e.g. `configWithFeatureFoo.xml` so it's immediately clear what it contains.
##########
src/main/java/org/apache/sling/xss/impl/AntiSamyPolicyAdapter.java:
##########
@@ -222,43 +224,28 @@ public boolean apply(String s) {
private static Predicate<String> matchesPatternsOrLiterals(List<Pattern> patternList, boolean ignoreCase, List<String> literalList) {
return new Predicate<String>() {
public boolean apply(String s) {
- // check if the string matches to the pattern
- for (Pattern pattern : patternList) {
- if (pattern.matcher(s).matches()) {
- return true;
- }
- }
- // if the pattern does not match it goes through the literals
- for (String string : literalList) {
- s = ignoreCase
- ? s.toLowerCase()
- : s;
- if (string.equals(s)) {
- return true;
- }
- }
- // if it neither matches the patterns nor the literals it returns false
- return false;
+ // check if the string matches to the pattern or one of the literal
+ s = ignoreCase ? s.toLowerCase() : s;
+ return matchesToPatterns(patternList).apply(s) || literalList.contains(s);
}
};
}
- public AttributePolicy newDynamicAttributePolicy(final Pattern pattern) {
+ public AttributePolicy newDynamicAttributePolicy(final List<Pattern> patternList, final boolean ignoreCase, final List<String> literalList) {
return new AttributePolicy() {
@Override
public @Nullable String apply(String elementName, String attributeName, String value) {
- return pattern.matcher(value).matches() ? value : null;
- }
- };
- }
+ if (!literalList.isEmpty() && !patternList.isEmpty()) {
+ return matchesPatternsOrLiterals(patternList,ignoreCase, literalList).apply(value) ? value : null;
- public AttributePolicy newDynamicAttributePolicy(boolean ignoreCase, String... allowedValues) {
- final List<String> allowed = Arrays.asList(allowedValues);
- return new AttributePolicy() {
- @Override
- public @Nullable String apply(String elementName, String attributeName, String uncanonValue) {
- String value = ignoreCase ? uncanonValue.toLowerCase() : uncanonValue;
- return allowed.contains(value) ? value : null;
+ } else if (!literalList.isEmpty()) {
+ value = ignoreCase ? value.toLowerCase() : value;
Review Comment:
`String.toLowerCase` without an explicit locale can be dangerous, see https://stackoverflow.com/questions/11063102/using-locales-with-javas-tolowercase-and-touppercase for some discussion. Please figure out whether we can run in the default locale or need to use something like english.
##########
src/test/java/org/apache/sling/xss/impl/AntiSamyPolicyWithTestConfigTest.java:
##########
@@ -0,0 +1,146 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
+package org.apache.sling.xss.impl;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+import java.util.regex.Pattern;
+
+import org.apache.sling.xss.impl.xml.AntiSamyPolicy;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class AntiSamyPolicyWithTestConfigTest {
Review Comment:
Please set a self-explanatory name , e.g. `AntiSamyPolicyWithFeatureFooTest` so it's immediately clear what this test.
##########
src/test/java/org/apache/sling/xss/impl/AntiSamyPolicyWithTestConfigTest.java:
##########
@@ -0,0 +1,146 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
+package org.apache.sling.xss.impl;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+import java.util.regex.Pattern;
+
+import org.apache.sling.xss.impl.xml.AntiSamyPolicy;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class AntiSamyPolicyWithTestConfigTest {
+
+ public static final String POLICY_FILE = "./testConfig.xml";
+ private static HtmlSanitizer antiSamy;
+
+ @BeforeAll
+ public static void setup() throws InvalidConfigException, XMLStreamException, IOException {
+ antiSamy = new HtmlSanitizer(new AntiSamyPolicy(AntiSamyPolicyWithTestConfigTest.class.getClassLoader().getResourceAsStream(POLICY_FILE)));
+ }
+
+ @ParameterizedTest
+ @MethodSource("dataForDynamicAttributes")
+ public void testDynamicAttributes(TestInput testInput) throws Exception {
+ testInput.skipComparingInputWithOutput = false;
+ testInput.runCheck();
+ }
+
+ @ParameterizedTest
+ @MethodSource("dataForGlobalAttributes")
+ public void testGlobalAttributes(TestInput testInput) throws Exception {
+ testInput.skipComparingInputWithOutput = false;
+ testInput.runCheck();
+ }
+
+
+ static TestInput[] dataForGlobalAttributes() {
+ return new TestInput[]{
+ new TestInput("<p draggable=\"wrong\">This is a paragraph.</p>",
+ "<p",false),
+ new TestInput("<p draggable=\"auto\">This is a paragraph.</p>",
+ "<p", true),
+ new TestInput("<p draggable=\"true\">This is a draggable paragraph.</p>",
+ "<p", true),
+ new TestInput("<p contenteditable=\"wrong\">This is a paragraph.</p>",
+ "<p",false),
+ new TestInput("<p contenteditable=\"true\">This is a paragraph.</p>",
+ "<p", true),
+ };
+ }
+
+ static TestInput[] dataForDynamicAttributes() {
+ return new TestInput[]{
+ new TestInput("<p data-test=\"test-purpose\">This is a paragraph.</p>",
+ "<p data-test=\"test-purpose\">This is a paragraph.</p>", true),
+ new TestInput("<p data-test=\"test\">This is a paragraph.</p>",
+ "<p data-test=\"test\">This is a paragraph.</p>", true),
+ };
+ }
+
+ private static class TestInput {
Review Comment:
I think this TestInput class already exists in the code, right? Please extract it to a shared class so it can be reused instead of duplicated.
##########
src/main/java/org/apache/sling/xss/impl/AntiSamyPolicyAdapter.java:
##########
@@ -222,43 +224,28 @@ public boolean apply(String s) {
private static Predicate<String> matchesPatternsOrLiterals(List<Pattern> patternList, boolean ignoreCase, List<String> literalList) {
return new Predicate<String>() {
public boolean apply(String s) {
- // check if the string matches to the pattern
- for (Pattern pattern : patternList) {
- if (pattern.matcher(s).matches()) {
- return true;
- }
- }
- // if the pattern does not match it goes through the literals
- for (String string : literalList) {
- s = ignoreCase
- ? s.toLowerCase()
- : s;
- if (string.equals(s)) {
- return true;
- }
- }
- // if it neither matches the patterns nor the literals it returns false
- return false;
+ // check if the string matches to the pattern or one of the literal
+ s = ignoreCase ? s.toLowerCase() : s;
Review Comment:
`String.toLowerCase` without an explicit locale can be dangerous, see https://stackoverflow.com/questions/11063102/using-locales-with-javas-tolowercase-and-touppercase for some discussion. Please figure out whether we can run in the default locale or need to use something like english.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] rombert commented on pull request #30: Add tests for the dynamic and global attribute
Posted by GitBox <gi...@apache.org>.
rombert commented on PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#issuecomment-1283704781
@nonanalou - while we figure out the CI issues, can you please reference a Jira issue in:
- the PR summary
- commit message
?
`SLING-XXXX - Fix problem X` is the pattern we prefer.
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] nonanalou commented on a diff in pull request #30: SLING-11592 - Fix policy adapter condition chaining and add tests
Posted by GitBox <gi...@apache.org>.
nonanalou commented on code in PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#discussion_r1000559727
##########
src/test/java/org/apache/sling/xss/impl/AntiSamyPolicyWithTestConfigTest.java:
##########
@@ -0,0 +1,146 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
+package org.apache.sling.xss.impl;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import javax.xml.stream.XMLStreamException;
+
+import java.io.IOException;
+import java.util.regex.Pattern;
+
+import org.apache.sling.xss.impl.xml.AntiSamyPolicy;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class AntiSamyPolicyWithTestConfigTest {
Review Comment:
Resolved: [9f0dbc5](https://github.com/apache/sling-org-apache-sling-xss/pull/30/commits/9f0dbc5a30a90a3949c0dcd7f2bc3b2bac41b420)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-xss] sonarcloud[bot] commented on pull request #30: SLING-11592 - Fix policy adapter condition chaining and add tests
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #30:
URL: https://github.com/apache/sling-org-apache-sling-xss/pull/30#issuecomment-1288669823
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-xss&pullRequest=30)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL) [16 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-xss&pullRequest=30&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_coverage&view=list) [77.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-xss&pullRequest=30&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org