diff --git a/.github/workflows/scorecards-analysis.yaml b/.github/workflows/scorecards-analysis.yaml index 6181e80de..e5ce47978 100644 --- a/.github/workflows/scorecards-analysis.yaml +++ b/.github/workflows/scorecards-analysis.yaml @@ -58,7 +58,7 @@ jobs: publish_results: true - name: "Upload artifact" - uses: actions/upload-artifact@1eb3cb2b3e0f29609092a73eb033bb759a334595 # 4.1.0 + uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8 # 4.3.0 with: name: SARIF file path: results.sarif diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 9198cfa1a..3fee1e1e3 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -32,6 +32,7 @@ jobs: sonarcloud: name: Scan runs-on: ubuntu-latest + if: ${{ !github.event.pull_request.head.repo.fork }} steps: - uses: actions/checkout@v4 with: diff --git a/Jenkinsfile b/Jenkinsfile index ca830acf2..45cd77441 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -67,7 +67,7 @@ pipeline { MAVEN_OPTS = "-Xmx1024m" } stages { - stage('Test') { + stage('Test & Coverage') { steps { sh './mvnw -B verify -Pcoverage -DskipAssembly --no-transfer-progress' } @@ -86,7 +86,7 @@ pipeline { } steps { withCredentials([string(credentialsId: 'asf-struts-sonarcloud', variable: 'SONARCLOUD_TOKEN')]) { - sh './mvnw -B -Pcoverage -DskipAssembly -Dsonar.login=${SONARCLOUD_TOKEN} verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar' + sh './mvnw -B -Pcoverage -DskipAssembly -Dsonar.login=${SONARCLOUD_TOKEN} verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar --no-transfer-progress' } } } @@ -100,7 +100,7 @@ pipeline { dir("local-snapshots-dir/") { deleteDir() } - sh './mvnw -B source:jar javadoc:jar -DskipTests -DskipAssembly' + sh './mvnw -B source:jar javadoc:jar -DskipTests -DskipAssembly --no-transfer-progress' } } stage('Deploy Snapshot') { @@ -111,7 +111,7 @@ pipeline { } steps { withCredentials([file(credentialsId: 'lukaszlenart-repository-access-token', variable: 'CUSTOM_SETTINGS')]) { - sh './mvnw -s \${CUSTOM_SETTINGS} deploy -DskipTests -DskipAssembly' + sh './mvnw -s \${CUSTOM_SETTINGS} deploy -DskipTests -DskipAssembly --no-transfer-progress' } } } @@ -123,7 +123,7 @@ pipeline { } } steps { - sh './mvnw -B package -DskipTests' + sh './mvnw -B package -DskipTests --no-transfer-progress' sshPublisher(publishers: [ sshPublisherDesc( configName: 'Nightlies', diff --git a/SECURITY.md b/SECURITY.md index eca65f01b..7908f9a34 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -2,13 +2,13 @@ ## Supported Versions -Please vist the [Releases](https://struts.apache.org/releases.html#prior-releases) page to see full information about each version +Please visit the [Releases](https://struts.apache.org/releases.html#prior-releases) page to see full information about each version and what potential vulnerability it can have: | Version | Supported | -| ------- | ------------------ | -| 6.0.0 | :white_check_mark: | -| 2.5.30 | :white_check_mark: | +|---------|--------------------| +| 6.x | :white_check_mark: | +| 2.5.x | :white_check_mark: | ## Reporting New Security Issues with thr Apache Struts @@ -28,8 +28,8 @@ All mail sent to this address that does not relate to security problems in the A ``` Note that all networked servers are subject to denial of service attacks, and we cannot promise magic -workarounds to generic problems (such as a client streaming lots of data to your server, or re-requesting -the same URL repeatedly). In general our philosophy is to avoid any attacks which can cause the server +workarounds to generic problems (such as a client streaming lots of data to your server, or requesting +the same URL repeatedly). In general, our philosophy is to avoid any attacks that can cause the server to consume resources in a non-linear relationship to the size of inputs. The mailing address is: [security@struts.apache.org](mailto:security@struts.apache.org) diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/UITagExample.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/UITagExample.java index ca5aecbb7..1e87b2193 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/UITagExample.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/UITagExample.java @@ -24,9 +24,15 @@ import com.opensymphony.xwork2.ActionSupport; import com.opensymphony.xwork2.Validateable; import com.opensymphony.xwork2.util.ValueStack; import org.apache.struts2.ServletActionContext; +import org.apache.struts2.interceptor.parameter.StrutsParameter; import java.io.File; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; /** */ @@ -89,6 +95,7 @@ public class UITagExample extends ActionSupport implements Validateable { return leftSideCartoonCharacters; } + @StrutsParameter public void setLeftSideCartoonCharacters(List leftSideCartoonCharacters) { this.leftSideCartoonCharacters = leftSideCartoonCharacters; } @@ -98,6 +105,7 @@ public class UITagExample extends ActionSupport implements Validateable { return rightSideCartoonCharacters; } + @StrutsParameter public void setRightSideCartoonCharacters(List rightSideCartoonCharacters) { this.rightSideCartoonCharacters = rightSideCartoonCharacters; } @@ -107,6 +115,7 @@ public class UITagExample extends ActionSupport implements Validateable { return favouriteVehicalType; } + @StrutsParameter public void setFavouriteVehicalType(String favouriteVehicalType) { this.favouriteVehicalType = favouriteVehicalType; } @@ -115,6 +124,7 @@ public class UITagExample extends ActionSupport implements Validateable { return favouriteVehicalSpecific; } + @StrutsParameter public void setFavouriteVehicalSpecific(String favouriteVehicalSpecific) { this.favouriteVehicalSpecific = favouriteVehicalSpecific; } @@ -145,6 +155,7 @@ public class UITagExample extends ActionSupport implements Validateable { return name; } + @StrutsParameter public void setName(String name) { this.name = name; } @@ -153,6 +164,7 @@ public class UITagExample extends ActionSupport implements Validateable { return birthday; } + @StrutsParameter public void setBirthday(Date birthday) { this.birthday = birthday; } @@ -161,6 +173,7 @@ public class UITagExample extends ActionSupport implements Validateable { return bio; } + @StrutsParameter public void setBio(String bio) { this.bio = bio; } @@ -169,6 +182,7 @@ public class UITagExample extends ActionSupport implements Validateable { return favouriteColor; } + @StrutsParameter public void setFavouriteColor(String favoriteColor) { this.favouriteColor = favoriteColor; } @@ -177,6 +191,7 @@ public class UITagExample extends ActionSupport implements Validateable { return friends; } + @StrutsParameter public void setFriends(List friends) { this.friends = friends; } @@ -193,6 +208,7 @@ public class UITagExample extends ActionSupport implements Validateable { return legalAge; } + @StrutsParameter public void setLegalAge(boolean legalAge) { this.legalAge = legalAge; } @@ -201,6 +217,7 @@ public class UITagExample extends ActionSupport implements Validateable { return state; } + @StrutsParameter public void setState(String state) { this.state = state; } @@ -209,6 +226,7 @@ public class UITagExample extends ActionSupport implements Validateable { return region; } + @StrutsParameter public void setRegion(String region) { this.region = region; } @@ -229,6 +247,7 @@ public class UITagExample extends ActionSupport implements Validateable { this.pictureFileName = pictureFileName; } + @StrutsParameter public void setFavouriteLanguage(String favouriteLanguage) { this.favouriteLanguage = favouriteLanguage; } @@ -237,7 +256,7 @@ public class UITagExample extends ActionSupport implements Validateable { return favouriteLanguage; } - + @StrutsParameter public void setThoughts(String thoughts) { this.thoughts = thoughts; } @@ -250,6 +269,7 @@ public class UITagExample extends ActionSupport implements Validateable { return wakeup; } + @StrutsParameter public void setWakeup(Date wakeup) { this.wakeup = wakeup; } diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/action/ParamsAnnotationAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/action/ParamsAnnotationAction.java new file mode 100644 index 000000000..0c3ff4f7c --- /dev/null +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/action/ParamsAnnotationAction.java @@ -0,0 +1,133 @@ +/* + * 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.struts2.showcase.action; + +import com.opensymphony.xwork2.ActionSupport; +import org.apache.struts2.interceptor.parameter.StrutsParameter; +import org.apache.struts2.showcase.model.MyDto; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; + +/** + * This class supports {@link com.atlassian.confluence.stateless.webdriver.selenium3.security.StrutsParametersTest} + * which prevents critical security regressions. Do NOT modify without understanding the motivation behind the tests and + * the implications of any changes. + */ +public class ParamsAnnotationAction extends ActionSupport { + + @StrutsParameter + public String varToPrint; + + public String publicField = "no"; + + @StrutsParameter + public String publicFieldAnnotated = "no"; + + private String privateField = "no"; + + public int[] publicArray = new int[]{0}; + + @StrutsParameter(depth = 1) + public int[] publicArrayAnnotated = new int[]{0}; + + public List publicList = new ArrayList<>(singletonList("no")); + + @StrutsParameter(depth = 1) + public List publicListAnnotated = new ArrayList<>(singletonList("no")); + + private List privateList = new ArrayList<>(singletonList("no")); + + public Map publicMap = new HashMap<>(singletonMap("key", "no")); + + @StrutsParameter(depth = 1) + public Map publicMapAnnotated = new HashMap<>(singletonMap("key", "no")); + + public MyDto publicMyDto = new MyDto(); + + @StrutsParameter(depth = 2) + public MyDto publicMyDtoAnnotated = new MyDto(); + + @StrutsParameter(depth = 1) + public MyDto publicMyDtoAnnotatedDepthOne = new MyDto(); + + private MyDto privateMyDto = new MyDto(); + + public void setPrivateFieldMethod(String privateField) { + this.privateField = privateField; + } + + @StrutsParameter + public void setPrivateFieldMethodAnnotated(String privateField) { + this.privateField = privateField; + } + + public List getPrivateListMethod() { + return privateList; + } + + @StrutsParameter(depth = 1) + public List getPrivateListMethodAnnotated() { + return privateList; + } + + public MyDto getUnsafeMethodMyDto() { + return privateMyDto; + } + + @StrutsParameter(depth = 2) + public MyDto getSafeMethodMyDto() { + return privateMyDto; + } + + @StrutsParameter(depth = 1) + public MyDto getSafeMethodMyDtoDepthOne() { + return privateMyDto; + } + + public String renderVarToPrint() throws ReflectiveOperationException { + if (varToPrint == null) { + return "null"; + } + Field field = this.getClass().getDeclaredField(varToPrint); + field.setAccessible(true); + try { + return String.format("%s{%s}", varToPrint, + field.getType().isArray() ? stringifyArray(field.get(this)) : field.get(this)); + } finally { + field.setAccessible(false); + } + } + + private String stringifyArray(Object array) { + switch (array.getClass().getComponentType().getName()) { + case "int": + return Arrays.toString((int[]) array); + default: + return "TODO"; + } + } +} diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/action/SkillAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/action/SkillAction.java index 1c96d20dc..6ba209691 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/action/SkillAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/action/SkillAction.java @@ -21,6 +21,7 @@ package org.apache.struts2.showcase.action; import com.opensymphony.xwork2.Preparable; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.struts2.interceptor.parameter.StrutsParameter; import org.apache.struts2.showcase.dao.Dao; import org.apache.struts2.showcase.dao.SkillDao; import org.apache.struts2.showcase.model.Skill; @@ -71,6 +72,7 @@ public class SkillAction extends AbstractCRUDAction implements Preparable { return skillDao; } + @StrutsParameter(depth = 1) public Skill getCurrentSkill() { return currentSkill; } diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/async/ChatRoomAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/async/ChatRoomAction.java index 5877fe6e1..67b27e3b6 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/async/ChatRoomAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/async/ChatRoomAction.java @@ -19,6 +19,7 @@ package org.apache.struts2.showcase.async; import com.opensymphony.xwork2.ActionSupport; +import org.apache.struts2.interceptor.parameter.StrutsParameter; import java.util.ArrayList; import java.util.List; @@ -34,10 +35,12 @@ public class ChatRoomAction extends ActionSupport { private static final List messages = new ArrayList<>(); + @StrutsParameter public void setMessage(String message) { this.message = message; } + @StrutsParameter public void setLastIndex(Integer lastIndex) { this.lastIndex = lastIndex; } diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/AddressAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/AddressAction.java index 66e9c2746..0f764b787 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/AddressAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/AddressAction.java @@ -21,6 +21,7 @@ package org.apache.struts2.showcase.conversion; import com.opensymphony.xwork2.ActionSupport; +import org.apache.struts2.interceptor.parameter.StrutsParameter; import java.util.LinkedHashSet; import java.util.Set; @@ -30,7 +31,7 @@ import java.util.Set; */ public class AddressAction extends ActionSupport { - private Set
addresses = new LinkedHashSet
(); + private Set
addresses = new LinkedHashSet<>(); public String input() throws Exception { return SUCCESS; @@ -41,6 +42,7 @@ public class AddressAction extends ActionSupport { return SUCCESS; } + @StrutsParameter(depth = 2) public Set
getAddresses() { return addresses; } diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/OperationsEnumAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/OperationsEnumAction.java index a8f8cd624..272c8b6cc 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/OperationsEnumAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/OperationsEnumAction.java @@ -21,6 +21,7 @@ package org.apache.struts2.showcase.conversion; import com.opensymphony.xwork2.ActionSupport; +import org.apache.struts2.interceptor.parameter.StrutsParameter; import java.util.Arrays; import java.util.LinkedList; @@ -47,6 +48,7 @@ public class OperationsEnumAction extends ActionSupport { return this.selectedOperations; } + @StrutsParameter public void setSelectedOperations(List selectedOperations) { this.selectedOperations = selectedOperations; } diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/PersonAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/PersonAction.java index 70307db80..27df30a97 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/PersonAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/conversion/PersonAction.java @@ -21,6 +21,7 @@ package org.apache.struts2.showcase.conversion; import com.opensymphony.xwork2.ActionSupport; +import org.apache.struts2.interceptor.parameter.StrutsParameter; import java.util.List; @@ -36,6 +37,7 @@ public class PersonAction extends ActionSupport { return SUCCESS; } + @StrutsParameter(depth = 2) public List getPersons() { return persons; } diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/filedownload/FileDownloadAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/filedownload/FileDownloadAction.java index c9fab7f46..5f23c19d7 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/filedownload/FileDownloadAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/filedownload/FileDownloadAction.java @@ -22,6 +22,7 @@ package org.apache.struts2.showcase.filedownload; import com.opensymphony.xwork2.Action; import org.apache.struts2.ServletActionContext; +import org.apache.struts2.interceptor.parameter.StrutsParameter; import java.io.InputStream; @@ -38,6 +39,7 @@ public class FileDownloadAction implements Action { return SUCCESS; } + @StrutsParameter public void setInputPath(String value) { inputPath = sanitizeInputPath(value); } diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java index be6f38669..a1381c08c 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java @@ -23,6 +23,7 @@ package org.apache.struts2.showcase.fileupload; import com.opensymphony.xwork2.ActionSupport; import org.apache.struts2.action.UploadedFilesAware; import org.apache.struts2.dispatcher.multipart.UploadedFile; +import org.apache.struts2.interceptor.parameter.StrutsParameter; import java.io.File; import java.util.List; @@ -66,6 +67,7 @@ public class FileUploadAction extends ActionSupport implements UploadedFilesAwar return caption; } + @StrutsParameter public void setCaption(String caption) { this.caption = caption; } diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/model/MyDto.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/model/MyDto.java new file mode 100644 index 000000000..7abee847e --- /dev/null +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/model/MyDto.java @@ -0,0 +1,38 @@ +/* + * 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.struts2.showcase.model; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; + +import static java.util.Collections.singletonMap; + +public class MyDto { + + public String str = "no"; + + public Map map = new HashMap<>(singletonMap("key", "no")); + public int[] array = new int[]{0}; + + @Override + public String toString() { + return "str=" + str + ", map=" + map + ", array=" + Arrays.toString(array); + } +} diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/validation/FieldValidatorsExampleAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/validation/FieldValidatorsExampleAction.java index a9e02deab..9639bacd1 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/validation/FieldValidatorsExampleAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/validation/FieldValidatorsExampleAction.java @@ -20,6 +20,8 @@ */ package org.apache.struts2.showcase.validation; +import org.apache.struts2.interceptor.parameter.StrutsParameter; + import java.sql.Date; /** @@ -44,6 +46,7 @@ public class FieldValidatorsExampleAction extends AbstractValidationActionSuppor return dateValidatorField; } + @StrutsParameter public void setDateValidatorField(Date dateValidatorField) { this.dateValidatorField = dateValidatorField; } @@ -52,6 +55,7 @@ public class FieldValidatorsExampleAction extends AbstractValidationActionSuppor return emailValidatorField; } + @StrutsParameter public void setEmailValidatorField(String emailValidatorField) { this.emailValidatorField = emailValidatorField; } @@ -60,6 +64,7 @@ public class FieldValidatorsExampleAction extends AbstractValidationActionSuppor return integerValidatorField; } + @StrutsParameter public void setIntegerValidatorField(Integer integerValidatorField) { this.integerValidatorField = integerValidatorField; } @@ -68,6 +73,7 @@ public class FieldValidatorsExampleAction extends AbstractValidationActionSuppor return regexValidatorField; } + @StrutsParameter public void setRegexValidatorField(String regexValidatorField) { this.regexValidatorField = regexValidatorField; } @@ -76,6 +82,7 @@ public class FieldValidatorsExampleAction extends AbstractValidationActionSuppor return requiredStringValidatorField; } + @StrutsParameter public void setRequiredStringValidatorField(String requiredStringValidatorField) { this.requiredStringValidatorField = requiredStringValidatorField; } @@ -84,6 +91,7 @@ public class FieldValidatorsExampleAction extends AbstractValidationActionSuppor return requiredValidatorField; } + @StrutsParameter public void setRequiredValidatorField(String requiredValidatorField) { this.requiredValidatorField = requiredValidatorField; } @@ -92,6 +100,7 @@ public class FieldValidatorsExampleAction extends AbstractValidationActionSuppor return stringLengthValidatorField; } + @StrutsParameter public void setStringLengthValidatorField(String stringLengthValidatorField) { this.stringLengthValidatorField = stringLengthValidatorField; } @@ -100,6 +109,7 @@ public class FieldValidatorsExampleAction extends AbstractValidationActionSuppor return fieldExpressionValidatorField; } + @StrutsParameter public void setFieldExpressionValidatorField( String fieldExpressionValidatorField) { this.fieldExpressionValidatorField = fieldExpressionValidatorField; @@ -109,6 +119,7 @@ public class FieldValidatorsExampleAction extends AbstractValidationActionSuppor return urlValidatorField; } + @StrutsParameter public void setUrlValidatorField(String urlValidatorField) { this.urlValidatorField = urlValidatorField; } diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/wait/LongProcessAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/wait/LongProcessAction.java index 8ff6859bb..cd14a3674 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/wait/LongProcessAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/wait/LongProcessAction.java @@ -21,6 +21,7 @@ package org.apache.struts2.showcase.wait; import com.opensymphony.xwork2.ActionSupport; +import org.apache.struts2.interceptor.parameter.StrutsParameter; /** * Example to illustrate the execAndWait interceptor. @@ -41,6 +42,7 @@ public class LongProcessAction extends ActionSupport { return time; } + @StrutsParameter public void setTime(int time) { this.time = time; } diff --git a/apps/showcase/src/main/resources/struts-params-annotation.xml b/apps/showcase/src/main/resources/struts-params-annotation.xml new file mode 100644 index 000000000..db3992884 --- /dev/null +++ b/apps/showcase/src/main/resources/struts-params-annotation.xml @@ -0,0 +1,32 @@ + + + + + + + + /WEB-INF/paramsannotation.vm + + + diff --git a/apps/showcase/src/main/resources/struts.xml b/apps/showcase/src/main/resources/struts.xml index a9fe3c9dc..33095326c 100644 --- a/apps/showcase/src/main/resources/struts.xml +++ b/apps/showcase/src/main/resources/struts.xml @@ -35,17 +35,7 @@ - - + @@ -93,6 +83,8 @@ + + diff --git a/apps/showcase/src/main/webapp/WEB-INF/paramsannotation.vm b/apps/showcase/src/main/webapp/WEB-INF/paramsannotation.vm new file mode 100644 index 000000000..a0c4efefc --- /dev/null +++ b/apps/showcase/src/main/webapp/WEB-INF/paramsannotation.vm @@ -0,0 +1,19 @@ +#* +* 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. +*# +
$action.renderVarToPrint()
diff --git a/apps/showcase/src/test/java/it/org/apache/struts2/showcase/StrutsParametersTest.java b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/StrutsParametersTest.java new file mode 100644 index 000000000..927f0d101 --- /dev/null +++ b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/StrutsParametersTest.java @@ -0,0 +1,239 @@ +/* + * 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 it.org.apache.struts2.showcase; + +import org.htmlunit.WebClient; +import org.htmlunit.html.HtmlPage; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.springframework.web.util.UriComponentsBuilder; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.Assert.assertEquals; + +public class StrutsParametersTest { + + private WebClient webClient; + + @Before + public void setUp() throws Exception { + webClient = new WebClient(); + } + + @After + public void tearDown() throws Exception { + webClient.close(); + } + + @Test + public void public_StringField_WithoutGetterSetter_FieldNotAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicField", "yes"); + params.put("varToPrint", "publicField"); + assertText(params, "publicField{no}"); + } + + @Test + public void public_StringField_WithoutGetterSetter_FieldAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicFieldAnnotated", "yes"); + params.put("varToPrint", "publicFieldAnnotated"); + assertText(params, "publicFieldAnnotated{yes}"); + } + + @Test + public void private_StringField_WithSetter_MethodNotAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("privateFieldMethod", "yes"); + params.put("varToPrint", "privateField"); + assertText(params, "privateField{no}"); + } + + @Test + public void private_StringField_WithSetter_MethodAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("privateFieldMethodAnnotated", "yes"); + params.put("varToPrint", "privateField"); + assertText(params, "privateField{yes}"); + } + + @Test + public void public_ArrayField_WithoutGetterSetter_FieldNotAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicArray[0]", "1"); + params.put("varToPrint", "publicArray"); + assertText(params, "publicArray{[0]}"); + } + + @Test + public void public_ArrayField_WithoutGetterSetter_FieldAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicArrayAnnotated[0]", "1"); + params.put("varToPrint", "publicArrayAnnotated"); + assertText(params, "publicArrayAnnotated{[1]}"); + } + + @Test + public void public_ListField_WithoutGetterSetter_FieldNotAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicList[0]", "yes"); + params.put("varToPrint", "publicList"); + assertText(params, "publicList{[no]}"); + } + + @Test + public void public_ListField_WithoutGetterSetter_FieldAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicListAnnotated[0]", "yes"); + params.put("varToPrint", "publicListAnnotated"); + assertText(params, "publicListAnnotated{[yes]}"); + } + + @Test + public void private_ListField_WithGetterNoSetter_MethodNotAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("privateListMethod[0]", "yes"); + params.put("varToPrint", "privateList"); + assertText(params, "privateList{[no]}"); + } + + @Test + public void private_ListField_WithGetterNoSetter_MethodAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("privateListMethodAnnotated[0]", "yes"); + params.put("varToPrint", "privateList"); + assertText(params, "privateList{[yes]}"); + } + + @Test + public void public_MapField_WithoutGetterSetter_FieldNotAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicMap['key']", "yes"); + params.put("varToPrint", "publicMap"); + assertText(params, "publicMap{{key=no}}"); + } + + @Test + public void public_MapField_WithoutGetterSetter_FieldAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicMapAnnotated['key']", "yes"); + params.put("varToPrint", "publicMapAnnotated"); + assertText(params, "publicMapAnnotated{{key=yes}}"); + } + + @Test + public void public_MapField_Insert_WithoutGetterSetter_FieldNotAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicMap[999]", "yes"); + params.put("varToPrint", "publicMap"); + assertText(params, "publicMap{{key=no}}"); + } + + @Test + public void public_MapField_Insert_WithoutGetterSetter_FieldAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicMapAnnotated[999]", "yes"); + params.put("varToPrint", "publicMapAnnotated"); + assertText(params, "publicMapAnnotated{{999=yes, key=no}}"); + } + + @Test + public void public_MyDtoField_WithoutGetter_FieldNotAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicMyDto.str", "yes"); + params.put("publicMyDto.map['key']", "yes"); + params.put("publicMyDto.array[0]", "1"); + params.put("varToPrint", "publicMyDto"); + assertText(params, "publicMyDto{str=no, map={key=no}, array=[0]}"); + } + + @Test + public void public_MyDtoField_WithoutGetter_FieldAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("publicMyDtoAnnotated.str", "yes"); + params.put("publicMyDtoAnnotated.map['key']", "yes"); + params.put("publicMyDtoAnnotated.array[0]", "1"); + params.put("varToPrint", "publicMyDtoAnnotated"); + assertText(params, "publicMyDtoAnnotated{str=yes, map={key=yes}, array=[1]}"); + } + + @Test + public void public_MyDtoField_WithoutGetter_FieldAnnotatedDepthOne() throws Exception { + Map params = new HashMap<>(); + params.put("publicMyDtoAnnotatedDepthOne.str", "yes"); + params.put("publicMyDtoAnnotatedDepthOne.map['key']", "yes"); + params.put("publicMyDtoAnnotatedDepthOne.array[0]", "1"); + params.put("varToPrint", "publicMyDtoAnnotatedDepthOne"); + assertText(params, "publicMyDtoAnnotatedDepthOne{str=yes, map={key=no}, array=[0]}"); + } + + @Test + public void private_MyDtoField_WithGetter_MethodNotAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("unsafeMethodMyDto.str", "yes"); + params.put("unsafeMethodMyDto.map['key']", "yes"); + params.put("unsafeMethodMyDto.array[0]", "1"); + params.put("varToPrint", "privateMyDto"); + assertText(params, "privateMyDto{str=no, map={key=no}, array=[0]}"); + } + + @Test + public void private_MyDtoField_WithGetter_MethodNotAnnotated_Alternate() throws Exception { + Map params = new HashMap<>(); + params.put("unsafeMethodMyDto['str']", "yes"); + params.put("unsafeMethodMyDto['map']['key']", "yes"); + params.put("unsafeMethodMyDto['map'][999]", "yes"); + params.put("unsafeMethodMyDto['array'][0]", "1"); + params.put("varToPrint", "privateMyDto"); + assertText(params, "privateMyDto{str=no, map={key=no}, array=[0]}"); + } + + @Test + public void private_MyDtoField_WithGetter_MethodAnnotated() throws Exception { + Map params = new HashMap<>(); + params.put("safeMethodMyDto.str", "yes"); + params.put("safeMethodMyDto.map['key']", "yes"); + params.put("safeMethodMyDto.array[0]", "1"); + params.put("varToPrint", "privateMyDto"); + assertText(params, "privateMyDto{str=yes, map={key=yes}, array=[1]}"); + } + + @Test + public void private_MyDtoField_WithGetter_MethodAnnotatedDepthOne() throws Exception { + Map params = new HashMap<>(); + params.put("safeMethodMyDtoDepthOne.str", "yes"); + params.put("safeMethodMyDtoDepthOne.map['key']", "yes"); + params.put("safeMethodMyDtoDepthOne.array[0]", "1"); + params.put("varToPrint", "privateMyDto"); + assertText(params, "privateMyDto{str=yes, map={key=no}, array=[0]}"); + } + + private void assertText(Map params, String text) throws IOException { + UriComponentsBuilder builder = UriComponentsBuilder.fromHttpUrl(ParameterUtils.getBaseUrl()).path("/paramsannotation/test.action"); + params.forEach(builder::queryParam); + String url = builder.toUriString(); + HtmlPage page = webClient.getPage(url); + String output = page.getElementById("output").asNormalizedText(); + assertEquals(text, output); + } +} diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java index 3a3674a70..7bf0e7c77 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java @@ -112,6 +112,7 @@ import org.apache.struts2.conversion.StrutsTypeConverterHolder; import org.apache.struts2.ognl.OgnlGuard; import org.apache.struts2.ognl.ProviderAllowlist; import org.apache.struts2.ognl.StrutsOgnlGuard; +import org.apache.struts2.ognl.ThreadAllowlist; import java.util.ArrayList; import java.util.Collections; @@ -395,6 +396,7 @@ public class DefaultConfiguration implements Configuration { .factory(SecurityMemberAccess.class, Scope.PROTOTYPE) .factory(OgnlGuard.class, StrutsOgnlGuard.class, Scope.SINGLETON) .factory(ProviderAllowlist.class, Scope.SINGLETON) + .factory(ThreadAllowlist.class, Scope.SINGLETON) .factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON); } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java index 0cd5bf789..510a65c60 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -26,6 +26,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; import org.apache.struts2.ognl.ProviderAllowlist; +import org.apache.struts2.ognl.ThreadAllowlist; import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; @@ -75,6 +76,7 @@ public class SecurityMemberAccess implements MemberAccess { ))); private final ProviderAllowlist providerAllowlist; + private final ThreadAllowlist threadAllowlist; private boolean allowStaticFieldAccess = true; private Set excludeProperties = emptySet(); private Set acceptProperties = emptySet(); @@ -89,8 +91,9 @@ public class SecurityMemberAccess implements MemberAccess { private boolean disallowDefaultPackageAccess = false; @Inject - public SecurityMemberAccess(@Inject ProviderAllowlist providerAllowlist) { + public SecurityMemberAccess(@Inject ProviderAllowlist providerAllowlist, @Inject ThreadAllowlist threadAllowlist) { this.providerAllowlist = providerAllowlist; + this.threadAllowlist = threadAllowlist; } /** @@ -99,11 +102,11 @@ public class SecurityMemberAccess implements MemberAccess { * - block or allow access to properties (configurable-after-construction) * * @param allowStaticFieldAccess if set to true static fields (constants) will be accessible - * @deprecated since 6.4.0, use {@link #SecurityMemberAccess(ProviderAllowlist)} instead. + * @deprecated since 6.4.0, use {@link #SecurityMemberAccess(ProviderAllowlist, ThreadAllowlist)} instead. */ @Deprecated public SecurityMemberAccess(boolean allowStaticFieldAccess) { - this(null); + this(null, null); useAllowStaticFieldAccess(String.valueOf(allowStaticFieldAccess)); } @@ -223,6 +226,7 @@ public class SecurityMemberAccess implements MemberAccess { return allowlistClasses.contains(clazz) || ALLOWLIST_REQUIRED_CLASSES.contains(clazz) || (providerAllowlist != null && providerAllowlist.getProviderAllowlist().contains(clazz)) + || (threadAllowlist != null && threadAllowlist.getAllowlist().contains(clazz)) || isClassBelongsToPackages(clazz, ALLOWLIST_REQUIRED_PACKAGES) || isClassBelongsToPackages(clazz, allowlistPackageNames); } diff --git a/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java b/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java index 0896fec82..be803b7e8 100644 --- a/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java +++ b/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java @@ -25,12 +25,14 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; -import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; import java.util.Set; import java.util.regex.Pattern; +import static java.util.Arrays.asList; +import static java.util.Collections.unmodifiableSet; +import static java.util.stream.Collectors.joining; + public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { private static final Logger LOG = LogManager.getLogger(DefaultAcceptedPatternsChecker.class); @@ -39,6 +41,12 @@ public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { "\\w+((\\.\\w+)|(\\[\\d+])|(\\(\\d+\\))|(\\['(\\w-?|[\\u4e00-\\u9fa5]-?)+'])|(\\('(\\w-?|[\\u4e00-\\u9fa5]-?)+'\\)))*" }; + /** + * Must match {@link #ACCEPTED_PATTERNS} RegEx. Signifies characters which result in a nested lookup via OGNL. + */ + public static final Set NESTING_CHARS = unmodifiableSet(new HashSet<>(asList('.', '[', '('))); + public static final String NESTING_CHARS_STR = NESTING_CHARS.stream().map(String::valueOf).collect(joining()); + public static final String[] DMI_AWARE_ACCEPTED_PATTERNS = { "\\w+([:]?\\w+)?((\\.\\w+)|(\\[\\d+])|(\\(\\d+\\))|(\\['(\\w-?|[\\u4e00-\\u9fa5]-?)+'])|(\\('(\\w-?|[\\u4e00-\\u9fa5]-?)+'\\)))*([!]?\\w+)?" }; @@ -74,7 +82,7 @@ public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { newAcceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); } } finally { - acceptedPatterns = Collections.unmodifiableSet(newAcceptedPatterns); + acceptedPatterns = unmodifiableSet(newAcceptedPatterns); } } @@ -85,7 +93,7 @@ public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { @Override public void setAcceptedPatterns(String[] additionalPatterns) { - setAcceptedPatterns(new HashSet<>(Arrays.asList(additionalPatterns))); + setAcceptedPatterns(new HashSet<>(asList(additionalPatterns))); } @Override @@ -97,7 +105,7 @@ public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { newAcceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); } } finally { - acceptedPatterns = Collections.unmodifiableSet(newAcceptedPatterns); + acceptedPatterns = unmodifiableSet(newAcceptedPatterns); } } diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 8c0ccbec6..e85ce9832 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -476,6 +476,9 @@ public final class StrutsConstants { public static final String STRUTS_ADDITIONAL_EXCLUDED_PATTERNS = "struts.additional.excludedPatterns"; public static final String STRUTS_ADDITIONAL_ACCEPTED_PATTERNS = "struts.additional.acceptedPatterns"; + public static final String STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS = "struts.parameters.requireAnnotations"; + public static final String STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS_TRANSITION = "struts.parameters.requireAnnotations.transitionMode"; + public static final String STRUTS_CONTENT_TYPE_MATCHER = "struts.contentTypeMatcher"; public static final String STRUTS_SMI_METHOD_REGEX = "struts.strictMethodInvocation.methodRegex"; diff --git a/core/src/main/java/org/apache/struts2/components/IteratorComponent.java b/core/src/main/java/org/apache/struts2/components/IteratorComponent.java index 2e8ee4ae6..7ff83bd45 100644 --- a/core/src/main/java/org/apache/struts2/components/IteratorComponent.java +++ b/core/src/main/java/org/apache/struts2/components/IteratorComponent.java @@ -18,9 +18,11 @@ */ package org.apache.struts2.components; +import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.util.ValueStack; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.struts2.ognl.ThreadAllowlist; import org.apache.struts2.util.MakeIterator; import org.apache.struts2.views.annotations.StrutsTag; import org.apache.struts2.views.annotations.StrutsTagAttribute; @@ -188,8 +190,8 @@ import java.util.List; * * *

Another way to create a simple loop, similar to JSTL's - * <c:forEach begin="..." end="..." ...> is to use some - * OGNL magic, which provides some under-the-covers magic to + * <c:forEach begin="..." end="..." ...> is to use some + * OGNL magic, which provides some under-the-covers magic to * make 0-n loops trivial. This example also loops five times.

* * @@ -237,11 +239,17 @@ public class IteratorComponent extends ContextBean { protected Integer end; protected String stepStr; protected Integer step; + private ThreadAllowlist threadAllowlist; public IteratorComponent(ValueStack stack) { super(stack); } + @Inject + public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { + this.threadAllowlist = threadAllowlist; + } + public boolean start(Writer writer) { //Create an iterator status if the status attribute was set. if (statusAttr != null) { @@ -298,6 +306,7 @@ public class IteratorComponent extends ContextBean { if ((iterator != null) && iterator.hasNext()) { Object currentValue = iterator.next(); stack.push(currentValue); + threadAllowlist.allowClass(currentValue.getClass()); String var = getVar(); diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index 2127bb663..4b335afe6 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -68,6 +68,7 @@ import org.apache.struts2.dispatcher.mapper.ActionMapper; import org.apache.struts2.dispatcher.mapper.ActionMapping; import org.apache.struts2.dispatcher.multipart.MultiPartRequest; import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; +import org.apache.struts2.ognl.ThreadAllowlist; import org.apache.struts2.util.ObjectFactoryDestroyable; import org.apache.struts2.util.fs.JBossFileManager; @@ -199,6 +200,7 @@ public class Dispatcher { private LocaleProviderFactory localeProviderFactory; private StaticContentLoader staticContentLoader; private ActionMapper actionMapper; + private ThreadAllowlist threadAllowlist; /** * Provide the dispatcher instance for the current thread. @@ -404,6 +406,11 @@ public class Dispatcher { return actionMapper; } + @Inject + public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { + this.threadAllowlist = threadAllowlist; + } + /** * Releases all instances bound to this dispatcher instance. */ @@ -1043,6 +1050,7 @@ public class Dispatcher { */ public void cleanUpRequest(HttpServletRequest request) { ContainerHolder.clear(); + threadAllowlist.clearAllowlist(); if (!(request instanceof MultiPartRequestWrapper)) { return; } diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java index efc4a7b04..e9215e533 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java @@ -25,6 +25,7 @@ import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.interceptor.MethodFilterInterceptor; import com.opensymphony.xwork2.interceptor.ValidationAware; import com.opensymphony.xwork2.security.AcceptedPatternsChecker; +import com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker; import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.util.ClearableValueStack; import com.opensymphony.xwork2.util.MemberAccessValueStack; @@ -33,6 +34,7 @@ import com.opensymphony.xwork2.util.ValueStack; import com.opensymphony.xwork2.util.ValueStackFactory; import com.opensymphony.xwork2.util.reflection.ReflectionContextState; import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.ClassUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; @@ -41,17 +43,33 @@ import org.apache.struts2.action.ParameterNameAware; import org.apache.struts2.action.ParameterValueAware; import org.apache.struts2.dispatcher.HttpParameters; import org.apache.struts2.dispatcher.Parameter; +import org.apache.struts2.ognl.ThreadAllowlist; +import java.beans.BeanInfo; +import java.beans.IntrospectionException; +import java.beans.Introspector; +import java.beans.PropertyDescriptor; +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.Arrays; import java.util.Collection; import java.util.Comparator; import java.util.HashSet; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.regex.Pattern; +import static com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS; +import static com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS_STR; import static java.util.Collections.unmodifiableSet; import static java.util.stream.Collectors.joining; +import static org.apache.commons.lang3.StringUtils.indexOfAny; import static org.apache.commons.lang3.StringUtils.normalizeSpace; /** @@ -70,8 +88,11 @@ public class ParametersInterceptor extends MethodFilterInterceptor { private boolean dmiEnabled = false; protected boolean ordered = false; + protected boolean requireAnnotations = false; + protected boolean requireAnnotationsTransitionMode = false; private ValueStackFactory valueStackFactory; + protected ThreadAllowlist threadAllowlist; private ExcludedPatternsChecker excludedPatterns; private AcceptedPatternsChecker acceptedPatterns; private Set excludedValuePatterns = null; @@ -82,11 +103,35 @@ public class ParametersInterceptor extends MethodFilterInterceptor { this.valueStackFactory = valueStackFactory; } + @Inject + public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { + this.threadAllowlist = threadAllowlist; + } + @Inject(StrutsConstants.STRUTS_DEVMODE) public void setDevMode(String mode) { this.devMode = BooleanUtils.toBoolean(mode); } + @Inject(value = StrutsConstants.STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS, required = false) + public void setRequireAnnotations(String requireAnnotations) { + this.requireAnnotations = BooleanUtils.toBoolean(requireAnnotations); + } + + /** + * When 'Transition Mode' is enabled, parameters that are not 'nested' will be accepted without annotations. What + * this means in practice is that all public setters on an Action will be exposed for parameter injection again, and + * only 'nested' parameters, i.e. public getters on an Action, will require annotations. + *

+ * In this mode, the OGNL auto-allowlisting capability is not degraded in any way, and as such, it offers a + * convenient option for applications to enable the OGNL allowlist capability whilst they work through the process + * of annotating all their Action parameters. + */ + @Inject(value = StrutsConstants.STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS_TRANSITION, required = false) + public void setRequireAnnotationsTransitionMode(String transitionMode) { + this.requireAnnotationsTransitionMode = BooleanUtils.toBoolean(transitionMode); + } + @Inject public void setExcludedPatterns(ExcludedPatternsChecker excludedPatterns) { this.excludedPatterns = excludedPatterns; @@ -295,13 +340,168 @@ public class ParametersInterceptor extends MethodFilterInterceptor { * @return true if parameter is accepted */ protected boolean isAcceptableParameter(String name, Object action) { - return acceptableName(name) && isAcceptableParameterNameAware(name, action); + return acceptableName(name) && isAcceptableParameterNameAware(name, action) && isParameterAnnotatedAndAllowlist(name, action); } protected boolean isAcceptableParameterNameAware(String name, Object action) { return !(action instanceof ParameterNameAware) || ((ParameterNameAware) action).acceptableParameterName(name); } + /** + * Checks if the Action class member corresponding to a parameter is appropriately annotated with + * {@link StrutsParameter} and OGNL allowlists any necessary classes. + *

+ * Note that this logic relies on the use of {@link DefaultAcceptedPatternsChecker#NESTING_CHARS} and may also + * be adversely impacted by the use of custom OGNL property accessors. + */ + protected boolean isParameterAnnotatedAndAllowlist(String name, Object action) { + if (!requireAnnotations) { + return true; + } + + long paramDepth = name.codePoints().mapToObj(c -> (char) c).filter(NESTING_CHARS::contains).count(); + if (requireAnnotationsTransitionMode && paramDepth == 0) { + return true; + } + + int nestingIndex = indexOfAny(name, NESTING_CHARS_STR); + String rootProperty = nestingIndex == -1 ? name : name.substring(0, nestingIndex); + String normalisedRootProperty = Character.toLowerCase(rootProperty.charAt(0)) + rootProperty.substring(1); + + return hasValidAnnotatedMember(normalisedRootProperty, action, paramDepth); + } + + /** + * Note that we check for a public field last or only if there is no valid, annotated property descriptor. This is + * because this check is likely to fail more often than not, as the relative use of public fields is low - so we + * save computation by checking this last. + */ + protected boolean hasValidAnnotatedMember(String rootProperty, Object action, long paramDepth) { + BeanInfo beanInfo = getBeanInfo(action); + if (beanInfo == null) { + return hasValidAnnotatedField(action, rootProperty, paramDepth); + } + + Optional propDescOpt = Arrays.stream(beanInfo.getPropertyDescriptors()) + .filter(desc -> desc.getName().equals(rootProperty)).findFirst(); + if (!propDescOpt.isPresent()) { + return hasValidAnnotatedField(action, rootProperty, paramDepth); + } + + if (hasValidAnnotatedPropertyDescriptor(propDescOpt.get(), paramDepth)) { + return true; + } + + return hasValidAnnotatedField(action, rootProperty, paramDepth); + } + + protected boolean hasValidAnnotatedPropertyDescriptor(PropertyDescriptor propDesc, long paramDepth) { + Method relevantMethod = paramDepth == 0 ? propDesc.getWriteMethod() : propDesc.getReadMethod(); + if (relevantMethod == null) { + return false; + } + if (getPermittedInjectionDepth(relevantMethod) < paramDepth) { + LOG.debug( + "Parameter injection for method [{}] on action [{}] rejected. Ensure it is annotated with @StrutsParameter with an appropriate 'depth'.", + relevantMethod.getName(), + relevantMethod.getDeclaringClass().getName()); + return false; + } + if (paramDepth >= 1) { + allowlistClass(relevantMethod.getReturnType()); + } + if (paramDepth >= 2) { + allowlistReturnTypeIfParameterized(relevantMethod); + } + return true; + } + + protected void allowlistReturnTypeIfParameterized(Method method) { + allowlistParameterizedTypeArg(method.getGenericReturnType()); + } + + protected void allowlistParameterizedTypeArg(Type genericType) { + if (!(genericType instanceof ParameterizedType)) { + return; + } + Type[] paramTypes = ((ParameterizedType) genericType).getActualTypeArguments(); + allowlistParamType(paramTypes[0]); + if (paramTypes.length > 1) { + // Probably useful for Map or Map-like classes + allowlistParamType(paramTypes[1]); + } + } + + protected void allowlistParamType(Type paramType) { + if (paramType instanceof Class) { + allowlistClass((Class) paramType); + } + } + + protected void allowlistClass(Class clazz) { + threadAllowlist.allowClass(clazz); + ClassUtils.getAllSuperclasses(clazz).forEach(threadAllowlist::allowClass); + ClassUtils.getAllInterfaces(clazz).forEach(threadAllowlist::allowClass); + } + + protected boolean hasValidAnnotatedField(Object action, String fieldName, long paramDepth) { + Field field; + try { + field = action.getClass().getDeclaredField(fieldName); + } catch (NoSuchFieldException e) { + return false; + } + if (!Modifier.isPublic(field.getModifiers())) { + return false; + } + if (getPermittedInjectionDepth(field) < paramDepth) { + LOG.debug( + "Parameter injection for field [{}] on action [{}] rejected. Ensure it is annotated with @StrutsParameter with an appropriate 'depth'.", + fieldName, + action.getClass().getName()); + return false; + } + if (paramDepth >= 1) { + allowlistClass(field.getType()); + } + if (paramDepth >= 2) { + allowlistFieldIfParameterized(field); + } + return true; + } + + protected void allowlistFieldIfParameterized(Field field) { + allowlistParameterizedTypeArg(field.getGenericType()); + } + + /** + * @return permitted injection depth where -1 indicates not permitted + */ + protected int getPermittedInjectionDepth(AnnotatedElement element) { + StrutsParameter annotation = getParameterAnnotation(element); + if (annotation == null) { + return -1; + } + return annotation.depth(); + } + + /** + * Annotation retrieval logic. Can be overridden to support extending annotations or some other form of annotation + * inheritance. + */ + protected StrutsParameter getParameterAnnotation(AnnotatedElement element) { + return element.getAnnotation(StrutsParameter.class); + } + + protected BeanInfo getBeanInfo(Object action) { + try { + return Introspector.getBeanInfo(action.getClass()); + } catch (IntrospectionException e) { + LOG.warn("Error introspecting Action {} for parameter injection validation", action.getClass(), e); + return null; + } + } + /** * Checks if parameter value can be accepted or thrown away * diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameter.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameter.java new file mode 100644 index 000000000..2a19aa83f --- /dev/null +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameter.java @@ -0,0 +1,44 @@ +/* + * 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.struts2.interceptor.parameter; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Used to annotate public getter/setter methods or fields on {@link com.opensymphony.xwork2.Action} classes that are + * intended for parameter injection by the {@link ParametersInterceptor}. + * + * @since 6.4.0 + */ +@Target({ElementType.METHOD, ElementType.FIELD}) +@Retention(RetentionPolicy.RUNTIME) +public @interface StrutsParameter { + + /** + * The depth to which parameter injection is permitted, where a depth of 0 only allows setters/fields directly on + * the action class. Setting within a POJO on an action will require a depth of 1 or more depending on the level of + * nesting within the POJO. + *

+ * In a practical sense, the depth dictates the number of periods or brackets that can appear in the parameter name. + */ + int depth() default 0; +} diff --git a/core/src/main/java/org/apache/struts2/ognl/ThreadAllowlist.java b/core/src/main/java/org/apache/struts2/ognl/ThreadAllowlist.java new file mode 100644 index 000000000..e07cae64e --- /dev/null +++ b/core/src/main/java/org/apache/struts2/ognl/ThreadAllowlist.java @@ -0,0 +1,51 @@ +/* + * 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.struts2.ognl; + +import java.util.HashSet; +import java.util.Set; + +import static java.util.Collections.emptySet; +import static java.util.Collections.unmodifiableSet; + +/** + * Allows any bean to allowlist a class for use in OGNL expressions, for the current thread only. The allowlist can be + * cleared once any desired OGNL expressions have been evaluated. + * + * @since 6.4.0 + */ +public class ThreadAllowlist { + + private final ThreadLocal>> allowlist = new ThreadLocal<>(); + + public void allowClass(Class clazz) { + if (allowlist.get() == null) { + allowlist.set(new HashSet<>()); + } + allowlist.get().add(clazz); + } + + public void clearAllowlist() { + allowlist.remove(); + } + + public Set> getAllowlist() { + return allowlist.get() != null ? unmodifiableSet(allowlist.get()) : emptySet(); + } +} diff --git a/core/src/main/java/org/apache/struts2/views/jsp/ComponentTagSupport.java b/core/src/main/java/org/apache/struts2/views/jsp/ComponentTagSupport.java index 7ef4eed1e..e090daf77 100644 --- a/core/src/main/java/org/apache/struts2/views/jsp/ComponentTagSupport.java +++ b/core/src/main/java/org/apache/struts2/views/jsp/ComponentTagSupport.java @@ -22,7 +22,6 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.jsp.JspException; -import com.opensymphony.xwork2.ActionContext; import org.apache.struts2.components.Component; import com.opensymphony.xwork2.inject.Container; @@ -39,8 +38,7 @@ public abstract class ComponentTagSupport extends StrutsBodyTagSupport { public int doEndTag() throws JspException { component.end(pageContext.getOut(), getBody()); component = null; // Always clear component reference (since clearTagStateForTagPoolingServers() is conditional). - clearTagStateForTagPoolingServers(); - return EVAL_PAGE; + return super.doEndTag(); } @Override @@ -49,7 +47,7 @@ public abstract class ComponentTagSupport extends StrutsBodyTagSupport { component = getBean(stack, (HttpServletRequest) pageContext.getRequest(), (HttpServletResponse) pageContext.getResponse()); Container container = stack.getActionContext().getContainer(); container.inject(component); - + populateParams(); boolean evalBody = component.start(pageContext.getOut()); @@ -62,7 +60,7 @@ public abstract class ComponentTagSupport extends StrutsBodyTagSupport { /** * Define method to populate component state based on the Tag parameters. - * + * * Descendants should override this method for custom behaviour, but should always call the ancestor method when doing so. */ protected void populateParams() { @@ -71,7 +69,7 @@ public abstract class ComponentTagSupport extends StrutsBodyTagSupport { /** * Specialized method to populate the performClearTagStateForTagPoolingServers state of the Component to match the value set in the Tag. - * + * * Generally only unit tests would call this method directly, to avoid calling the whole populateParams() chain again after doStartTag() * has been called. Doing that can break tag / component state behaviour, but unit tests still need a way to set the * performClearTagStateForTagPoolingServers state for the component (which only comes into being after doStartTag() is called). diff --git a/core/src/main/java/org/apache/struts2/views/jsp/IteratorStatus.java b/core/src/main/java/org/apache/struts2/views/jsp/IteratorStatus.java index 13022f856..fedef363e 100644 --- a/core/src/main/java/org/apache/struts2/views/jsp/IteratorStatus.java +++ b/core/src/main/java/org/apache/struts2/views/jsp/IteratorStatus.java @@ -26,23 +26,29 @@ package org.apache.struts2.views.jsp; *

  • count: iterations so far, starts on 1. count is always index + 1
    • *
  • first: true if index == 0
    • *
  • even: true if (index + 1) % 2 == 0
    • - *
  • last: true if current iteration is the last iteration
    • + *
  • last: true if current iteration is the last iteration
    • *
  • odd: true if (index + 1) % 2 == 1
    • * *

    Example

    * 
      *   <s:iterator status="status" value='{0, 1}'>
  *      Index: <s:property value="%{#status.index}" /> <br />
- *      Count: <s:property value="%{#status.count}" /> <br />  
+ *      Index Str: <s:property value="%{#status.indexStr}" /> <br />
+ *      Count: <s:property value="%{#status.count}" /> <br />
+ *      Count Str: <s:property value="%{#status.countStr}" /> <br />
  *   </s:iterator>
  *
    - * + * *

    will print

    * 
      *      Index: 0
+ *      Index Str: 0
  *      Count: 1
+ *      Count Str: 1
  *      Index: 1
+ *      Index Str: 1
  *      Count: 2
+ *      Count Str: 2
  *
    */ public class IteratorStatus { @@ -56,6 +62,10 @@ public class IteratorStatus { return state.index + 1; } + public String getCountStr() { + return String.valueOf(state.index + 1); + } + public boolean isEven() { return ((state.index + 1) % 2) == 0; } @@ -68,6 +78,10 @@ public class IteratorStatus { return state.index; } + public String getIndexStr() { + return String.valueOf(state.index); + } + public boolean isLast() { return state.last; } diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml index 93614fa0b..5c3121f77 100644 --- a/core/src/main/resources/struts-beans.xml +++ b/core/src/main/resources/struts-beans.xml @@ -170,6 +170,7 @@ + diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java index 7e5a22bcc..03bad82e4 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java @@ -25,6 +25,7 @@ import com.opensymphony.xwork2.util.Foo; import ognl.MemberAccess; import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.struts2.ognl.ProviderAllowlist; +import org.apache.struts2.ognl.ThreadAllowlist; import org.junit.Before; import org.junit.Test; @@ -54,18 +55,21 @@ public class SecurityMemberAccessTest { private FooBar target; protected SecurityMemberAccess sma; private ProviderAllowlist mockedProviderAllowlist; + private ThreadAllowlist mockedThreadAllowlist; @Before public void setUp() throws Exception { context = new HashMap<>(); target = new FooBar(); mockedProviderAllowlist = mock(ProviderAllowlist.class); + mockedThreadAllowlist = mock(ThreadAllowlist.class); assignNewSma(true); } protected void assignNewSma(boolean allowStaticFieldAccess) { when(mockedProviderAllowlist.getProviderAllowlist()).thenReturn(new HashSet<>()); - sma = new SecurityMemberAccess(mockedProviderAllowlist); + when(mockedThreadAllowlist.getAllowlist()).thenReturn(new HashSet<>()); + sma = new SecurityMemberAccess(mockedProviderAllowlist, mockedThreadAllowlist); sma.useAllowStaticFieldAccess(String.valueOf(allowStaticFieldAccess)); } diff --git a/core/src/test/java/com/opensymphony/xwork2/test/User.java b/core/src/test/java/com/opensymphony/xwork2/test/User.java index d377fe018..e6c5d2af4 100644 --- a/core/src/test/java/com/opensymphony/xwork2/test/User.java +++ b/core/src/test/java/com/opensymphony/xwork2/test/User.java @@ -37,6 +37,12 @@ public class User implements UserMarker { private String email2; private String name; + public User() { + } + + public User(String name) { + this.name = name; + } public void setCollection(Collection collection) { this.collection = collection; diff --git a/core/src/test/java/org/apache/struts2/TestAction.java b/core/src/test/java/org/apache/struts2/TestAction.java index 77f784a61..b9595de11 100644 --- a/core/src/test/java/org/apache/struts2/TestAction.java +++ b/core/src/test/java/org/apache/struts2/TestAction.java @@ -45,6 +45,7 @@ public class TestAction extends ActionSupport { private String result; private User user; private String[] array; + private Object[] objectArray; private String[][] list; private List list2; private List list3; @@ -135,6 +136,14 @@ public class TestAction extends ActionSupport { this.array = array; } + public Object[] getObjectArray() { + return objectArray; + } + + public void setObjectArray(Object[] arrayObject) { + this.objectArray = arrayObject; + } + public String[][] getList() { return list; } diff --git a/core/src/test/java/org/apache/struts2/components/IteratorComponentTest.java b/core/src/test/java/org/apache/struts2/components/IteratorComponentTest.java index d6cc5305d..7f08ef64e 100644 --- a/core/src/test/java/org/apache/struts2/components/IteratorComponentTest.java +++ b/core/src/test/java/org/apache/struts2/components/IteratorComponentTest.java @@ -19,23 +19,38 @@ package org.apache.struts2.components; import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.test.User; import com.opensymphony.xwork2.util.ValueStack; import org.apache.struts2.StrutsInternalTestCase; +import org.apache.struts2.ognl.ThreadAllowlist; +import org.apache.struts2.TestAction; import java.io.StringWriter; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Locale; public class IteratorComponentTest extends StrutsInternalTestCase { + private ValueStack stack; + private IteratorComponent ic; + + @Override + public void setUp() throws Exception { + super.setUp(); + stack = ActionContext.getContext().getValueStack(); + ic = new IteratorComponent(stack); + ThreadAllowlist threadAllowlist = new ThreadAllowlist(); + ic.setThreadAllowlist(threadAllowlist); + } + public void testIterator() throws Exception { // given - final ValueStack stack = ActionContext.getContext().getValueStack(); stack.push(new FooAction()); StringWriter out = new StringWriter(); - IteratorComponent ic = new IteratorComponent(stack); ic.setValue("items"); ic.setVar("val"); @@ -62,14 +77,53 @@ public class IteratorComponentTest extends StrutsInternalTestCase { assertEquals("item1 item2 item3 item4 ", out.getBuffer().toString()); } - public void testIteratorWithBegin() throws Exception { + public void testSimpleIterator() { + // given + stack.push(new FooAction()); + + StringWriter out = new StringWriter(); + + ic.setBegin("1"); + ic.setEnd("8"); + ic.setStep("2"); + ic.setStatus("status"); + + Property prop = new Property(stack); + Property status = new Property(stack); + status.setValue("#status.index"); + + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + + String body = " "; + + // when + assertTrue(ic.start(out)); + + for (int i = 0; i < 4; i++) { + status.start(out); + status.end(out, body); + prop.start(out); + prop.end(out, body); + ic.end(out, null); + } + + // then + assertEquals("0 1 1 3 2 5 3 7 ", out.getBuffer().toString()); + } + + public void testIteratorWithBegin() { // given - final ValueStack stack = ActionContext.getContext().getValueStack(); stack.push(new FooAction()); StringWriter out = new StringWriter(); - IteratorComponent ic = new IteratorComponent(stack); ic.setValue("items"); ic.setVar("val"); ic.setBegin("1"); @@ -94,20 +148,18 @@ public class IteratorComponentTest extends StrutsInternalTestCase { assertEquals("item2 item3 item4 ", out.getBuffer().toString()); } - public void testIteratorWithNulls() throws Exception { + public void testIteratorWithNulls() { // given - final ValueStack stack = ActionContext.getContext().getValueStack(); stack.push(new FooAction() { - private List items = Arrays.asList("1", "2", null, "4"); + private final List items = Arrays.asList("1", "2", null, "4"); - public List getItems() { + public List getItems() { return items; } }); StringWriter out = new StringWriter(); - IteratorComponent ic = new IteratorComponent(stack); ic.setValue("items"); ic.setVar("val"); Property prop = new Property(stack); @@ -132,15 +184,147 @@ public class IteratorComponentTest extends StrutsInternalTestCase { assertEquals("1, 2, , 4, ", out.getBuffer().toString()); } + public void testIteratorWithDifferentLocale() { + // given + ActionContext.getContext().withLocale(new Locale("fa_IR")); + stack.push(new FooAction()); + + StringWriter out = new StringWriter(); + + ic.setBegin("1"); + ic.setEnd("3"); + ic.setStatus("status"); + + Property prop = new Property(stack); + Property status = new Property(stack); + status.setValue("#status.count"); + + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + + String body = ","; + + // when + assertTrue(ic.start(out)); + + for (int i = 0; i < 3; i++) { + status.start(out); + status.end(out, body); + + prop.start(out); + prop.end(out, body); + ic.end(out, null); + } + + // then + assertEquals("1,1,2,2,3,3,", out.getBuffer().toString()); + } + + public void testListOfBeansIterator() { + // given + TestAction action = new TestAction(); + action.setList2(new ArrayList() {{ + add(new User("Anton")); + add(new User("Tym")); + add(new User("Luk")); + }}); + stack.push(action); + + StringWriter out = new StringWriter(); + + ic.setValue("list2"); + ic.setStatus("status"); + + Property prop = new Property(stack); + prop.setValue("name"); + Property status = new Property(stack); + status.setValue("#status.indexStr"); + + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + + String body = ","; + + // when + assertTrue(ic.start(out)); + + for (int i = 0; i < 3; i++) { + status.start(out); + status.end(out, body); + + prop.start(out); + prop.end(out, body); + + ic.end(out, null); + } + + // then + assertEquals("0,Anton,1,Tym,2,Luk,", out.getBuffer().toString()); + } + + public void testArrayOfBeansIterator() { + // given + TestAction action = new TestAction(); + action.setObjectArray(new ArrayList() {{ + add(new User("Anton")); + add(new User("Tym")); + add(new User("Luk")); + }}.toArray()); + stack.push(action); + + StringWriter out = new StringWriter(); + + ic.setValue("objectArray"); + ic.setStatus("status"); + + Property prop = new Property(stack); + prop.setValue("name"); + Property status = new Property(stack); + status.setValue("#status.countStr"); + + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + ic.getComponentStack().push(status); + ic.getComponentStack().push(prop); + + String body = " "; + + // when + assertTrue(ic.start(out)); + + for (int i = 0; i < 3; i++) { + status.start(out); + status.end(out, body); + + prop.start(out); + prop.end(out, body); + + ic.end(out, null); + } + + // then + assertEquals("1 Anton 2 Tym 3 Luk ", out.getBuffer().toString()); + } + static class FooAction { - private List items; + private final List items; public FooAction() { items = Arrays.asList("item1", "item2", "item3", "item4"); } - public List getItems() { + public List getItems() { return items; } } diff --git a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java new file mode 100644 index 000000000..53fa14717 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java @@ -0,0 +1,348 @@ +/* + * 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.struts2.interceptor.parameter; + +import com.opensymphony.xwork2.security.AcceptedPatternsChecker; +import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; +import org.apache.commons.lang3.ClassUtils; +import org.apache.struts2.dispatcher.HttpParameters; +import org.apache.struts2.dispatcher.Parameter; +import org.apache.struts2.ognl.ThreadAllowlist; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class StrutsParameterAnnotationTest { + + private ParametersInterceptor parametersInterceptor; + + private ThreadAllowlist threadAllowlist; + + @Before + public void setUp() throws Exception { + parametersInterceptor = new ParametersInterceptor(); + parametersInterceptor.setRequireAnnotations(Boolean.TRUE.toString()); + + threadAllowlist = new ThreadAllowlist(); + parametersInterceptor.setThreadAllowlist(threadAllowlist); + + NotExcludedAcceptedPatternsChecker checker = mock(NotExcludedAcceptedPatternsChecker.class); + when(checker.isAccepted(anyString())).thenReturn(AcceptedPatternsChecker.IsAccepted.yes("")); + when(checker.isExcluded(anyString())).thenReturn(NotExcludedAcceptedPatternsChecker.IsExcluded.no(new HashSet<>())); + parametersInterceptor.setAcceptedPatterns(checker); + parametersInterceptor.setExcludedPatterns(checker); + } + + @After + public void tearDown() throws Exception { + threadAllowlist.clearAllowlist(); + } + + private void testParameter(Object action, String paramName, boolean shouldContain) { + Map requestParamMap = new HashMap<>(); + requestParamMap.put(paramName, new String[]{"value"}); + HttpParameters httpParameters = HttpParameters.create(requestParamMap).build(); + + Map acceptedParameters = parametersInterceptor.toAcceptableParameters(httpParameters, action); + + if (shouldContain) { + assertThat(acceptedParameters).containsOnlyKeys(paramName); + } else { + assertThat(acceptedParameters).isEmpty(); + assertThat(threadAllowlist.getAllowlist()).isEmpty(); + } + } + + private Set> getParentClasses(Class ...clazzes) { + Set> set = new HashSet<>(); + for (Class clazz : clazzes) { + set.add(clazz); + set.addAll(ClassUtils.getAllSuperclasses(clazz)); + set.addAll(ClassUtils.getAllInterfaces(clazz)); + } + return set; + } + + @Test + public void privateStrAnnotated() { + testParameter(new FieldAction(), "privateStr", false); + } + + @Test + public void publicStrAnnotated() { + testParameter(new FieldAction(), "publicStr", true); + assertThat(threadAllowlist.getAllowlist()).isEmpty(); + } + + @Test + public void publicStrNotAnnotated() { + testParameter(new FieldAction(), "publicStrNotAnnotated", false); + } + + @Test + public void privatePojoAnnotated() { + testParameter(new FieldAction(), "privatePojo.key", false); + } + + @Test + public void publicPojoDepthZero() { + testParameter(new FieldAction(), "publicPojoDepthZero.key", false); + } + + @Test + public void publicPojoDepthOne() { + testParameter(new FieldAction(), "publicPojoDepthOne.key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); + } + + @Test + public void publicPojoDepthOne_sqrBracket() { + testParameter(new FieldAction(), "publicPojoDepthOne['key']", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); + } + + @Test + public void publicPojoDepthOne_bracket() { + testParameter(new FieldAction(), "publicPojoDepthOne('key')", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); + } + + @Test + public void publicNestedPojoDepthOne() { + testParameter(new FieldAction(), "publicPojoDepthOne.key.key", false); + } + + @Test + public void publicPojoDepthTwo() { + testParameter(new FieldAction(), "publicPojoDepthTwo.key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); + } + + @Test + public void publicNestedPojoDepthTwo() { + testParameter(new FieldAction(), "publicPojoDepthTwo.key.key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); + } + + @Test + public void publicNestedPojoDepthTwo_sqrBracket() { + testParameter(new FieldAction(), "publicPojoDepthTwo['key']['key']", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); + } + + @Test + public void publicNestedPojoDepthTwo_bracket() { + testParameter(new FieldAction(), "publicPojoDepthTwo('key')('key')", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); + } + + @Test + public void privateStrAnnotatedMethod() { + testParameter(new MethodAction(), "privateStr", false); + } + + @Test + public void publicStrAnnotatedMethod() { + testParameter(new MethodAction(), "publicStr", true); + assertThat(threadAllowlist.getAllowlist()).isEmpty(); + } + + @Test + public void publicStrNotAnnotatedMethod() { + testParameter(new MethodAction(), "publicStrNotAnnotated", false); + } + + @Test + public void privatePojoAnnotatedMethod() { + testParameter(new MethodAction(), "privatePojo.key", false); + } + + @Test + public void publicPojoDepthZeroMethod() { + testParameter(new MethodAction(), "publicPojoDepthZero.key", false); + } + + @Test + public void publicPojoDepthOneMethod() { + testParameter(new MethodAction(), "publicPojoDepthOne.key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); + } + + @Test + public void publicNestedPojoDepthOneMethod() { + testParameter(new MethodAction(), "publicPojoDepthOne.key.key", false); + } + + @Test + public void publicPojoDepthTwoMethod() { + testParameter(new MethodAction(), "publicPojoDepthTwo.key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); + } + + @Test + public void publicNestedPojoDepthTwoMethod() { + testParameter(new MethodAction(), "publicPojoDepthTwo.key.key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); + } + + @Test + public void publicPojoListDepthOne() { + testParameter(new FieldAction(), "publicPojoListDepthOne[0].key", false); + } + + @Test + public void publicPojoListDepthTwo() { + testParameter(new FieldAction(), "publicPojoListDepthTwo[0].key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(List.class, Pojo.class)); + } + + @Test + public void publicPojoMapDepthTwo() { + testParameter(new FieldAction(), "publicPojoMapDepthTwo['a'].key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Map.class, String.class, Pojo.class)); + } + + @Test + public void publicPojoListDepthOneMethod() { + testParameter(new MethodAction(), "publicPojoListDepthOne[0].key", false); + } + + @Test + public void publicPojoListDepthTwoMethod() { + testParameter(new MethodAction(), "publicPojoListDepthTwo[0].key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(List.class, Pojo.class)); + } + + @Test + public void publicPojoMapDepthTwoMethod() { + testParameter(new MethodAction(), "publicPojoMapDepthTwo['a'].key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Map.class, String.class, Pojo.class)); + } + + @Test + public void publicStrNotAnnotated_transitionMode() { + parametersInterceptor.setRequireAnnotationsTransitionMode(Boolean.TRUE.toString()); + testParameter(new FieldAction(), "publicStrNotAnnotated", true); + } + + @Test + public void publicStrNotAnnotatedMethod_transitionMode() { + parametersInterceptor.setRequireAnnotationsTransitionMode(Boolean.TRUE.toString()); + testParameter(new MethodAction(), "publicStrNotAnnotated", true); + } + + + class FieldAction { + @StrutsParameter + private String privateStr; + + @StrutsParameter + public String publicStr; + + public String publicStrNotAnnotated; + + @StrutsParameter(depth = 1) + private Pojo privatePojo; + + @StrutsParameter + public Pojo publicPojoDepthZero; + + @StrutsParameter(depth = 1) + public Pojo publicPojoDepthOne ; + + @StrutsParameter(depth = 2) + public Pojo publicPojoDepthTwo; + + @StrutsParameter(depth = 1) + public List publicPojoListDepthOne; + + @StrutsParameter(depth = 2) + public List publicPojoListDepthTwo; + + @StrutsParameter(depth = 2) + public Map publicPojoMapDepthTwo; + } + + class MethodAction { + + @StrutsParameter + private void setPrivateStr(String str) { + } + + @StrutsParameter + public void setPublicStr(String str) { + } + + public void setPublicStrNotAnnotated(String str) { + } + + @StrutsParameter(depth = 1) + private Pojo getPrivatePojo() { + return null; + } + + @StrutsParameter + public Pojo getPublicPojoDepthZero() { + return null; + } + + @StrutsParameter + public void setPublicPojoDepthZero() { + } + + @StrutsParameter(depth = 1) + public Pojo getPublicPojoDepthOne() { + return null; + } + + @StrutsParameter(depth = 2) + public Pojo getPublicPojoDepthTwo() { + return null; + } + + @StrutsParameter(depth = 1) + public List getPublicPojoListDepthOne() { + return null; + } + + @StrutsParameter(depth = 2) + public List getPublicPojoListDepthTwo() { + return null; + } + + @StrutsParameter(depth = 2) + public Map getPublicPojoMapDepthTwo() { + return null; + } + } + + class Pojo { + } +} diff --git a/core/src/test/java/org/apache/struts2/views/jsp/IteratorTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/IteratorTagTest.java index ad3f68745..3c355bf1d 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/IteratorTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/IteratorTagTest.java @@ -18,6 +18,12 @@ */ package org.apache.struts2.views.jsp; +import jakarta.servlet.jsp.JspException; +import jakarta.servlet.jsp.tagext.TagSupport; +import org.apache.commons.collections.ListUtils; +import org.springframework.mock.web.MockBodyContent; +import org.springframework.mock.web.MockJspWriter; + import java.io.StringWriter; import java.io.Writer; import java.util.ArrayList; @@ -25,27 +31,19 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; -import org.apache.commons.collections.ListUtils; -import org.springframework.mock.web.MockBodyContent; -import org.springframework.mock.web.MockJspWriter; - -import jakarta.servlet.jsp.JspException; -import jakarta.servlet.jsp.tagext.TagSupport; - /** * Test Case for Iterator Tag - * */ public class IteratorTagTest extends AbstractUITagTest { - IteratorTag tag; - + private IteratorTag tag; public void testIteratingWithIdSpecified() throws Exception { - List list = new ArrayList(); + List list = new ArrayList<>(); list.add("one"); list.add("two"); list.add("three"); @@ -107,12 +105,12 @@ public class IteratorTagTest extends AbstractUITagTest { IteratorTag freshTag = new IteratorTag(); freshTag.setPageContext(pageContext); assertFalse("Tag state after doEndTag() under default tag clear state is equal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testIteratingWithIdSpecified_clearTagStateSet() throws Exception { - List list = new ArrayList(); + List list = new ArrayList<>(); list.add("one"); list.add("two"); list.add("three"); @@ -177,12 +175,12 @@ public class IteratorTagTest extends AbstractUITagTest { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testIteratingWithIdSpecifiedAndNullElementOnCollection() throws Exception { - List list = new ArrayList(); + List list = new ArrayList<>(); list.add("one"); list.add(null); list.add("three"); @@ -227,12 +225,12 @@ public class IteratorTagTest extends AbstractUITagTest { IteratorTag freshTag = new IteratorTag(); freshTag.setPageContext(pageContext); assertFalse("Tag state after doEndTag() under default tag clear state is equal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testIteratingWithIdSpecifiedAndNullElementOnCollection_clearTagStateSet() throws Exception { - List list = new ArrayList(); + List list = new ArrayList<>(); list.add("one"); list.add(null); list.add("three"); @@ -280,7 +278,7 @@ public class IteratorTagTest extends AbstractUITagTest { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -297,7 +295,7 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCollectionIterator() { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + List list = new ArrayList<>(); list.add("test1"); list.add("test2"); list.add("test3"); @@ -317,7 +315,7 @@ public class IteratorTagTest extends AbstractUITagTest { public void testMapIterator() { Foo foo = new Foo(); - HashMap map = new HashMap(); + HashMap map = new HashMap<>(); map.put("test1", "123"); map.put("test2", "456"); map.put("test3", "789"); @@ -332,8 +330,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doStartTag(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_INCLUDE, result); @@ -343,8 +340,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_AGAIN, result); @@ -354,8 +350,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_AGAIN, result); @@ -365,8 +360,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.SKIP_BODY, result); @@ -375,8 +369,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doEndTag(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_PAGE, result); @@ -385,13 +378,13 @@ public class IteratorTagTest extends AbstractUITagTest { IteratorTag freshTag = new IteratorTag(); freshTag.setPageContext(pageContext); assertFalse("Tag state after doEndTag() under default tag clear state is equal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } public void testMapIterator_clearTagStateSet() { Foo foo = new Foo(); - HashMap map = new HashMap(); + HashMap map = new HashMap<>(); map.put("test1", "123"); map.put("test2", "456"); map.put("test3", "789"); @@ -408,8 +401,7 @@ public class IteratorTagTest extends AbstractUITagTest { result = tag.doStartTag(); setComponentTagClearTagState(tag, true); // Ensure component tag state clearing is set true (to match tag). } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_INCLUDE, result); @@ -419,8 +411,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_AGAIN, result); @@ -430,8 +421,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_AGAIN, result); @@ -441,8 +431,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.SKIP_BODY, result); @@ -451,8 +440,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doEndTag(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_PAGE, result); @@ -462,7 +450,7 @@ public class IteratorTagTest extends AbstractUITagTest { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -480,8 +468,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doStartTag(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_INCLUDE, result); @@ -493,15 +480,16 @@ public class IteratorTagTest extends AbstractUITagTest { assertFalse(status.isLast()); assertTrue(status.isFirst()); assertEquals(0, status.getIndex()); + assertEquals("0", status.getIndexStr()); assertEquals(1, status.getCount()); + assertEquals("1", status.getCountStr()); assertTrue(status.isOdd()); assertFalse(status.isEven()); try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_AGAIN, result); @@ -520,8 +508,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_AGAIN, result); @@ -540,8 +527,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doEndTag(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_PAGE, result); @@ -550,7 +536,7 @@ public class IteratorTagTest extends AbstractUITagTest { IteratorTag freshTag = new IteratorTag(); freshTag.setPageContext(pageContext); assertFalse("Tag state after doEndTag() under default tag clear state is equal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -570,8 +556,7 @@ public class IteratorTagTest extends AbstractUITagTest { result = tag.doStartTag(); setComponentTagClearTagState(tag, true); // Ensure component tag state clearing is set true (to match tag). } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_INCLUDE, result); @@ -590,8 +575,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_AGAIN, result); @@ -610,8 +594,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_AGAIN, result); @@ -630,8 +613,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doEndTag(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_PAGE, result); @@ -641,7 +623,7 @@ public class IteratorTagTest extends AbstractUITagTest { freshTag.setPerformClearTagStateForTagPoolingServers(true); freshTag.setPageContext(pageContext); assertTrue("Tag state after doEndTag() and explicit tag state clearing is inequal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } @@ -669,7 +651,7 @@ public class IteratorTagTest extends AbstractUITagTest { public void testEmptyCollection() { Foo foo = new Foo(); - foo.setList(new ArrayList()); + foo.setList(new ArrayList<>()); stack.push(foo); @@ -695,17 +677,41 @@ public class IteratorTagTest extends AbstractUITagTest { validateCounter(new Integer[]{0, 1, 2, 3, 4, 5}); } - public void testCounterWithStackValues() throws JspException { + public void testCounterWithDifferentLocale() throws JspException { + stack.getActionContext().withLocale(new Locale("fa_IR")); + tag.setVar("it"); + tag.setBegin("0"); + tag.setEnd("5"); + List expectedValues = Arrays.asList("0", "1", "2", "3", "4", "5"); + + ArrayList values = new ArrayList<>(); + try { + int result = tag.doStartTag(); + assertEquals(TagSupport.EVAL_BODY_INCLUDE, result); + values.add((String) stack.findValue("it", String.class)); + } catch (JspException e) { + fail(e.getMessage()); + } + + while (tag.doAfterBody() == TagSupport.EVAL_BODY_AGAIN) { + values.add((String) stack.findValue("top", String.class)); + } + + assertEquals(expectedValues.size(), values.size()); + assertEquals(expectedValues, values); + } + + public void testCounterWithStackValues() throws JspException { stack.getContext().put("begin", 0); stack.getContext().put("end", 5); - tag.setBegin("%{#begin}"); - tag.setEnd("%{#end}"); + tag.setBegin("begin"); + tag.setEnd("end"); validateCounter(new Integer[]{0, 1, 2, 3, 4, 5}); } public void testCounterWithList() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + ArrayList list = new ArrayList<>(); list.add("a"); list.add("b"); list.add("c"); @@ -723,7 +729,6 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithArray() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); foo.setArray(new String[]{"a", "b", "c", "d"}); stack.push(foo); @@ -738,7 +743,7 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithListNoEnd() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + ArrayList list = new ArrayList<>(); list.add("a"); list.add("b"); list.add("c"); @@ -755,7 +760,6 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithArrayNoEnd() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); foo.setArray(new String[]{"a", "b", "c", "d"}); stack.push(foo); @@ -768,7 +772,7 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithList2() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + ArrayList list = new ArrayList<>(); list.add("a"); list.add("b"); list.add("c"); @@ -786,7 +790,6 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithArray2() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); foo.setArray(new String[]{"a", "b", "c", "d"}); stack.push(foo); @@ -800,7 +803,7 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithListNoEnd2() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + ArrayList list = new ArrayList<>(); list.add("a"); list.add("b"); list.add("c"); @@ -815,9 +818,8 @@ public class IteratorTagTest extends AbstractUITagTest { validateCounter(new String[]{"c", "d"}); } - public void testCounterWithArrayNoEnd2() throws JspException { + public void testCounterWithArrayNoEnd2() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); foo.setArray(new String[]{"a", "b", "c", "d"}); stack.push(foo); @@ -841,9 +843,9 @@ public class IteratorTagTest extends AbstractUITagTest { validateCounter(new Integer[]{0, 2, 4}); } - public void testCounterWithListAndStep() throws JspException { + public void testCounterWithListAndStep() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + ArrayList list = new ArrayList<>(); list.add("a"); list.add("b"); list.add("c"); @@ -861,9 +863,8 @@ public class IteratorTagTest extends AbstractUITagTest { validateCounter(new String[]{"a", "c"}); } - public void testCounterWithArrayAndStep() throws JspException { + public void testCounterWithArrayAndStep() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); foo.setArray(new String[]{"a", "b", "c", "d"}); stack.push(foo); @@ -879,7 +880,7 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithListAndStepNoEnd() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + ArrayList list = new ArrayList<>(); list.add("a"); list.add("b"); list.add("c"); @@ -898,7 +899,6 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithArrayAndStepNoEnd() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); foo.setArray(new String[]{"a", "b", "c", "d"}); stack.push(foo); @@ -920,7 +920,7 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithListAndNegativeStep() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + ArrayList list = new ArrayList<>(); list.add("a"); list.add("b"); list.add("c"); @@ -940,7 +940,7 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithListAndNegativeStepNoEnd() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + ArrayList list = new ArrayList<>(); list.add("a"); list.add("b"); list.add("c"); @@ -957,9 +957,9 @@ public class IteratorTagTest extends AbstractUITagTest { validateCounter(new String[]{"d", "c", "b", "a"}); } - public void testCounterWithArrayAndNegativeStep() throws JspException { + public void testCounterWithArrayAndNegativeStep() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + ArrayList list = new ArrayList<>(); list.add("a"); list.add("b"); list.add("c"); @@ -979,7 +979,7 @@ public class IteratorTagTest extends AbstractUITagTest { public void testCounterWithArrayAndNegativeStepNoEnd() throws JspException { Foo foo = new Foo(); - ArrayList list = new ArrayList(); + ArrayList list = new ArrayList<>(); list.add("a"); list.add("b"); list.add("c"); @@ -997,14 +997,13 @@ public class IteratorTagTest extends AbstractUITagTest { } protected void validateCounter(Object[] expectedValues) throws JspException { - List values = new ArrayList(); + ArrayList values = new ArrayList<>(); try { int result = tag.doStartTag(); assertEquals(TagSupport.EVAL_BODY_INCLUDE, result); values.add(stack.getRoot().peek()); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } while (tag.doAfterBody() == TagSupport.EVAL_BODY_AGAIN) { @@ -1012,7 +1011,7 @@ public class IteratorTagTest extends AbstractUITagTest { } assertEquals(expectedValues.length, values.size()); - ListUtils.isEqualList(Arrays.asList(expectedValues), values); + assertTrue(ListUtils.isEqualList(Arrays.asList(expectedValues), values)); } @Override @@ -1035,8 +1034,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doStartTag(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_INCLUDE, result); @@ -1046,8 +1044,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_AGAIN, result); @@ -1057,8 +1054,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_BODY_AGAIN, result); @@ -1068,8 +1064,7 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doAfterBody(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.SKIP_BODY, result); @@ -1082,16 +1077,14 @@ public class IteratorTagTest extends AbstractUITagTest { try { result = tag.doStartTag(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.SKIP_BODY, result); try { result = tag.doEndTag(); } catch (JspException e) { - e.printStackTrace(); - fail(); + fail(e.getMessage()); } assertEquals(TagSupport.EVAL_PAGE, result); @@ -1100,13 +1093,13 @@ public class IteratorTagTest extends AbstractUITagTest { IteratorTag freshTag = new IteratorTag(); freshTag.setPageContext(pageContext); assertFalse("Tag state after doEndTag() under default tag clear state is equal to new Tag with pageContext/parent set. " + - "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", + "May indicate that clearTagStateForTagPoolingServers() calls are not working properly.", strutsBodyTagsAreReflectionEqual(tag, freshTag)); } - class Foo { - private Collection list; - private Map map; + static class Foo { + private Collection list; + private Map map; private String[] array; public void setArray(String[] array) { @@ -1117,19 +1110,19 @@ public class IteratorTagTest extends AbstractUITagTest { return array; } - public void setList(Collection list) { + public void setList(Collection list) { this.list = list; } - public Collection getList() { + public Collection getList() { return list; } - public void setMap(Map map) { + public void setMap(Map map) { this.map = map; } - public Map getMap() { + public Map getMap() { return map; } } diff --git a/src/etc/project-suppression.xml b/src/etc/project-suppression.xml index 38324163d..b0d5ebc95 100644 --- a/src/etc/project-suppression.xml +++ b/src/etc/project-suppression.xml @@ -132,21 +132,16 @@ ^pkg:maven/org\.codehaus\.plexus/plexus\-utils@.*$ cpe:/a:plexus-utils_project:plexus-utils + CVE-2022-4244 + CVE-2022-4245 + CVE-2017-1000487 - - ^pkg:maven/org\.codehaus\.plexus/plexus\-utils@.*$ - CVE-2017-1000487 - - - - ^pkg:maven/org\.codehaus\.plexus/plexus\-utils@.*$ - Directory traversal in org.codehaus.plexus.util.Expand - - - - ^pkg:maven/org\.codehaus\.plexus/plexus\-utils@.*$ - Possible XML Injection + + ^pkg:maven/org\.codehaus\.plexus\/plexus\-container\-default@.*$ + cpe:/a:plexus-utils_project:plexus-utils + CVE-2022-4244 + CVE-2022-4245