diff --git a/.asf.yaml b/.asf.yaml index 87706aa578..5e259c7103 100644 --- a/.asf.yaml +++ b/.asf.yaml @@ -18,8 +18,9 @@ github: contexts: - build required_pull_request_reviews: - require_code_owner_reviews: true - required_approving_review_count: 1 + # it does not work because our github teams are private/secret, see INFRA-25666 + require_code_owner_reviews: false + required_approving_review_count: 0 autolink_jira: - WW dependabot_alerts: true diff --git a/.github/workflows/scorecards-analysis.yaml b/.github/workflows/scorecards-analysis.yaml index 163cb2c443..4fa5e4890b 100644 --- a/.github/workflows/scorecards-analysis.yaml +++ b/.github/workflows/scorecards-analysis.yaml @@ -46,7 +46,7 @@ jobs: persist-credentials: false - name: "Run analysis" - uses: ossf/scorecard-action@0864cf19026789058feabb7e87baa5f140aac736 # 2.3.1 + uses: ossf/scorecard-action@dc50aa9510b46c811795eb24b2f1ba02a914e534 # 2.3.3 with: results_file: results.sarif results_format: sarif @@ -58,7 +58,7 @@ jobs: publish_results: true - name: "Upload artifact" - uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # 4.3.1 + uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # 4.3.3 with: name: SARIF file path: results.sarif diff --git a/apps/showcase/pom.xml b/apps/showcase/pom.xml index e2ed9f10a0..3219ef6944 100644 --- a/apps/showcase/pom.xml +++ b/apps/showcase/pom.xml @@ -197,7 +197,7 @@ org.apache.maven.plugins maven-failsafe-plugin - 3.0.0-M6 + 3.2.5 it.org.apache.struts2.showcase.*Test diff --git a/assembly/pom.xml b/assembly/pom.xml index 94677eed6f..af0a5c03e3 100644 --- a/assembly/pom.xml +++ b/assembly/pom.xml @@ -106,7 +106,7 @@ org.apache.maven.plugins maven-assembly-plugin - 3.6.0 + 3.7.1 make-assembly diff --git a/core/pom.xml b/core/pom.xml index b76bb76159..66e3a89ff0 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -220,6 +220,12 @@ test + + org.awaitility + awaitility + test + + junit junit diff --git a/core/src/main/java/com/opensymphony/xwork2/ActionSupport.java b/core/src/main/java/com/opensymphony/xwork2/ActionSupport.java index 8c7e15e609..ab1a18099a 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ActionSupport.java +++ b/core/src/main/java/com/opensymphony/xwork2/ActionSupport.java @@ -90,6 +90,11 @@ public boolean isValidLocale(Locale locale) { return getLocaleProvider().isValidLocale(locale); } + @Override + public Locale toLocale(String localeStr) { + return getLocaleProvider().toLocale(localeStr); + } + @Override public boolean hasKey(String key) { return getTextProvider().hasKey(key); diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultLocaleProvider.java b/core/src/main/java/com/opensymphony/xwork2/DefaultLocaleProvider.java index da89306c0c..35f16191a8 100644 --- a/core/src/main/java/com/opensymphony/xwork2/DefaultLocaleProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/DefaultLocaleProvider.java @@ -46,17 +46,23 @@ public Locale getLocale() { @Override public boolean isValidLocaleString(String localeStr) { + Locale locale = this.toLocale(localeStr); + return isValidLocale(locale); + } + + @Override + public boolean isValidLocale(Locale locale) { + return locale != null && LocaleUtils.isAvailableLocale(locale); + } + + @Override + public Locale toLocale(String localeStr) { Locale locale = null; try { locale = LocaleUtils.toLocale(StringUtils.trimToNull(localeStr)); } catch (IllegalArgumentException e) { LOG.warn(new ParameterizedMessage("Cannot convert [{}] to proper locale", localeStr), e); } - return isValidLocale(locale); - } - - @Override - public boolean isValidLocale(Locale locale) { - return LocaleUtils.isAvailableLocale(locale); + return locale; } } diff --git a/core/src/main/java/com/opensymphony/xwork2/LocaleProvider.java b/core/src/main/java/com/opensymphony/xwork2/LocaleProvider.java index 67972af341..00a41a25b3 100644 --- a/core/src/main/java/com/opensymphony/xwork2/LocaleProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/LocaleProvider.java @@ -18,6 +18,9 @@ */ package com.opensymphony.xwork2; +import org.apache.commons.lang3.LocaleUtils; +import org.apache.commons.lang3.StringUtils; + import java.util.Locale; @@ -58,4 +61,17 @@ public interface LocaleProvider { */ boolean isValidLocale(Locale locale); + /** + * Tries to convert provided locale string into {@link Locale} or returns null + * @param localeStr a String representing locale, e.g.: en_EN + * @return instance of {@link Locale} or null + * @since Struts 6.5.0 + */ + default Locale toLocale(String localeStr) { + try { + return LocaleUtils.toLocale(StringUtils.trimToNull(localeStr)); + } catch (IllegalArgumentException e) { + return null; + } + } } 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 43ae992405..f882b2c583 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -29,6 +29,7 @@ import org.apache.struts2.ognl.ThreadAllowlist; import java.lang.reflect.AccessibleObject; +import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Member; import java.lang.reflect.Modifier; @@ -147,11 +148,11 @@ public boolean isAccessible(Map context, Object target, Member member, String pr if (target != null) { // Special case: Target is a Class object but not Class.class if (Class.class.equals(target.getClass()) && !Class.class.equals(target)) { - if (!isStatic(member)) { - throw new IllegalArgumentException("Member expected to be static!"); + if (!isStatic(member) && !Constructor.class.equals(member.getClass())) { + throw new IllegalArgumentException("Member expected to be static or constructor!"); } if (!member.getDeclaringClass().equals(target)) { - throw new IllegalArgumentException("Target class does not match static member!"); + throw new IllegalArgumentException("Target class does not match member!"); } target = null; // This information is not useful to us and conflicts with following logic which expects target to be null or an instance containing the member // Standard case: Member should exist on target diff --git a/core/src/main/java/com/opensymphony/xwork2/validator/ActionValidatorManager.java b/core/src/main/java/com/opensymphony/xwork2/validator/ActionValidatorManager.java index d78224b264..ea0de3a724 100644 --- a/core/src/main/java/com/opensymphony/xwork2/validator/ActionValidatorManager.java +++ b/core/src/main/java/com/opensymphony/xwork2/validator/ActionValidatorManager.java @@ -36,7 +36,7 @@ public interface ActionValidatorManager { * @param method the name of the method being invoked on the action - can be null. * @return a list of all validators for the given class and context. */ - List getValidators(Class clazz, String context, String method); + List getValidators(Class clazz, String context, String method); /** * Returns a list of validators for the given class and context. This is the primary @@ -46,7 +46,7 @@ public interface ActionValidatorManager { * @param context the context of the action class - can be null. * @return a list of all validators for the given class and context. */ - List getValidators(Class clazz, String context); + List getValidators(Class clazz, String context); /** * Validates the given object using action and its context. diff --git a/core/src/main/java/com/opensymphony/xwork2/validator/DefaultActionValidatorManager.java b/core/src/main/java/com/opensymphony/xwork2/validator/DefaultActionValidatorManager.java index 3af54669ef..f05b804860 100644 --- a/core/src/main/java/com/opensymphony/xwork2/validator/DefaultActionValidatorManager.java +++ b/core/src/main/java/com/opensymphony/xwork2/validator/DefaultActionValidatorManager.java @@ -125,7 +125,7 @@ public void validate(Object object, String context, ValidatorContext validatorCo * @param context context * @return a validator key which is the class name plus context. */ - protected String buildValidatorKey(Class clazz, String context) { + protected String buildValidatorKey(Class clazz, String context) { return clazz.getName() + "/" + context; } @@ -137,7 +137,7 @@ protected Validator getValidatorFromValidatorConfig(ValidatorConfig config, Valu } @Override - public synchronized List getValidators(Class clazz, String context, String method) { + public synchronized List getValidators(Class clazz, String context, String method) { String validatorKey = buildValidatorKey(clazz, context); if (!validatorCache.containsKey(validatorKey)) { @@ -158,7 +158,7 @@ public synchronized List getValidators(Class clazz, String context, S } @Override - public synchronized List getValidators(Class clazz, String context) { + public synchronized List getValidators(Class clazz, String context) { return getValidators(clazz, context, null); } @@ -277,7 +277,7 @@ public void validate(Object object, String context, ValidatorContext validatorCo * @param checked the set of previously checked class-contexts, null if none have been checked * @return a list of validator configs for the given class and context. */ - protected List buildValidatorConfigs(Class clazz, String context, boolean checkFile, Set checked) { + protected List buildValidatorConfigs(Class clazz, String context, boolean checkFile, Set checked) { List validatorConfigs = new ArrayList<>(); if (checked == null) { @@ -287,7 +287,7 @@ protected List buildValidatorConfigs(Class clazz, String contex } if (clazz.isInterface()) { - for (Class anInterface : clazz.getInterfaces()) { + for (Class anInterface : clazz.getInterfaces()) { validatorConfigs.addAll(buildValidatorConfigs(anInterface, context, checkFile, checked)); } } else { @@ -297,7 +297,7 @@ protected List buildValidatorConfigs(Class clazz, String contex } // look for validators for implemented interfaces - for (Class anInterface1 : clazz.getInterfaces()) { + for (Class anInterface1 : clazz.getInterfaces()) { if (checked.contains(anInterface1.getName())) { continue; } @@ -317,17 +317,17 @@ protected List buildValidatorConfigs(Class clazz, String contex return validatorConfigs; } - protected List buildAliasValidatorConfigs(Class aClass, String context, boolean checkFile) { + protected List buildAliasValidatorConfigs(Class aClass, String context, boolean checkFile) { String fileName = aClass.getName().replace('.', '/') + "-" + context + VALIDATION_CONFIG_SUFFIX; return loadFile(fileName, aClass, checkFile); } - protected List buildClassValidatorConfigs(Class aClass, boolean checkFile) { + protected List buildClassValidatorConfigs(Class aClass, boolean checkFile) { String fileName = aClass.getName().replace('.', '/') + VALIDATION_CONFIG_SUFFIX; return loadFile(fileName, aClass, checkFile); } - protected List loadFile(String fileName, Class clazz, boolean checkFile) { + protected List loadFile(String fileName, Class clazz, boolean checkFile) { List retList = Collections.emptyList(); URL fileUrl = ClassLoaderUtil.getResource(fileName, clazz); diff --git a/core/src/main/java/com/opensymphony/xwork2/validator/DelegatingValidatorContext.java b/core/src/main/java/com/opensymphony/xwork2/validator/DelegatingValidatorContext.java index 5c7f2c136a..bc8c88875c 100644 --- a/core/src/main/java/com/opensymphony/xwork2/validator/DelegatingValidatorContext.java +++ b/core/src/main/java/com/opensymphony/xwork2/validator/DelegatingValidatorContext.java @@ -122,10 +122,15 @@ public boolean isValidLocale(Locale locale) { return localeProvider.isValidLocale(locale); } + @Override + public Locale toLocale(String localeStr) { + return localeProvider.toLocale(localeStr); + } + public boolean hasKey(String key) { return textProvider.hasKey(key); } - + public String getText(String aTextName) { return textProvider.getText(aTextName); } @@ -280,6 +285,11 @@ public boolean isValidLocaleString(String localeStr) { public boolean isValidLocale(Locale locale) { return getLocaleProvider().isValidLocale(locale); } + + @Override + public Locale toLocale(String localeStr) { + return getLocaleProvider().toLocale(localeStr); + } } /** diff --git a/core/src/main/java/org/apache/struts2/components/Set.java b/core/src/main/java/org/apache/struts2/components/Set.java index cca990ea84..4d8f7c8981 100644 --- a/core/src/main/java/org/apache/struts2/components/Set.java +++ b/core/src/main/java/org/apache/struts2/components/Set.java @@ -104,17 +104,17 @@ public boolean end(Writer writer, String body) { body=""; if (DispatcherConstants.APPLICATION.equalsIgnoreCase(scope)) { - stack.setValue("#application['" + getVar() + "']", o); + stack.setValue(String.format("#application[\"%s\"]", getVar()), o); } else if (DispatcherConstants.SESSION.equalsIgnoreCase(scope)) { - stack.setValue("#session['" + getVar() + "']", o); + stack.setValue(String.format("#session[\"%s\"]", getVar()), o); } else if (DispatcherConstants.REQUEST.equalsIgnoreCase(scope)) { - stack.setValue("#request['" + getVar() + "']", o); + stack.setValue(String.format("#request[\"%s\"]", getVar()), o); } else if (DispatcherConstants.PAGE.equalsIgnoreCase(scope)) { - stack.setValue("#attr['" + getVar() + "']", o, false); + stack.setValue(String.format("#attr[\"%s\"]", getVar()), o, false); } else { // Default scope is action. Note: The action scope handling also adds the var to the page scope. - stack.getContext().put(getVar(), o); - stack.setValue("#attr['" + getVar() + "']", o, false); + putInContext(o); + stack.setValue(String.format("#attr[\"%s\"]", getVar()), o, false); } return super.end(writer, body); diff --git a/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java index e64d891b07..6f4bae0e82 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java @@ -24,7 +24,6 @@ import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.interceptor.AbstractInterceptor; import com.opensymphony.xwork2.util.TextParseUtil; -import org.apache.commons.lang3.LocaleUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; @@ -85,7 +84,7 @@ public void setRequestCookieParameterName(String requestCookieParameterName) { } public void setLocaleStorage(String storageName) { - if (storageName == null || "".equals(storageName)) { + if (storageName == null || storageName.isEmpty()) { this.storage = Storage.ACCEPT_LANGUAGE; } else { try { @@ -169,27 +168,21 @@ protected LocaleHandler getLocaleHandler(ActionInvocation invocation) { } /** - * Creates a Locale object from the request param, which might - * be already a Local or a String + * Creates a Locale object from the request param * * @param requestedLocale the parameter from the request - * @return the Locale + * @return instance of {@link Locale} or null */ - protected Locale getLocaleFromParam(Object requestedLocale) { + protected Locale getLocaleFromParam(String requestedLocale) { LocaleProvider localeProvider = localeProviderFactory.createLocaleProvider(); Locale locale = null; if (requestedLocale != null) { - if (requestedLocale instanceof Locale) { - locale = (Locale) requestedLocale; - } else { - String localeStr = requestedLocale.toString(); - if (localeProvider.isValidLocaleString(localeStr)) { - locale = LocaleUtils.toLocale(localeStr); - } else { - locale = localeProvider.getLocale(); - } + locale = localeProvider.toLocale(requestedLocale); + if (locale == null) { + locale = localeProvider.getLocale(); } + if (locale != null) { LOG.debug("Found locale: {}", locale); } @@ -285,7 +278,7 @@ protected AcceptLanguageLocaleHandler(ActionInvocation invocation) { @Override @SuppressWarnings("rawtypes") public Locale find() { - if (supportedLocale.size() > 0) { + if (!supportedLocale.isEmpty()) { Enumeration locales = actionInvocation.getInvocationContext().getServletRequest().getLocales(); while (locales.hasMoreElements()) { Locale locale = (Locale) locales.nextElement(); diff --git a/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java index 58ef078c6c..a693dab5a9 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java @@ -19,7 +19,9 @@ package org.apache.struts2.interceptor.csp; import com.opensymphony.xwork2.ActionInvocation; +import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.interceptor.AbstractInterceptor; +import com.opensymphony.xwork2.util.ClassLoaderUtil; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.action.CspSettingsAware; @@ -46,6 +48,9 @@ public final class CspInterceptor extends AbstractInterceptor { private boolean prependServletContext = true; private boolean enforcingMode; private String reportUri; + private String reportTo; + + private String cspSettingsClassName = DefaultCspSettings.class.getName(); @Override public String intercept(ActionInvocation invocation) throws Exception { @@ -54,12 +59,30 @@ public String intercept(ActionInvocation invocation) throws Exception { LOG.trace("Using CspSettings provided by the action: {}", action); applySettings(invocation, ((CspSettingsAware) action).getCspSettings()); } else { - LOG.trace("Using DefaultCspSettings with action: {}", action); - applySettings(invocation, new DefaultCspSettings()); + LOG.trace("Using {} with action: {}", cspSettingsClassName, action); + CspSettings cspSettings = createCspSettings(invocation); + applySettings(invocation, cspSettings); } return invocation.invoke(); } + private CspSettings createCspSettings(ActionInvocation invocation) throws ClassNotFoundException { + Class cspSettingsClass; + + try { + cspSettingsClass = ClassLoaderUtil.loadClass(cspSettingsClassName, getClass()); + } catch (ClassNotFoundException e) { + throw new ConfigurationException(String.format("The class %s doesn't exist!", cspSettingsClassName)); + } + + if (!CspSettings.class.isAssignableFrom(cspSettingsClass)) { + throw new ConfigurationException(String.format("The class %s doesn't implement %s!", + cspSettingsClassName, CspSettings.class.getName())); + } + + return (CspSettings) invocation.getInvocationContext().getContainer().inject(cspSettingsClass); + } + private void applySettings(ActionInvocation invocation, CspSettings cspSettings) { HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); HttpServletResponse response = invocation.getInvocationContext().getServletResponse(); @@ -76,6 +99,12 @@ private void applySettings(ActionInvocation invocation, CspSettings cspSettings) } cspSettings.setReportUri(finalReportUri); + + // apply reportTo if set + if (reportTo != null) { + LOG.trace("Applying: {} to reportTo", reportTo); + cspSettings.setReportTo(reportTo); + } } invocation.addPreResultListener((actionInvocation, resultCode) -> { @@ -97,6 +126,17 @@ public void setReportUri(String reportUri) { this.reportUri = reportUri; } + /** + * Sets the report group where csp violation reports will be sent. This will + * only be used if the reportUri is set. + * + * @param reportTo the report group where csp violation reports will be sent + * @since Struts 6.5.0 + */ + public void setReportTo(String reportTo) { + this.reportTo = reportTo; + } + private Optional buildUri(String reportUri) { try { return Optional.of(URI.create(reportUri)); @@ -124,4 +164,13 @@ public void setPrependServletContext(boolean prependServletContext) { this.prependServletContext = prependServletContext; } + /** + * Sets the class name of the default {@link CspSettings} implementation to use when the action does not + * set its own values. If not set, the default is {@link DefaultCspSettings}. + * + * @since Struts 6.5.0 + */ + public void setCspSettingsClassName(String cspSettingsClassName) { + this.cspSettingsClassName = cspSettingsClassName; + } } diff --git a/core/src/main/java/org/apache/struts2/interceptor/csp/CspSettings.java b/core/src/main/java/org/apache/struts2/interceptor/csp/CspSettings.java index cf970abb59..3132b9d6a8 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/csp/CspSettings.java +++ b/core/src/main/java/org/apache/struts2/interceptor/csp/CspSettings.java @@ -37,6 +37,7 @@ public interface CspSettings { String SCRIPT_SRC = "script-src"; String BASE_URI = "base-uri"; String REPORT_URI = "report-uri"; + String REPORT_TO = "report-to"; String NONE = "none"; String STRICT_DYNAMIC = "strict-dynamic"; String HTTP = "http:"; @@ -56,6 +57,13 @@ public interface CspSettings { */ void setReportUri(String uri); + /** + * Sets the report group where csp violation reports will be sent + * + * @since Struts 6.5.0 + */ + void setReportTo(String group); + /** * Sets CSP headers in enforcing mode when true, and report-only when false */ diff --git a/core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java b/core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java index 9e5b6180ad..338a29914f 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java +++ b/core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java @@ -20,6 +20,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.struts2.action.CspSettingsAware; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; @@ -31,7 +32,11 @@ /** * Default implementation of {@link CspSettings}. - * The default policy implements strict CSP with a nonce based approach and follows the guide: https://csp.withgoogle.com/docs/index.html/ + * The default policy implements strict CSP with a nonce based approach and follows the guide: + * https://csp.withgoogle.com/docs/index.html/ + * You may extend or replace this class if you wish to customize the default policy further, and use your class + * by setting the {@link CspInterceptor} defaultCspSettingsClassName parameter. Actions that + * implement the {@link CspSettingsAware} interface will ignore the defaultCspSettingsClassName parameter. * * @see CspSettings * @see CspInterceptor @@ -42,20 +47,22 @@ public class DefaultCspSettings implements CspSettings { private final SecureRandom sRand = new SecureRandom(); - private String reportUri; + protected String reportUri; + protected String reportTo; // default to reporting mode - private String cspHeader = CSP_REPORT_HEADER; + protected String cspHeader = CSP_REPORT_HEADER; @Override public void addCspHeaders(HttpServletResponse response) { throw new UnsupportedOperationException("Unsupported implementation, use #addCspHeaders(HttpServletRequest request, HttpServletResponse response)"); } + @Override public void addCspHeaders(HttpServletRequest request, HttpServletResponse response) { if (isSessionActive(request)) { LOG.trace("Session is active, applying CSP settings"); associateNonceWithSession(request); - response.setHeader(cspHeader, cratePolicyFormat(request)); + response.setHeader(cspHeader, createPolicyFormat(request)); } else { LOG.trace("Session is not active, ignoring CSP settings"); } @@ -70,7 +77,7 @@ private void associateNonceWithSession(HttpServletRequest request) { request.getSession().setAttribute("nonce", nonceValue); } - private String cratePolicyFormat(HttpServletRequest request) { + protected String createPolicyFormat(HttpServletRequest request) { StringBuilder policyFormatBuilder = new StringBuilder() .append(OBJECT_SRC) .append(format(" '%s'; ", NONE)) @@ -84,13 +91,18 @@ private String cratePolicyFormat(HttpServletRequest request) { if (reportUri != null) { policyFormatBuilder .append(REPORT_URI) - .append(format(" %s", reportUri)); + .append(format(" %s; ", reportUri)); + if(reportTo != null) { + policyFormatBuilder + .append(REPORT_TO) + .append(format(" %s; ", reportTo)); + } } return format(policyFormatBuilder.toString(), getNonceString(request)); } - private String getNonceString(HttpServletRequest request) { + protected String getNonceString(HttpServletRequest request) { Object nonce = request.getSession().getAttribute("nonce"); return Objects.toString(nonce); } @@ -101,20 +113,28 @@ private byte[] getRandomBytes() { return ret; } + @Override public void setEnforcingMode(boolean enforcingMode) { if (enforcingMode) { cspHeader = CSP_ENFORCE_HEADER; } } + @Override public void setReportUri(String reportUri) { this.reportUri = reportUri; } + @Override + public void setReportTo(String reportTo) { + this.reportTo = reportTo; + } + @Override public String toString() { return "DefaultCspSettings{" + "reportUri='" + reportUri + '\'' + + ", reportTo='" + reportTo + '\'' + ", cspHeader='" + cspHeader + '\'' + '}'; } diff --git a/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java b/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java index 42237262f8..b385c9b8fa 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java +++ b/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java @@ -20,6 +20,8 @@ import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionInvocation; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import java.io.Serializable; @@ -30,6 +32,8 @@ public class StrutsBackgroundProcess implements BackgroundProcess, Serializable private static final long serialVersionUID = 3884464776311686443L; + private static final Logger LOG = LogManager.getLogger(StrutsBackgroundProcess.class); + private final String threadName; private final int threadPriority; @@ -44,8 +48,8 @@ public class StrutsBackgroundProcess implements BackgroundProcess, Serializable /** * Constructs a background process * - * @param invocation The action invocation - * @param threadName The name of background thread + * @param invocation The action invocation + * @param threadName The name of background thread * @param threadPriority The priority of background thread */ public StrutsBackgroundProcess(ActionInvocation invocation, String threadName, int threadPriority) { @@ -61,11 +65,19 @@ public BackgroundProcess prepare() { try { beforeInvocation(); result = invocation.invokeActionOnly(); - afterInvocation(); } catch (Exception e) { + LOG.warn("Exception during invokeActionOnly() execution", e); exception = e; } finally { - done = true; + try { + afterInvocation(); + } catch (Exception ex) { + if (exception == null) { + exception = ex; + } + LOG.warn("Exception during afterInvocation() execution", ex); + } + done = true; } }); processThread.setName(threadName); diff --git a/core/src/test/java/com/opensymphony/xwork2/DefaultLocaleProviderTest.java b/core/src/test/java/com/opensymphony/xwork2/DefaultLocaleProviderTest.java new file mode 100644 index 0000000000..bb5178abd9 --- /dev/null +++ b/core/src/test/java/com/opensymphony/xwork2/DefaultLocaleProviderTest.java @@ -0,0 +1,174 @@ +/* + * 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 com.opensymphony.xwork2; + +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.util.Locale; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +public class DefaultLocaleProviderTest { + + private DefaultLocaleProvider provider; + + @Before + public void setUp() throws Exception { + provider = new DefaultLocaleProvider(); + } + + @BeforeClass + public static void beforeClass() throws Exception { + ActionContext.of().bind(); + } + + @AfterClass + public static void afterClass() throws Exception { + ActionContext.clear(); + } + + @Test + public void getLocale() { + // given + ActionContext.getContext().withLocale(Locale.ITALY); + + // when + Locale actual = provider.getLocale(); + + // then + assertEquals(Locale.ITALY, actual); + } + + @Test + public void getLocaleNull() { + // given + ActionContext backup = ActionContext.getContext(); + ActionContext.clear(); + + // when + Locale actual = provider.getLocale(); + + // then + assertNull(actual); + ActionContext.bind(backup); + } + + @Test + public void toLocale() { + // given + ActionContext.getContext().withLocale(Locale.GERMAN); + + // when + Locale actual = provider.toLocale("it"); + + // then + assertEquals(Locale.ITALIAN, actual); + } + + @Test + public void toLocaleFull() { + // given + ActionContext.getContext().withLocale(Locale.GERMAN); + + // when + Locale actual = provider.toLocale("it_IT"); + + // then + assertEquals(Locale.ITALY, actual); + } + + @Test + public void toLocaleTrimEndOfLine() { + // given + ActionContext.getContext().withLocale(Locale.GERMAN); + + // when + Locale actual = provider.toLocale("it_IT\n"); + + // then + assertEquals(Locale.ITALY, actual); + } + + @Test + public void toLocaleTrimEmptySpace() { + // given + ActionContext.getContext().withLocale(Locale.GERMAN); + + // when + Locale actual = provider.toLocale(" it_IT "); + + // then + assertEquals(Locale.ITALY, actual); + } + + @Test + public void isValidLocaleNull() { + // given + ActionContext.getContext().withLocale(Locale.GERMAN); + + // when + boolean actual = provider.isValidLocale(null); + + // then + assertFalse(actual); + } + + @Test + public void isValidLocale() { + // given + ActionContext.getContext().withLocale(Locale.GERMAN); + + // when + boolean actual = provider.isValidLocale(Locale.ITALIAN); + + // then + assertTrue(actual); + } + + @Test + public void isValidLocaleString() { + // given + ActionContext.getContext().withLocale(Locale.GERMAN); + + // when + boolean actual = provider.isValidLocaleString("it"); + + // then + assertTrue(actual); + } + + @Test + public void isValidLocaleStringNot() { + // given + ActionContext.getContext().withLocale(Locale.GERMAN); + + // when + boolean actual = provider.isValidLocaleString("italy"); + + // then + assertFalse(actual); + } + +} diff --git a/core/src/test/java/com/opensymphony/xwork2/LocaleProviderTest.java b/core/src/test/java/com/opensymphony/xwork2/LocaleProviderTest.java new file mode 100644 index 0000000000..03e05f5c23 --- /dev/null +++ b/core/src/test/java/com/opensymphony/xwork2/LocaleProviderTest.java @@ -0,0 +1,82 @@ +/* + * 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 com.opensymphony.xwork2; + +import org.junit.Test; + +import java.util.Locale; + +import static org.junit.Assert.*; + +public class LocaleProviderTest { + + @Test + public void toLocale() { + // given + DummyLocale locale = new DummyLocale(); + + // when + Locale actual = locale.toLocale("de"); + + // then + assertEquals(Locale.GERMAN, actual); + } + + @Test + public void toLocaleTrim() { + // given + DummyLocale locale = new DummyLocale(); + + // when + Locale actual = locale.toLocale(" de_DE "); + + // then + assertEquals(Locale.GERMANY, actual); + } + + @Test + public void toLocaleNull() { + // given + DummyLocale locale = new DummyLocale(); + + // when + Locale actual = locale.toLocale("germany"); + + // then + assertNull(actual); + } + +} + +class DummyLocale implements LocaleProvider { + @Override + public Locale getLocale() { + return null; + } + + @Override + public boolean isValidLocaleString(String localeStr) { + return false; + } + + @Override + public boolean isValidLocale(Locale locale) { + return false; + } +} diff --git a/core/src/test/java/com/opensymphony/xwork2/validator/DefaultActionValidatorManagerTest.java b/core/src/test/java/com/opensymphony/xwork2/validator/DefaultActionValidatorManagerTest.java index c6f4b3d6f2..5014a374f7 100644 --- a/core/src/test/java/com/opensymphony/xwork2/validator/DefaultActionValidatorManagerTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/validator/DefaultActionValidatorManagerTest.java @@ -18,161 +18,167 @@ */ package com.opensymphony.xwork2.validator; -import com.mockobjects.dynamic.C; -import com.mockobjects.dynamic.Mock; -import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.FileManagerFactory; import com.opensymphony.xwork2.SimpleAction; -import com.opensymphony.xwork2.StubValueStack; import com.opensymphony.xwork2.TestBean; +import com.opensymphony.xwork2.ValidationOrderAction; import com.opensymphony.xwork2.XWorkTestCase; -import com.opensymphony.xwork2.config.ConfigurationException; +import com.opensymphony.xwork2.interceptor.ValidationAware; import com.opensymphony.xwork2.test.DataAware2; -import com.opensymphony.xwork2.test.SimpleAction2; import com.opensymphony.xwork2.test.SimpleAction3; -import com.opensymphony.xwork2.util.ValueStack; -import com.opensymphony.xwork2.util.fs.DefaultFileManager; -import com.opensymphony.xwork2.util.fs.DefaultFileManagerFactory; +import com.opensymphony.xwork2.test.User; +import com.opensymphony.xwork2.validator.validators.DateRangeFieldValidator; +import com.opensymphony.xwork2.validator.validators.DoubleRangeFieldValidator; +import com.opensymphony.xwork2.validator.validators.ExpressionValidator; +import com.opensymphony.xwork2.validator.validators.IntRangeFieldValidator; +import com.opensymphony.xwork2.validator.validators.LongRangeFieldValidator; +import com.opensymphony.xwork2.validator.validators.RequiredFieldValidator; +import com.opensymphony.xwork2.validator.validators.RequiredStringValidator; +import com.opensymphony.xwork2.validator.validators.ShortRangeFieldValidator; import org.apache.struts2.StrutsException; +import org.xml.sax.SAXParseException; import java.util.ArrayList; -import java.util.HashMap; +import java.util.Iterator; import java.util.List; +import java.util.Map; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; -/** - * DefaultActionValidatorManagerTest - * - * @author Jason Carreira - * @author tm_jee - * @version $Date$ $Id$ - */ public class DefaultActionValidatorManagerTest extends XWorkTestCase { protected final String alias = "validationAlias"; DefaultActionValidatorManager actionValidatorManager; - Mock mockValidatorFileParser; - Mock mockValidatorFactory; - ValueStack stubValueStack; @Override protected void setUp() throws Exception { - actionValidatorManager = new DefaultActionValidatorManager(); super.setUp(); - mockValidatorFileParser = new Mock(ValidatorFileParser.class); - actionValidatorManager.setValidatorFileParser((ValidatorFileParser)mockValidatorFileParser.proxy()); - - mockValidatorFactory = new Mock(ValidatorFactory.class); - actionValidatorManager.setValidatorFactory((ValidatorFactory)mockValidatorFactory.proxy()); - - stubValueStack = new StubValueStack(); - ActionContext.of() - .withValueStack(stubValueStack) - .bind(); - - DefaultFileManagerFactory factory = new DefaultFileManagerFactory(); - factory.setContainer(container); - factory.setFileManager(new DefaultFileManager()); - actionValidatorManager.setFileManagerFactory(factory); + actionValidatorManager = container.inject(DefaultActionValidatorManager.class); } @Override protected void tearDown() throws Exception { - actionValidatorManager = null; super.tearDown(); - mockValidatorFactory = null; - mockValidatorFileParser = null; + actionValidatorManager = null; } - public void testBuildValidatorKey() { String validatorKey = actionValidatorManager.buildValidatorKey(SimpleAction.class, alias); assertEquals(SimpleAction.class.getName() + "/" + alias, validatorKey); } public void testBuildsValidatorsForAlias() { - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/SimpleAction-validation.xml")), - new ArrayList()); - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/SimpleAction-validationAlias-validation.xml")), - new ArrayList()); - actionValidatorManager.getValidators(SimpleAction.class, alias); - mockValidatorFileParser.verify(); + List validators = actionValidatorManager.getValidators(SimpleAction.class, alias); + + assertThat(validators).hasSize(11).map(Validator::getClass).containsExactly( + ExpressionValidator.class, + RequiredFieldValidator.class, + IntRangeFieldValidator.class, + DoubleRangeFieldValidator.class, + DateRangeFieldValidator.class, + IntRangeFieldValidator.class, + IntRangeFieldValidator.class, + LongRangeFieldValidator.class, + ShortRangeFieldValidator.class, + RequiredFieldValidator.class, + IntRangeFieldValidator.class + ); + assertThat(validators).hasSize(11).map(Validator::getDefaultMessage).containsExactly( + "Foo must be greater than Bar. Foo = ${foo}, Bar = ${bar}.", + "You must enter a value for bar.", + "bar must be between ${min} and ${max}, current value is ${bar}.", + "percentage must be between ${minExclusive} and ${maxExclusive}, current value is ${percentage}.", + "The date must be between 12-22-2002 and 12-25-2002.", + "Could not find foo.range!", + "Could not find baz.range!", + "Could not find foo.range!", + "Could not find foo.range!", + "You must enter a value for baz.", + "baz out of range." + ); } public void testBuildsValidatorsForAliasError() { - boolean pass = false; - try { - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/TestBean-validation.xml")), - new ArrayList()); - mockValidatorFileParser.expectAndThrow("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/TestBean-badtest-validation.xml")), - new ConfigurationException()); - List validatorList = actionValidatorManager.getValidators(TestBean.class, "badtest"); - } catch (StrutsException ex) { - pass = true; - } - mockValidatorFileParser.verify(); - assertTrue("Didn't throw exception on load failure", pass); + assertThatThrownBy(() -> actionValidatorManager.getValidators(TestBean.class, "badtest")) + .isInstanceOf(StrutsException.class) + .hasCause(new SAXParseException("Attribute \"foo\" must be declared for element type \"field-validator\".", null)); } public void testGetValidatorsForInterface() { - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/test/DataAware-validation.xml")), - new ArrayList()); - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/test/DataAware-validationAlias-validation.xml")), - new ArrayList()); - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/test/DataAware2-validation.xml")), - new ArrayList()); - actionValidatorManager.getValidators(DataAware2.class, alias); - mockValidatorFileParser.verify(); + List validators = actionValidatorManager.getValidators(DataAware2.class, alias); + + assertThat(validators).hasSize(3).map(Validator::getClass).containsExactly( + RequiredFieldValidator.class, + RequiredStringValidator.class, + RequiredStringValidator.class + ); + assertThat(validators).hasSize(3).map(Validator::getValidatorType).containsExactly( + "required", + "requiredstring", + "requiredstring" + ); + assertThat(validators).hasSize(3).map(Validator::getDefaultMessage).containsExactly( + "You must enter a value for data.", + "You must enter a value for data.", + "You must enter a value for data." + ); } public void testGetValidatorsFromInterface() { - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/SimpleAction-validation.xml")), - new ArrayList()); - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/SimpleAction-validationAlias-validation.xml")), - new ArrayList()); - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/test/DataAware-validation.xml")), - new ArrayList()); - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/test/DataAware-validationAlias-validation.xml")), - new ArrayList()); - actionValidatorManager.getValidators(SimpleAction3.class, alias); - mockValidatorFileParser.verify(); - } - - public void testSameAliasWithDifferentClass() { - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/SimpleAction-validation.xml")), - new ArrayList()); - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/SimpleAction-validationAlias-validation.xml")), - new ArrayList()); - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/test/SimpleAction2-validation.xml")), - new ArrayList()); - mockValidatorFileParser.expectAndReturn("parseActionValidatorConfigs", - C.args(C.IS_NOT_NULL, C.IS_NOT_NULL, C.eq("com/opensymphony/xwork2/test/SimpleAction2-validationAlias-validation.xml")), - new ArrayList()); - actionValidatorManager.getValidators(SimpleAction.class, alias); - actionValidatorManager.getValidators(SimpleAction2.class, alias); - mockValidatorFileParser.verify(); + List validators = actionValidatorManager.getValidators(SimpleAction3.class, alias); + + assertThat(validators).hasSize(13).map(Validator::getClass).containsExactly( + ExpressionValidator.class, + RequiredFieldValidator.class, + IntRangeFieldValidator.class, + DoubleRangeFieldValidator.class, + DateRangeFieldValidator.class, + IntRangeFieldValidator.class, + IntRangeFieldValidator.class, + LongRangeFieldValidator.class, + ShortRangeFieldValidator.class, + RequiredFieldValidator.class, + IntRangeFieldValidator.class, + RequiredFieldValidator.class, + RequiredStringValidator.class + ); + assertThat(validators).hasSize(13).map(Validator::getValidatorType).containsExactly( + "expression", + "required", + "int", + "double", + "date", + "int", + "int", + "long", + "short", + "required", + "int", + "required", + "requiredstring" + ); + assertThat(validators).hasSize(13).map(Validator::getDefaultMessage).containsExactly( + "Foo must be greater than Bar. Foo = ${foo}, Bar = ${bar}.", + "You must enter a value for bar.", + "bar must be between ${min} and ${max}, current value is ${bar}.", + "percentage must be between ${minExclusive} and ${maxExclusive}, current value is ${percentage}.", + "The date must be between 12-22-2002 and 12-25-2002.", + "Could not find foo.range!", + "Could not find baz.range!", + "Could not find foo.range!", + "Could not find foo.range!", + "You must enter a value for baz.", + "baz out of range.", + "You must enter a value for data.", + "You must enter a value for data." + ); } /** * Test to verify WW-3850. - * - * @since 2.3.5 */ public void testBuildsValidatorsForClassError() { // for this test we need to have a file manager with reloadingConfigs to true @@ -188,12 +194,8 @@ public void testBuildsValidatorsForClassError() { } } - /* - // TODO: this all need to be converted to real unit tests - public void testSkipUserMarkerActionLevelShortCircuit() { - // get validators - List validatorList = actionValidatorManager.getValidators(User.class, null); + List validatorList = actionValidatorManager.getValidators(User.class, null); assertEquals(10, validatorList.size()); try { @@ -202,16 +204,17 @@ public void testSkipUserMarkerActionLevelShortCircuit() { user.setEmail("bad_email"); user.setEmail2("bad_email"); - ValidatorContext context = new GenericValidatorContext(user); + ValidationAware validationAware = new SimpleAction(); + ValidatorContext context = new DelegatingValidatorContext(validationAware, actionValidatorManager.textProviderFactory); actionValidatorManager.validate(user, null, context); assertTrue(context.hasFieldErrors()); // check field errors - List l = (List) context.getFieldErrors().get("email"); + List l = context.getFieldErrors().get("email"); assertNotNull(l); assertEquals(1, l.size()); assertEquals("Not a valid e-mail.", l.get(0)); - l = (List) context.getFieldErrors().get("email2"); + l = context.getFieldErrors().get("email2"); assertNotNull(l); assertEquals(2, l.size()); assertEquals("Not a valid e-mail2.", l.get(0)); @@ -219,19 +222,17 @@ public void testSkipUserMarkerActionLevelShortCircuit() { // check action errors assertTrue(context.hasActionErrors()); - l = (List) context.getActionErrors(); + l = new ArrayList<>(context.getActionErrors()); assertNotNull(l); assertEquals(2, l.size()); // both expression test failed see User-validation.xml assertEquals("Email does not start with mark", l.get(0)); } catch (ValidationException ex) { - ex.printStackTrace(); fail("Validation error: " + ex.getMessage()); } } public void testSkipAllActionLevelShortCircuit2() { - // get validators - List validatorList = actionValidatorManager.getValidators(User.class, null); + List validatorList = actionValidatorManager.getValidators(User.class, null); assertEquals(10, validatorList.size()); try { @@ -244,34 +245,30 @@ public void testSkipAllActionLevelShortCircuit2() { user.setEmail("mark_bad_email_for_field_val@foo.com"); user.setEmail2("mark_bad_email_for_field_val@foo.com"); - ValidatorContext context = new GenericValidatorContext(user); + ValidationAware validationAware = new SimpleAction(); + ValidatorContext context = new DelegatingValidatorContext(validationAware, actionValidatorManager.textProviderFactory); actionValidatorManager.validate(user, null, context); assertTrue(context.hasFieldErrors()); // check field errors // we have an error in this field level, email does not ends with mycompany.com - List l = (List) context.getFieldErrors().get("email"); + List l = context.getFieldErrors().get("email"); assertNotNull(l); assertEquals(1, l.size()); // because email-field-val is short-circuit assertEquals("Email not from the right company.", l.get(0)); - // check action errors - l = (List) context.getActionErrors(); + l = new ArrayList<>(context.getActionErrors()); assertFalse(context.hasActionErrors()); assertEquals(0, l.size()); - - } catch (ValidationException ex) { - ex.printStackTrace(); fail("Validation error: " + ex.getMessage()); } } public void testActionLevelShortCircuit() throws Exception { - - List validatorList = actionValidatorManager.getValidators(User.class, null); + List validatorList = actionValidatorManager.getValidators(User.class, null); assertEquals(10, validatorList.size()); User user = new User(); @@ -280,18 +277,18 @@ public void testActionLevelShortCircuit() throws Exception { user.setEmail("tmjee(at)yahoo.co.uk"); user.setEmail("tm_jee(at)yahoo.co.uk"); - ValidatorContext context = new GenericValidatorContext(user); + ValidationAware validationAware = new SimpleAction(); + ValidatorContext context = new DelegatingValidatorContext(validationAware, actionValidatorManager.textProviderFactory); actionValidatorManager.validate(user, null, context); - // check field level errors + // check field level errors // shouldn't have any because action error prevents validation of anything else - List l = (List) context.getFieldErrors().get("email2"); + List l = context.getFieldErrors().get("email2"); assertNull(l); - // check action errors assertTrue(context.hasActionErrors()); - l = (List) context.getActionErrors(); + l = new ArrayList<>(context.getActionErrors()); assertNotNull(l); // we only get one, because UserMarker-validation.xml action-level validator // already sc it :-) @@ -299,10 +296,8 @@ public void testActionLevelShortCircuit() throws Exception { assertEquals("Email not the same as email2", l.get(0)); } - public void testShortCircuitNoErrors() { - // get validators - List validatorList = actionValidatorManager.getValidators(User.class, null); + List validatorList = actionValidatorManager.getValidators(User.class, null); assertEquals(10, validatorList.size()); try { @@ -311,73 +306,72 @@ public void testShortCircuitNoErrors() { user.setEmail("mark@mycompany.com"); user.setEmail2("mark@mycompany.com"); - ValidatorContext context = new GenericValidatorContext(user); + ValidationAware validationAware = new SimpleAction(); + ValidatorContext context = new DelegatingValidatorContext(validationAware, actionValidatorManager.textProviderFactory); actionValidatorManager.validate(user, null, context); assertFalse(context.hasErrors()); } catch (ValidationException ex) { - ex.printStackTrace(); fail("Validation error: " + ex.getMessage()); } } public void testFieldErrorsOrder() throws Exception { - ValidationOrderAction action = new ValidationOrderAction(); - actionValidatorManager.validate(action, "actionContext"); - Map fieldErrors = action.getFieldErrors(); - Iterator i = fieldErrors.entrySet().iterator(); + ValidationOrderAction action = new ValidationOrderAction(); + actionValidatorManager.validate(action, "actionContext"); + Map> fieldErrors = action.getFieldErrors(); + Iterator>> i = fieldErrors.entrySet().iterator(); - assertNotNull(fieldErrors); - assertEquals(fieldErrors.size(), 12); + assertNotNull(fieldErrors); + assertEquals(fieldErrors.size(), 12); - Map.Entry e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "username"); - assertEquals(((List)e.getValue()).get(0), "username required"); + Map.Entry> e = i.next(); + assertEquals(e.getKey(), "username"); + assertEquals(e.getValue().get(0), "username required"); - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "password"); - assertEquals(((List)e.getValue()).get(0), "password required"); + e = i.next(); + assertEquals(e.getKey(), "password"); + assertEquals((e.getValue()).get(0), "password required"); - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "confirmPassword"); - assertEquals(((List)e.getValue()).get(0), "confirm password required"); + e = i.next(); + assertEquals(e.getKey(), "confirmPassword"); + assertEquals((e.getValue()).get(0), "confirm password required"); - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "firstName"); - assertEquals(((List)e.getValue()).get(0), "first name required"); + e = i.next(); + assertEquals(e.getKey(), "firstName"); + assertEquals((e.getValue()).get(0), "first name required"); - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "lastName"); - assertEquals(((List)e.getValue()).get(0), "last name required"); + e = i.next(); + assertEquals(e.getKey(), "lastName"); + assertEquals((e.getValue()).get(0), "last name required"); - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "city"); - assertEquals(((List)e.getValue()).get(0), "city is required"); + e = i.next(); + assertEquals(e.getKey(), "city"); + assertEquals((e.getValue()).get(0), "city is required"); - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "province"); - assertEquals(((List)e.getValue()).get(0), "province is required"); + e = i.next(); + assertEquals(e.getKey(), "province"); + assertEquals((e.getValue()).get(0), "province is required"); - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "country"); - assertEquals(((List)e.getValue()).get(0), "country is required"); + e = i.next(); + assertEquals(e.getKey(), "country"); + assertEquals((e.getValue()).get(0), "country is required"); - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "postalCode"); - assertEquals(((List)e.getValue()).get(0), "postal code is required"); + e = i.next(); + assertEquals(e.getKey(), "postalCode"); + assertEquals((e.getValue()).get(0), "postal code is required"); - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "email"); - assertEquals(((List)e.getValue()).get(0), "email is required"); + e = i.next(); + assertEquals(e.getKey(), "email"); + assertEquals((e.getValue()).get(0), "email is required"); - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "website"); - assertEquals(((List)e.getValue()).get(0), "website is required"); - - e = (Map.Entry) i.next(); - assertEquals(e.getKey(), "passwordHint"); - assertEquals(((List)e.getValue()).get(0), "password hint is required"); + e = i.next(); + assertEquals(e.getKey(), "website"); + assertEquals((e.getValue()).get(0), "website is required"); + e = i.next(); + assertEquals(e.getKey(), "passwordHint"); + assertEquals((e.getValue()).get(0), "password hint is required"); } - */ + } diff --git a/core/src/test/java/com/opensymphony/xwork2/validator/VisitorFieldValidatorTest.java b/core/src/test/java/com/opensymphony/xwork2/validator/VisitorFieldValidatorTest.java index 76c2eac718..de605d2c5f 100644 --- a/core/src/test/java/com/opensymphony/xwork2/validator/VisitorFieldValidatorTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/validator/VisitorFieldValidatorTest.java @@ -28,6 +28,8 @@ import com.opensymphony.xwork2.conversion.impl.ConversionData; import org.easymock.EasyMock; +import java.sql.Date; +import java.time.LocalDate; import java.util.Calendar; import java.util.GregorianCalendar; import java.util.HashMap; @@ -142,6 +144,15 @@ public void testCollectionValidation() throws Exception { assertEquals(1, errors.size()); } + public void testDateValidation() throws Exception { + action.setBirthday(Date.valueOf(LocalDate.now().minusYears(20))); + action.setContext("birthday"); + + validate("birthday"); + + assertFalse(action.hasFieldErrors()); + } + public void testContextIsOverriddenByContextParamInValidationXML() throws Exception { validate("visitorValidationAlias"); assertTrue(action.hasFieldErrors()); diff --git a/core/src/test/java/com/opensymphony/xwork2/validator/VisitorValidatorTestAction.java b/core/src/test/java/com/opensymphony/xwork2/validator/VisitorValidatorTestAction.java index 2050726f7f..9e672bf489 100644 --- a/core/src/test/java/com/opensymphony/xwork2/validator/VisitorValidatorTestAction.java +++ b/core/src/test/java/com/opensymphony/xwork2/validator/VisitorValidatorTestAction.java @@ -22,6 +22,7 @@ import com.opensymphony.xwork2.TestBean; import java.util.ArrayList; +import java.util.Date; import java.util.List; @@ -37,7 +38,7 @@ public class VisitorValidatorTestAction extends ActionSupport { private String context; private TestBean bean = new TestBean(); private TestBean[] testBeanArray; - + private Date birthday; public VisitorValidatorTestAction() { testBeanArray = new TestBean[5]; @@ -80,4 +81,12 @@ public void setTestBeanList(List testBeanList) { public List getTestBeanList() { return testBeanList; } + + public Date getBirthday() { + return birthday; + } + + public void setBirthday(Date birthday) { + this.birthday = birthday; + } } diff --git a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java index c711eccd75..f913f74d56 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java @@ -19,6 +19,7 @@ package org.apache.struts2.interceptor; import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.mock.MockActionInvocation; import org.apache.logging.log4j.util.Strings; import org.apache.struts2.StrutsInternalTestCase; @@ -32,6 +33,7 @@ import org.springframework.mock.web.MockHttpServletResponse; import jakarta.servlet.http.HttpSession; +import jakarta.servlet.http.HttpServletRequest; import static org.junit.Assert.assertNotEquals; @@ -74,8 +76,10 @@ public void test_whenNonceAlreadySetInSession_andRequestReceived_thenNewNonceIsS public void testEnforcingCspHeadersSet() throws Exception { String reportUri = "/csp-reports"; + String reportTo = "csp-group"; boolean enforcingMode = true; interceptor.setReportUri(reportUri); + interceptor.setReportTo(reportTo); interceptor.setEnforcingMode(enforcingMode); session.setAttribute("nonce", "foo"); @@ -84,13 +88,15 @@ public void testEnforcingCspHeadersSet() throws Exception { assertNotNull("Nonce key does not exist", session.getAttribute("nonce")); assertFalse("Nonce value is empty", Strings.isEmpty((String) session.getAttribute("nonce"))); assertNotEquals("New nonce value couldn't be set", "foo", session.getAttribute("nonce")); - checkHeader(reportUri, enforcingMode); + checkHeader(reportUri, reportTo, enforcingMode); } public void testReportingCspHeadersSet() throws Exception { String reportUri = "/csp-reports"; + String reportTo = "csp-group"; boolean enforcingMode = false; interceptor.setReportUri(reportUri); + interceptor.setReportTo(reportTo); interceptor.setEnforcingMode(enforcingMode); session.setAttribute("nonce", "foo"); @@ -98,7 +104,7 @@ public void testReportingCspHeadersSet() throws Exception { assertNotNull("Nonce value is empty", session.getAttribute("nonce")); assertNotEquals("New nonce value couldn't be set", "foo", session.getAttribute("nonce")); - checkHeader(reportUri, enforcingMode); + checkHeader(reportUri, reportTo, enforcingMode); } public void test_uriSetOnlyWhenSetIsCalled() throws Exception { @@ -174,21 +180,89 @@ public void testNoPrependContext() throws Exception { checkHeader("/report-uri", enforcingMode); } + public void testNonExistingCspSettingsClassName() throws Exception { + boolean enforcingMode = true; + mai.setAction(new TestAction()); + request.setContextPath("/app"); + + interceptor.setEnforcingMode(enforcingMode); + interceptor.setReportUri("/report-uri"); + interceptor.setPrependServletContext(false); + + try { + interceptor.setCspSettingsClassName("foo"); + interceptor.intercept(mai); + fail("Expected exception"); + } catch (ConfigurationException e) { + assertEquals("The class foo doesn't exist!", e.getMessage()); + } + } + + public void testInvalidCspSettingsClassName() throws Exception { + boolean enforcingMode = true; + mai.setAction(new TestAction()); + request.setContextPath("/app"); + + interceptor.setEnforcingMode(enforcingMode); + interceptor.setReportUri("/report-uri"); + interceptor.setPrependServletContext(false); + + try { + interceptor.setCspSettingsClassName(Integer.class.getName()); + interceptor.intercept(mai); + fail("Expected exception"); + } catch (ConfigurationException e) { + assertEquals("The class java.lang.Integer doesn't implement org.apache.struts2.interceptor.csp.CspSettings!", e.getMessage()); + } + } + + public void testCustomCspSettingsClassName() throws Exception { + boolean enforcingMode = true; + mai.setAction(new TestAction()); + request.setContextPath("/app"); + + interceptor.setEnforcingMode(enforcingMode); + interceptor.setReportUri("/report-uri"); + interceptor.setPrependServletContext(false); + interceptor.setCspSettingsClassName(CustomDefaultCspSettings.class.getName()); + + interceptor.intercept(mai); + + String header = response.getHeader(CspSettings.CSP_ENFORCE_HEADER); + + // no other customization matters for this particular class + assertEquals("foo", header); + } + public void checkHeader(String reportUri, boolean enforcingMode) { + checkHeader(reportUri, null, enforcingMode); + } + + public void checkHeader(String reportUri, String reportTo, boolean enforcingMode) { String expectedCspHeader; if (Strings.isEmpty(reportUri)) { expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s %s; %s '%s'; ", - CspSettings.OBJECT_SRC, CspSettings.NONE, - CspSettings.SCRIPT_SRC, session.getAttribute("nonce"), CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS, - CspSettings.BASE_URI, CspSettings.NONE + CspSettings.OBJECT_SRC, CspSettings.NONE, + CspSettings.SCRIPT_SRC, session.getAttribute("nonce"), CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS, + CspSettings.BASE_URI, CspSettings.NONE ); } else { - expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s %s; %s '%s'; %s %s", - CspSettings.OBJECT_SRC, CspSettings.NONE, - CspSettings.SCRIPT_SRC, session.getAttribute("nonce"), CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS, - CspSettings.BASE_URI, CspSettings.NONE, - CspSettings.REPORT_URI, reportUri - ); + if (Strings.isEmpty(reportTo)) { + expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s %s; %s '%s'; %s %s; ", + CspSettings.OBJECT_SRC, CspSettings.NONE, + CspSettings.SCRIPT_SRC, session.getAttribute("nonce"), CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS, + CspSettings.BASE_URI, CspSettings.NONE, + CspSettings.REPORT_URI, reportUri + ); + } else { + expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s %s; %s '%s'; %s %s; %s %s; ", + CspSettings.OBJECT_SRC, CspSettings.NONE, + CspSettings.SCRIPT_SRC, session.getAttribute("nonce"), CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS, + CspSettings.BASE_URI, CspSettings.NONE, + CspSettings.REPORT_URI, reportUri, + CspSettings.REPORT_TO, reportTo + ); + } } String header; @@ -207,10 +281,11 @@ protected void setUp() throws Exception { super.setUp(); container.inject(interceptor); ActionContext context = ActionContext.getContext() - .withServletRequest(request) - .withServletResponse(response) - .withSession(new SessionMap(request)) - .bind(); + .withContainer(container) + .withServletRequest(request) + .withServletResponse(response) + .withSession(new SessionMap(request)) + .bind(); mai.setInvocationContext(context); session = request.getSession(); } @@ -230,4 +305,15 @@ public CspSettings getCspSettings() { return settings; } } + + /** + * Custom DefaultCspSettings class that overrides the createPolicyFormat method + * to return a fixed value. + */ + public static class CustomDefaultCspSettings extends DefaultCspSettings { + + protected String createPolicyFormat(HttpServletRequest request) { + return "foo"; + } + } } diff --git a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java index 8a14230fe3..3e71af74e1 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java @@ -147,6 +147,26 @@ public void testNotExistingLocale() throws Exception { assertEquals(Locale.getDefault(), session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should create a locale object } + public void testTrimableLocaleString1() throws Exception { + prepare(I18nInterceptor.DEFAULT_PARAMETER, "de\n"); + + interceptor.intercept(mai); + + assertFalse(mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined()); // should have been removed + assertNotNull(session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should be stored here + assertEquals(Locale.GERMAN, session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should create a locale object + } + + public void testTrimableLocaleString2() throws Exception { + prepare(I18nInterceptor.DEFAULT_PARAMETER, "de "); + + interceptor.intercept(mai); + + assertFalse(mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined()); // should have been removed + assertNotNull(session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should be stored here + assertEquals(Locale.GERMAN, session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should create a locale object + } + public void testWithVariant() throws Exception { prepare(I18nInterceptor.DEFAULT_PARAMETER, "ja_JP_JP"); interceptor.intercept(mai); diff --git a/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java b/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java index 19faf41ea6..3b1617bbde 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java @@ -20,6 +20,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.NotSerializableException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.security.SecureRandom; @@ -42,6 +43,8 @@ import com.opensymphony.xwork2.ActionInvocation; import com.opensymphony.xwork2.mock.MockActionInvocation; +import static org.awaitility.Awaitility.await; + /** * Test case for BackgroundProcessTest. */ @@ -60,9 +63,9 @@ public void testSerializeDeserialize() throws Exception { invocation.setInvocationContext(ActionContext.getContext()); StrutsBackgroundProcess bp = (StrutsBackgroundProcess) new StrutsBackgroundProcess( - invocation, - "BackgroundProcessTest.testSerializeDeserialize", - Thread.MIN_PRIORITY + invocation, + "BackgroundProcessTest.testSerializeDeserialize", + Thread.MIN_PRIORITY ).prepare(); executor.execute(bp); @@ -121,6 +124,31 @@ public void testMultipleProcesses() throws InterruptedException { assertEquals(100, mutableState.get()); } + public void testErrorableProcesses1() { + MockActionInvocationWithActionInvoker invocation = new MockActionInvocationWithActionInvoker(() -> { + throw new IllegalStateException("boom"); + }); + + BackgroundProcess bp = new ErrorableBackgroundProcess(invocation, null).prepare(); + executor.execute(bp); + + await().atLeast(100, TimeUnit.MILLISECONDS).until(bp::isDone); + + assertTrue("afterInvocation not called in case of exception", ((ErrorableBackgroundProcess) bp).isDoneAfter()); + } + + public void testErrorableProcesses2() { + MockActionInvocationWithActionInvoker invocation = new MockActionInvocationWithActionInvoker(() -> "done"); + + IllegalStateException expected = new IllegalStateException("after!"); + BackgroundProcess bp = new ErrorableBackgroundProcess(invocation, expected).prepare(); + executor.execute(bp); + + await().atLeast(100, TimeUnit.MILLISECONDS).until(bp::isDone); + + assertEquals(expected, bp.getException()); + } + public void testUnpreparedProcess() throws ExecutionException, InterruptedException, TimeoutException { // given MockActionInvocationWithActionInvoker invocation = new MockActionInvocationWithActionInvoker(() -> "done"); @@ -148,7 +176,8 @@ public String invokeActionOnly() throws Exception { } private static class NotSerializableException extends Exception { - private MockHttpServletRequest notSerializableField; + @SuppressWarnings("unused") + private final MockHttpServletRequest notSerializableField; NotSerializableException(MockHttpServletRequest notSerializableField) { this.notSerializableField = notSerializableField; @@ -171,10 +200,29 @@ public void run() { super.run(); } } +} + +class ErrorableBackgroundProcess extends StrutsBackgroundProcess { + + private final Exception afterException; + private boolean doneAfter; + + public ErrorableBackgroundProcess(ActionInvocation invocation, Exception afterException) { + super(invocation, "errorabale process", Thread.NORM_PRIORITY); + this.afterException = afterException; + } @Override protected void afterInvocation() throws Exception { - super.afterInvocation(); - lock.notify(); + if (afterException != null) { + throw afterException; + } else { + super.afterInvocation(); + doneAfter = true; + } + } + + public boolean isDoneAfter() { + return doneAfter; } } diff --git a/core/src/test/java/org/apache/struts2/views/jsp/SetTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/SetTagTest.java index 5c1f9047a8..078c4a626b 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/SetTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/SetTagTest.java @@ -25,14 +25,10 @@ import jakarta.servlet.jsp.JspException; - -/** - */ public class SetTagTest extends AbstractUITagTest { - Chewbacca chewie; - SetTag tag; - + private Chewbacca chewie; + private SetTag tag; public void testApplicationScope() throws JspException { tag.setName("foo"); @@ -400,6 +396,50 @@ public void testEmptyBody_clearTagStateSet() throws JspException { strutsBodyTagsAreReflectionEqual(tag, freshTag)); } + public void testShortVarNameInPageScope() throws JspException { + tag.setName("f"); + tag.setValue("name"); + tag.setScope("page"); + + tag.doStartTag(); + tag.doEndTag(); + + assertEquals("chewie", pageContext.getAttribute("f")); + } + + public void testShortVarNameInRequestScope() throws JspException { + tag.setName("f"); + tag.setValue("name"); + tag.setScope("request"); + + tag.doStartTag(); + tag.doEndTag(); + + assertEquals("chewie", request.getAttribute("f")); + } + + public void testShortVarNameInSessionScope() throws JspException { + tag.setName("f"); + tag.setValue("name"); + tag.setScope("session"); + + tag.doStartTag(); + tag.doEndTag(); + + assertEquals("chewie", session.get("f")); + } + + public void testShortVarNameInApplicationScope() throws JspException { + tag.setName("f"); + tag.setValue("name"); + tag.setScope("application"); + + tag.doStartTag(); + tag.doEndTag(); + + assertEquals("chewie", servletContext.getAttribute("f")); + } + @Override protected void setUp() throws Exception { super.setUp(); @@ -411,9 +451,9 @@ protected void setUp() throws Exception { } - public class Chewbacca { - String name; - boolean furry; + public static class Chewbacca { + private String name; + private boolean furry; public Chewbacca(String name, boolean furry) { this.name = name; diff --git a/core/src/test/resources/com/opensymphony/xwork2/validator/VisitorValidatorTestAction-validation.xml b/core/src/test/resources/com/opensymphony/xwork2/validator/VisitorValidatorTestAction-validation.xml index a8de9f705c..fb2aa80bf2 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/validator/VisitorValidatorTestAction-validation.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/validator/VisitorValidatorTestAction-validation.xml @@ -26,4 +26,12 @@ You must enter a context. + + + + + + diff --git a/plugins/tiles/pom.xml b/plugins/tiles/pom.xml index 706984e28c..8a1ecf89f8 100644 --- a/plugins/tiles/pom.xml +++ b/plugins/tiles/pom.xml @@ -40,7 +40,7 @@ org.codehaus.mojo exec-maven-plugin - 3.1.0 + 3.2.0 compile @@ -103,6 +103,16 @@ log4j-jcl test + + org.assertj + assertj-core + test + + + org.springframework + spring-test + test + UTF-8 diff --git a/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesContainerFactory.java b/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesContainerFactory.java index def7aab961..d6e7cf9b36 100644 --- a/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesContainerFactory.java +++ b/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsTilesContainerFactory.java @@ -68,10 +68,7 @@ import jakarta.el.ResourceBundleELResolver; import jakarta.servlet.jsp.JspFactory; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -105,19 +102,16 @@ public class StrutsTilesContainerFactory extends BasicTilesContainerFactory { /** * Default pattern to be used to collect Tiles definitions if user didn't configure any - * - * @deprecated since Struts 6.4.0, use {@link #TILES_DEFAULT_PATTERNS} instead */ - @Deprecated - public static final String TILES_DEFAULT_PATTERN = "/WEB-INF/**/tiles*.xml,classpath*:META-INF/**/tiles*.xml"; + public static final Set TILES_DEFAULT_PATTERNS = TextParseUtil.commaDelimitedStringToSet("*tiles*.xml"); /** * Default pattern to be used to collect Tiles definitions if user didn't configure any + * + * @deprecated since Struts 6.4.0, use {@link #TILES_DEFAULT_PATTERNS} instead */ - public static final Set TILES_DEFAULT_PATTERNS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( - "/WEB-INF/**/tiles*.xml", - "classpath*:META-INF/**/tiles*.xml" - ))); + @Deprecated + public static final String TILES_DEFAULT_PATTERN = String.join(",", TILES_DEFAULT_PATTERNS); /** * Supported expression languages diff --git a/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsWildcardServletApplicationContext.java b/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsWildcardServletApplicationContext.java index 814850d77b..f973bb29e8 100644 --- a/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsWildcardServletApplicationContext.java +++ b/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsWildcardServletApplicationContext.java @@ -43,7 +43,7 @@ public class StrutsWildcardServletApplicationContext extends ServletApplicationC private static final Logger LOG = LogManager.getLogger(StrutsWildcardServletApplicationContext.class); - private ResourceFinder finder; + private final ResourceFinder finder; public StrutsWildcardServletApplicationContext(ServletContext context) { super(context); @@ -64,16 +64,15 @@ public StrutsWildcardServletApplicationContext(ServletContext context) { } try { - Enumeration resources = getClass().getClassLoader().getResources("/"); + Enumeration resources = getClass().getClassLoader().getResources(""); while (resources.hasMoreElements()) { - URL resource = resources.nextElement(); - urls.add(resource); + urls.add(resources.nextElement()); } } catch (IOException e) { throw new ConfigurationException(e); } - finder = new ResourceFinder(urls.toArray(new URL[urls.size()])); + finder = new ResourceFinder(urls.toArray(new URL[0])); } public Collection getResources(String path) { diff --git a/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesContainerFactoryTest.java b/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesContainerFactoryTest.java index dbadffa7a8..89d4b53975 100644 --- a/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesContainerFactoryTest.java +++ b/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsTilesContainerFactoryTest.java @@ -36,6 +36,7 @@ import jakarta.servlet.ServletContext; import jakarta.servlet.jsp.JspFactory; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Objects; @@ -64,12 +65,11 @@ public void getSources() { Objects.requireNonNull(getClass().getResource("/org/apache/tiles/core/config/tiles-defs.xml")) ); ApplicationResource classpathResource = new URLApplicationResource( - "/org/apache/tiles/core/config/defs1.xml", - Objects.requireNonNull(getClass().getResource("/org/apache/tiles/core/config/defs1.xml")) + "/org/apache/tiles/core/config/tiles_defs1.xml", + Objects.requireNonNull(getClass().getResource("/org/apache/tiles/core/config/tiles_defs1.xml")) ); when(applicationContext.getInitParams()).thenReturn(Collections.emptyMap()); - when(applicationContext.getResources("/WEB-INF/**/tiles*.xml")).thenReturn(Collections.singleton(pathResource)); - when(applicationContext.getResources("classpath*:META-INF/**/tiles*.xml")).thenReturn(Collections.singleton(classpathResource)); + when(applicationContext.getResources("*tiles*.xml")).thenReturn(Arrays.asList(pathResource, classpathResource)); List resources = factory.getSources(applicationContext); assertEquals("The urls list is not two-sized", 2, resources.size()); diff --git a/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsWildcardServletApplicationContextTest.java b/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsWildcardServletApplicationContextTest.java new file mode 100644 index 0000000000..ec12873f91 --- /dev/null +++ b/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsWildcardServletApplicationContextTest.java @@ -0,0 +1,78 @@ +/* + * 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.tiles; + +import org.apache.tiles.request.ApplicationResource; +import org.junit.Before; +import org.junit.Test; +import org.springframework.core.io.FileSystemResource; +import org.springframework.core.io.FileUrlResource; +import org.springframework.core.io.Resource; +import org.springframework.core.io.ResourceLoader; +import org.springframework.mock.web.MockServletContext; + +import jakarta.servlet.ServletContext; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Collection; + +import static org.assertj.core.api.Assertions.assertThat; + +public class StrutsWildcardServletApplicationContextTest { + + private ServletContext context; + + @Before + public void setUp() throws Exception { + URL resource = getClass().getResource("/"); + context = new MockServletContext(resource.getPath(), new ResourceLoader() { + @Override + public Resource getResource(String location) { + try { + String finalLocation = location.replaceAll("//", "/"); + if (finalLocation.endsWith("/")) { + return new FileSystemResource(finalLocation); + } + return new FileUrlResource(finalLocation); + } catch (MalformedURLException e) { + return null; + } + } + + @Override + public ClassLoader getClassLoader() { + return StrutsWildcardServletApplicationContextTest.class.getClassLoader(); + } + }); + } + + @Test + public void wildcardSupport() { + StrutsWildcardServletApplicationContext applicationContext = new StrutsWildcardServletApplicationContext(context); + + Collection resources = applicationContext.getResources("*tiles*.xml"); + + assertThat(resources) + .hasSize(1) + .extracting(ApplicationResource::getLocalePath) + .first().asString() + .endsWith("/WEB-INF/tiles.xml"); + } + +} \ No newline at end of file diff --git a/plugins/tiles/src/test/resources/WEB-INF/tiles.xml b/plugins/tiles/src/test/resources/WEB-INF/tiles.xml new file mode 100644 index 0000000000..9fc36a4d6a --- /dev/null +++ b/plugins/tiles/src/test/resources/WEB-INF/tiles.xml @@ -0,0 +1,39 @@ + + + + + + + + + + + + + + + + + + diff --git a/plugins/tiles/src/test/resources/org/apache/tiles/core/config/tiles_defs1.xml b/plugins/tiles/src/test/resources/org/apache/tiles/core/config/tiles_defs1.xml new file mode 100644 index 0000000000..621d7cef9e --- /dev/null +++ b/plugins/tiles/src/test/resources/org/apache/tiles/core/config/tiles_defs1.xml @@ -0,0 +1,75 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/pom.xml b/pom.xml index d4fb873098..d330c70b49 100644 --- a/pom.xml +++ b/pom.xml @@ -110,17 +110,17 @@ 17 - 9.6 + 9.7 1.14.11 - 2.3.32 + 2.3.33 8.0.1.Final - 2.16.0 + 2.17.1 2.23.1 3.2.5 5.8.0 3.3.5 2.5.0 - 2.0.12 + 2.0.13 6.0.13 3.0.8 1.0.7 @@ -206,7 +206,7 @@ org.jacoco jacoco-maven-plugin - 0.8.11 + 0.8.12 prepare-agent @@ -240,7 +240,7 @@ org.apache.maven.plugins maven-project-info-reports-plugin - 3.0.0 + 3.5.0 org.apache.maven.plugins @@ -287,7 +287,7 @@ org.apache.maven.plugins maven-source-plugin - 3.3.0 + 3.3.1 org.apache.rat @@ -332,7 +332,7 @@ org.owasp dependency-check-maven - 8.4.2 + 9.2.0 src/etc/project-suppression.xml @@ -485,7 +485,7 @@ org.codehaus.mojo versions-maven-plugin - 2.16.1 + 2.16.2 @@ -730,6 +730,13 @@ test + + org.awaitility + awaitility + 4.2.1 + test + + jakarta.servlet jakarta.servlet-api @@ -826,7 +833,7 @@ org.apache.commons commons-text - 1.11.0 + 1.12.0 commons-el @@ -914,7 +921,7 @@ org.assertj assertj-core - 3.25.2 + 3.25.3 test @@ -978,7 +985,7 @@ org.apache.commons commons-compress - 1.26.0 + 1.26.2