From 63fcf0f14fa56258631e5b19a60dd365d0f5e739 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Jun 2024 01:57:26 +0000 Subject: [PATCH 01/16] Bump commons-validator:commons-validator from 1.8.0 to 1.9.0 Bumps commons-validator:commons-validator from 1.8.0 to 1.9.0. --- updated-dependencies: - dependency-name: commons-validator:commons-validator dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index c0dba2980..8c198290a 100644 --- a/pom.xml +++ b/pom.xml @@ -892,7 +892,7 @@ commons-validator commons-validator - 1.8.0 + 1.9.0 From 54bf309f88477fd9fd935875cb3975a10646a6e2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Jun 2024 01:57:32 +0000 Subject: [PATCH 02/16] Bump org.apache.felix:org.apache.felix.main from 6.0.3 to 7.0.5 Bumps org.apache.felix:org.apache.felix.main from 6.0.3 to 7.0.5. --- updated-dependencies: - dependency-name: org.apache.felix:org.apache.felix.main dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index c0dba2980..20cd80fa9 100644 --- a/pom.xml +++ b/pom.xml @@ -685,7 +685,7 @@ org.apache.felix org.apache.felix.main - 6.0.3 + 7.0.5 org.apache.felix From b07268d5bda838f414fde854190d7c867fb74400 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 17 Jun 2024 01:28:48 +0000 Subject: [PATCH 03/16] Bump org.apache.maven.plugins:maven-enforcer-plugin from 3.4.1 to 3.5.0 Bumps [org.apache.maven.plugins:maven-enforcer-plugin](https://github.com/apache/maven-enforcer) from 3.4.1 to 3.5.0. - [Release notes](https://github.com/apache/maven-enforcer/releases) - [Commits](https://github.com/apache/maven-enforcer/compare/enforcer-3.4.1...enforcer-3.5.0) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-enforcer-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 185ab02a1..9622406f5 100644 --- a/pom.xml +++ b/pom.xml @@ -354,7 +354,7 @@ org.apache.maven.plugins maven-enforcer-plugin - 3.4.1 + 3.5.0 enforce From a99162a1a440dc670863503edc888e4df76e495d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 17 Jun 2024 01:28:53 +0000 Subject: [PATCH 04/16] Bump org.codehaus.mojo:exec-maven-plugin from 3.2.0 to 3.3.0 Bumps [org.codehaus.mojo:exec-maven-plugin](https://github.com/mojohaus/exec-maven-plugin) from 3.2.0 to 3.3.0. - [Release notes](https://github.com/mojohaus/exec-maven-plugin/releases) - [Commits](https://github.com/mojohaus/exec-maven-plugin/compare/3.2.0...3.3.0) --- updated-dependencies: - dependency-name: org.codehaus.mojo:exec-maven-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- plugins/tiles/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/tiles/pom.xml b/plugins/tiles/pom.xml index 845062b6b..3dd989730 100644 --- a/plugins/tiles/pom.xml +++ b/plugins/tiles/pom.xml @@ -40,7 +40,7 @@ org.codehaus.mojo exec-maven-plugin - 3.2.0 + 3.3.0 compile From 13916c8b843a6b1694302e437382d63f335dcc9c Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 18 Jun 2024 09:39:17 +0200 Subject: [PATCH 05/16] WW-5310 Fixes broken support for Fragments in tag --- .../components/ServletUrlRenderer.java | 18 ++++++---- .../url/StrutsQueryStringParserTest.java | 8 +++++ .../apache/struts2/views/jsp/URLTagTest.java | 36 +++++++++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java b/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java index 853c0a389..460aeebff 100644 --- a/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java +++ b/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java @@ -103,9 +103,9 @@ public class ServletUrlRenderer implements UrlRenderer { } result = urlHelper.buildUrl(_value, urlComponent.getHttpServletRequest(), urlComponent.getHttpServletResponse(), urlComponent.getParameters(), scheme, urlComponent.isIncludeContext(), urlComponent.isEncode(), urlComponent.isForceAddSchemeHostAndPort(), urlComponent.isEscapeAmp()); } - String anchor = urlComponent.getAnchor(); - if (StringUtils.isNotEmpty(anchor)) { - result += '#' + urlComponent.findString(anchor); + if (StringUtils.isNotEmpty(urlComponent.getAnchor())) { + String anchor = urlComponent.findString(urlComponent.getAnchor()); + result += '#' + anchor; } if (urlComponent.isPutInContext()) { @@ -292,7 +292,7 @@ public class ServletUrlRenderer implements UrlRenderer { private void includeGetParameters(UrlProvider urlComponent) { String query = extractQueryString(urlComponent); QueryStringParser.Result result = queryStringParser.parse(query); - mergeRequestParameters(urlComponent.getValue(), urlComponent.getParameters(), result.getQueryParams()); + result = mergeRequestParameters(urlComponent.getValue(), urlComponent.getParameters(), result.getQueryParams()); if (!result.getQueryFragment().isEmpty()) { urlComponent.setAnchor(result.getQueryFragment()); } @@ -331,10 +331,11 @@ public class ServletUrlRenderer implements UrlRenderer { * @param value the value attribute (URL to be generated by this component) * @param parameters component parameters * @param contextParameters request parameters + * @return {@link QueryStringParser.Result} of value's ?query-string or empty() */ - protected void mergeRequestParameters(String value, Map parameters, Map contextParameters) { - + protected QueryStringParser.Result mergeRequestParameters(String value, Map parameters, Map contextParameters) { Map mergedParams = new LinkedHashMap<>(contextParameters); + QueryStringParser.Result result = queryStringParser.empty(); // Merge contextParameters (from current request) with parameters specified in value attribute // eg. value="someAction.action?id=someId&venue=someVenue" @@ -343,7 +344,8 @@ public class ServletUrlRenderer implements UrlRenderer { if (StringUtils.contains(value, "?")) { String queryString = value.substring(value.indexOf('?') + 1); - mergedParams = new LinkedHashMap<>(queryStringParser.parse(queryString).getQueryParams()); + result = queryStringParser.parse(queryString); + mergedParams = new LinkedHashMap<>(result.getQueryParams()); for (Map.Entry entry : contextParameters.entrySet()) { if (!mergedParams.containsKey(entry.getKey())) { mergedParams.put(entry.getKey(), entry.getValue()); @@ -362,6 +364,8 @@ public class ServletUrlRenderer implements UrlRenderer { parameters.put(entry.getKey(), entry.getValue()); } } + + return result; } } diff --git a/core/src/test/java/org/apache/struts2/url/StrutsQueryStringParserTest.java b/core/src/test/java/org/apache/struts2/url/StrutsQueryStringParserTest.java index c8183725b..8108a8d01 100644 --- a/core/src/test/java/org/apache/struts2/url/StrutsQueryStringParserTest.java +++ b/core/src/test/java/org/apache/struts2/url/StrutsQueryStringParserTest.java @@ -112,6 +112,14 @@ public class StrutsQueryStringParserTest { assertEquals("test", queryParameters.getQueryFragment()); } + @Test + public void shouldHandleOnlyFragment() { + QueryStringParser.Result queryParameters = parser.parse("#test"); + + assertTrue(queryParameters.getQueryParams().isEmpty()); + assertEquals("test", queryParameters.getQueryFragment()); + } + @Before public void setUp() throws Exception { this.parser = new StrutsQueryStringParser(new StrutsUrlDecoder()); diff --git a/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java index fc9fbe757..237e78d18 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java @@ -2092,6 +2092,42 @@ public class URLTagTest extends AbstractUITagTest { strutsBodyTagsAreReflectionEqual(tag, freshTag)); } + public void testQueryParamsAndFragment() throws Exception { + request.setRequestURI("/public/about"); + tag.setAction("company"); + tag.setValue("/books?hl=en&lr=Y&redir_esc=y#v=twopage&q&f=false"); + tag.setEscapeAmp("false"); + + tag.doStartTag(); + tag.doEndTag(); + + assertEquals("/books?hl=en&lr=Y&redir_esc=y#v=twopage&q&f=false", writer.toString()); + } + + public void testDoubleEqualSigns() throws Exception { + request.setRequestURI("/public/about"); + tag.setAction("company"); + tag.setValue("/PublicationsDetail.aspx?ID=GjTu91suYQI=&t=1"); + tag.setEscapeAmp("false"); + + tag.doStartTag(); + tag.doEndTag(); + + assertEquals("/PublicationsDetail.aspx?ID=GjTu91suYQI%3D&t=1", writer.toString()); + } + + public void testOnlyFragment() throws Exception { + request.setRequestURI("/public/about"); + tag.setAction("company"); + tag.setValue("/books#v=twopage&q&f=false"); + tag.setEscapeAmp("false"); + + tag.doStartTag(); + tag.doEndTag(); + + assertEquals("/books#v=twopage&q&f=false", writer.toString()); + } + @Override protected void setUp() throws Exception { super.setUp(); From b96cf2c0721f73a931c9a4ff2dce866f5f762803 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Tue, 18 Jun 2024 19:07:50 +1000 Subject: [PATCH 06/16] WW-5429 Log parameter annotation issues at ERROR level when in DevMode --- .../xwork2/interceptor/ValidationAware.java | 6 +- .../xwork2/ognl/ErrorMessageBuilder.java | 4 +- .../opensymphony/xwork2/util/DebugUtils.java | 42 +++++++++++++ .../parameter/ParametersInterceptor.java | 51 +++++++++------- .../parameter/ParametersInterceptorTest.java | 61 +++++++++++-------- 5 files changed, 112 insertions(+), 52 deletions(-) create mode 100644 core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/ValidationAware.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/ValidationAware.java index c19d099f9..9db77e45f 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/ValidationAware.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ValidationAware.java @@ -26,7 +26,7 @@ import java.util.Map; * ValidationAware classes can accept Action (class level) or field level error messages. Action level messages are kept * in a Collection. Field level error messages are kept in a Map from String field name to a List of field error msgs. * - * @author plightbo + * @author plightbo */ public interface ValidationAware { @@ -119,7 +119,9 @@ public interface ValidationAware { * * @return (hasActionErrors() || hasFieldErrors()) */ - boolean hasErrors(); + default boolean hasErrors() { + return hasActionErrors() || hasFieldErrors(); + } /** * Check whether there are any field errors associated with this action. diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/ErrorMessageBuilder.java b/core/src/main/java/com/opensymphony/xwork2/ognl/ErrorMessageBuilder.java index b904f4ba1..ad54b2015 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/ErrorMessageBuilder.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/ErrorMessageBuilder.java @@ -33,7 +33,7 @@ public class ErrorMessageBuilder { } public ErrorMessageBuilder errorSettingExpressionWithValue(String expr, Object value) { - appenExpression(expr); + appendExpression(expr); if (value instanceof Object[]) { appendValueAsArray((Object[]) value, message); } else { @@ -42,7 +42,7 @@ public class ErrorMessageBuilder { return this; } - private void appenExpression(String expr) { + private void appendExpression(String expr) { message.append("Error setting expression '"); message.append(expr); message.append("' with value "); diff --git a/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java b/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java new file mode 100644 index 000000000..05ae875dd --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java @@ -0,0 +1,42 @@ +/* + * 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.util; + +import com.opensymphony.xwork2.TextProvider; +import com.opensymphony.xwork2.interceptor.ValidationAware; +import org.apache.logging.log4j.Logger; + +/** + * @since 6.5.0 + */ +public class DebugUtils { + + public static void notifyDeveloperOfError(Logger log, Object action, String message) { + if (action instanceof TextProvider) { + TextProvider tp = (TextProvider) action; + message = tp.getText("devmode.notification", "Developer Notification:\n{0}", new String[]{message}); + } + log.error(message); + if (action instanceof ValidationAware) { + ValidationAware validationAware = (ValidationAware) action; + validationAware.addActionError(message); + } + } + +} 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 e9215e533..239bc6d6c 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 @@ -20,10 +20,8 @@ package org.apache.struts2.interceptor.parameter; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionInvocation; -import com.opensymphony.xwork2.TextProvider; 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; @@ -56,7 +54,6 @@ 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; @@ -67,6 +64,8 @@ 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 com.opensymphony.xwork2.util.DebugUtils.notifyDeveloperOfError; +import static java.lang.String.format; import static java.util.Collections.unmodifiableSet; import static java.util.stream.Collectors.joining; import static org.apache.commons.lang3.StringUtils.indexOfAny; @@ -317,19 +316,8 @@ public class ParametersInterceptor extends MethodFilterInterceptor { } protected void notifyDeveloperParameterException(Object action, String property, String message) { - String logMsg = "Unexpected Exception caught setting '" + property + "' on '" + action.getClass() + ": " + message; - if (action instanceof TextProvider) { - TextProvider tp = (TextProvider) action; - logMsg = tp.getText("devmode.notification", "Developer Notification:\n{0}", new String[]{logMsg}); - } - LOG.error(logMsg); - - if (action instanceof ValidationAware) { - ValidationAware validationAware = (ValidationAware) action; - Collection messages = validationAware.getActionMessages(); - messages.add(message); - validationAware.setActionMessages(messages); - } + String logMsg = format("Unexpected Exception caught setting '%s' on '%s: %s", property, action.getClass(), message); + notifyDeveloperOfError(LOG, action, logMsg); } /** @@ -388,23 +376,37 @@ public class ParametersInterceptor extends MethodFilterInterceptor { return hasValidAnnotatedField(action, rootProperty, paramDepth); } - if (hasValidAnnotatedPropertyDescriptor(propDescOpt.get(), paramDepth)) { + if (hasValidAnnotatedPropertyDescriptor(action, propDescOpt.get(), paramDepth)) { return true; } return hasValidAnnotatedField(action, rootProperty, paramDepth); } + /** + * @deprecated since 6.5.0, use {@link #hasValidAnnotatedPropertyDescriptor(Object, PropertyDescriptor, long)} + * instead. + */ + @Deprecated protected boolean hasValidAnnotatedPropertyDescriptor(PropertyDescriptor propDesc, long paramDepth) { + return hasValidAnnotatedPropertyDescriptor(null, propDesc, paramDepth); + } + + protected boolean hasValidAnnotatedPropertyDescriptor(Object action, 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'.", + String logMessage = format( + "Parameter injection for method [%s] on action [%s] rejected. Ensure it is annotated with @StrutsParameter with an appropriate 'depth'.", relevantMethod.getName(), relevantMethod.getDeclaringClass().getName()); + if (devMode) { + notifyDeveloperOfError(LOG, action, logMessage); + } else { + LOG.debug(logMessage); + } return false; } if (paramDepth >= 1) { @@ -455,10 +457,15 @@ public class ParametersInterceptor extends MethodFilterInterceptor { 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'.", + String logMessage = format( + "Parameter injection for field [%s] on action [%s] rejected. Ensure it is annotated with @StrutsParameter with an appropriate 'depth'.", fieldName, action.getClass().getName()); + if (devMode) { + notifyDeveloperOfError(LOG, action, logMessage); + } else { + LOG.debug(logMessage); + } return false; } if (paramDepth >= 1) { @@ -533,7 +540,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor { return "NONE"; } return parameters.entrySet().stream() - .map(entry -> String.format("%s => %s ", entry.getKey(), entry.getValue().getValue())) + .map(entry -> format("%s => %s ", entry.getKey(), entry.getValue().getValue())) .collect(joining()); } diff --git a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java index a142014b3..b15594f95 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java @@ -116,15 +116,17 @@ public class ParametersInterceptorTest extends XWorkTestCase { pi.setParameters(action, vs, HttpParameters.create(params).build()); // then - assertEquals(3, action.getActionMessages().size()); + assertEquals(3, action.getActionErrors().size()); - String msg1 = action.getActionMessage(0); - String msg2 = action.getActionMessage(1); - String msg3 = action.getActionMessage(2); + List actionErrors = new ArrayList<>(action.getActionErrors()); - assertEquals("Error setting expression 'expression' with value '#f=#_memberAccess.getClass().getDeclaredField('allowStaticMethodAccess'),#f.setAccessible(true),#f.set(#_memberAccess,true),#req=@org.apache.struts2.ServletActionContext@getRequest(),#resp=@org.apache.struts2.ServletActionContext@getResponse().getWriter(),#resp.println(#req.getRealPath('/')),#resp.close()'", msg1); - assertEquals("Error setting expression 'name' with value '(#context[\"xwork.MethodAccessor.denyMethodExecution\"]= new java.lang.Boolean(false), #_memberAccess[\"allowStaticMethodAccess\"]= new java.lang.Boolean(true), @java.lang.Runtime@getRuntime().exec('mkdir /tmp/PWNAGE'))(meh)'", msg2); - assertEquals("Error setting expression 'top['name'](0)' with value 'true'", msg3); + String msg1 = actionErrors.get(0); + String msg2 = actionErrors.get(1); + String msg3 = actionErrors.get(2); + + assertEquals("Unexpected Exception caught setting 'expression' on 'class org.apache.struts2.interceptor.parameter.ValidateAction: Error setting expression 'expression' with value '#f=#_memberAccess.getClass().getDeclaredField('allowStaticMethodAccess'),#f.setAccessible(true),#f.set(#_memberAccess,true),#req=@org.apache.struts2.ServletActionContext@getRequest(),#resp=@org.apache.struts2.ServletActionContext@getResponse().getWriter(),#resp.println(#req.getRealPath('/')),#resp.close()'", msg1); + assertEquals("Unexpected Exception caught setting 'name' on 'class org.apache.struts2.interceptor.parameter.ValidateAction: Error setting expression 'name' with value '(#context[\"xwork.MethodAccessor.denyMethodExecution\"]= new java.lang.Boolean(false), #_memberAccess[\"allowStaticMethodAccess\"]= new java.lang.Boolean(true), @java.lang.Runtime@getRuntime().exec('mkdir /tmp/PWNAGE'))(meh)'", msg2); + assertEquals("Unexpected Exception caught setting 'top['name'](0)' on 'class org.apache.struts2.interceptor.parameter.ValidateAction: Error setting expression 'top['name'](0)' with value 'true'", msg3); assertNull(action.getName()); } @@ -201,15 +203,16 @@ public class ParametersInterceptorTest extends XWorkTestCase { pi.setParameters(action, vs, HttpParameters.create(params).build()); // then - assertEquals(3, action.getActionMessages().size()); + assertEquals(3, action.getActionErrors().size()); - String msg1 = action.getActionMessage(0); - String msg2 = action.getActionMessage(1); - String msg3 = action.getActionMessage(2); + List actionErrors = new ArrayList<>(action.getActionErrors()); + String msg1 = actionErrors.get(0); + String msg2 = actionErrors.get(1); + String msg3 = actionErrors.get(2); - assertEquals("Error setting expression 'class.classLoader.defaultAssertionStatus' with value 'true'", msg1); - assertEquals("Error setting expression 'class.classLoader.jarPath' with value 'bad'", msg2); - assertEquals("Error setting expression 'model.class.classLoader.jarPath' with value 'very bad'", msg3); + assertEquals("Unexpected Exception caught setting 'class.classLoader.defaultAssertionStatus' on 'class org.apache.struts2.interceptor.parameter.ValidateAction: Error setting expression 'class.classLoader.defaultAssertionStatus' with value 'true'", msg1); + assertEquals("Unexpected Exception caught setting 'class.classLoader.jarPath' on 'class org.apache.struts2.interceptor.parameter.ValidateAction: Error setting expression 'class.classLoader.jarPath' with value 'bad'", msg2); + assertEquals("Unexpected Exception caught setting 'model.class.classLoader.jarPath' on 'class org.apache.struts2.interceptor.parameter.ValidateAction: Error setting expression 'model.class.classLoader.jarPath' with value 'very bad'", msg3); assertFalse(excluded.get(pollution1)); assertFalse(excluded.get(pollution2)); @@ -582,8 +585,8 @@ public class ParametersInterceptorTest extends XWorkTestCase { container.inject(config.getInterceptors().get(0).getInterceptor()); ActionProxy proxy = actionProxyFactory.createActionProxy("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME, null, extraContext.getContextMap()); proxy.execute(); - final String actionMessage = "" + ((SimpleAction) proxy.getAction()).getActionMessages().toArray()[0]; - assertTrue(actionMessage.contains("Error setting expression 'not_a_property' with value 'There is no action property named like this'")); + final String actionError = "" + ((SimpleAction) proxy.getAction()).getActionErrors().toArray()[0]; + assertTrue(actionError.contains("Error setting expression 'not_a_property' with value 'There is no action property named like this'")); } public void testNonexistentParametersAreIgnoredInProductionMode() throws Exception { @@ -1014,59 +1017,65 @@ public class ParametersInterceptorTest extends XWorkTestCase { class ValidateAction implements ValidationAware { private final List messages = new LinkedList<>(); + private final List errors = new LinkedList<>(); private String name; + @Override public void setActionErrors(Collection errorMessages) { } + @Override public Collection getActionErrors() { - return null; + return errors; } + @Override public void setActionMessages(Collection messages) { } + @Override public Collection getActionMessages() { return messages; } + @Override public void setFieldErrors(Map> errorMap) { } + @Override public Map> getFieldErrors() { return null; } + @Override public void addActionError(String anErrorMessage) { + errors.add(anErrorMessage); } + @Override public void addActionMessage(String aMessage) { messages.add(aMessage); } + @Override public void addFieldError(String fieldName, String errorMessage) { } + @Override public boolean hasActionErrors() { - return false; + return !errors.isEmpty(); } + @Override public boolean hasActionMessages() { return !messages.isEmpty(); } - public boolean hasErrors() { - return false; - } - + @Override public boolean hasFieldErrors() { return false; } - public String getActionMessage(int index) { - return messages.get(index); - } - public String getName() { return name; } From ba46c18f072d4a9a58f3c9cd913a4d72e031be61 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Fri, 21 Jun 2024 11:04:10 +1000 Subject: [PATCH 07/16] WW-5429 Make DebugUtils final and remove @author JavaDoc tag --- .../com/opensymphony/xwork2/interceptor/ValidationAware.java | 2 -- core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/ValidationAware.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/ValidationAware.java index 9db77e45f..485cb42fb 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/ValidationAware.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ValidationAware.java @@ -25,8 +25,6 @@ import java.util.Map; /** * ValidationAware classes can accept Action (class level) or field level error messages. Action level messages are kept * in a Collection. Field level error messages are kept in a Map from String field name to a List of field error msgs. - * - * @author plightbo */ public interface ValidationAware { diff --git a/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java b/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java index 05ae875dd..3fdf8b0a7 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java @@ -25,7 +25,7 @@ import org.apache.logging.log4j.Logger; /** * @since 6.5.0 */ -public class DebugUtils { +public final class DebugUtils { public static void notifyDeveloperOfError(Logger log, Object action, String message) { if (action instanceof TextProvider) { From 75ebbf4367aba11f4934a61527c5b6b2e821e0e9 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 21 Jun 2024 08:24:54 +0200 Subject: [PATCH 08/16] WW-5431 Marks unused constants as deprecated To be removed in Struts 7 --- .../views/freemarker/FreemarkerManager.java | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java b/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java index 62c7cc9ae..987306c5a 100644 --- a/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java +++ b/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java @@ -118,9 +118,6 @@ public class FreemarkerManager { public static final String INITPARAM_DEBUG = "Debug"; public static final String KEY_REQUEST = "Request"; - public static final String KEY_INCLUDE = "include_page"; - public static final String KEY_REQUEST_PRIVATE = "__FreeMarkerServlet.Request__"; - public static final String KEY_REQUEST_PARAMETERS = "RequestParameters"; public static final String KEY_SESSION = "Session"; public static final String KEY_APPLICATION = "Application"; public static final String KEY_APPLICATION_PRIVATE = "__FreeMarkerServlet.Application__"; @@ -138,10 +135,29 @@ public class FreemarkerManager { // for Struts public static final String KEY_REQUEST_PARAMETERS_STRUTS = "Parameters"; - public static final String KEY_HASHMODEL_PRIVATE = "__FreeMarkerManager.Request__"; - public static final String EXPIRATION_DATE; + /** + * @deprecated since Struts 6.5.0, do not use as it will be removed in Struts 7.0.0 + */ + @Deprecated + public static final String KEY_INCLUDE = "include_page"; + /** + * @deprecated since Struts 6.5.0, do not use as it will be removed in Struts 7.0.0 + */ + @Deprecated + public static final String KEY_REQUEST_PRIVATE = "__FreeMarkerServlet.Request__"; + /** + * @deprecated since Struts 6.5.0, do not use as it will be removed in Struts 7.0.0 + */ + @Deprecated + public static final String KEY_REQUEST_PARAMETERS = "RequestParameters"; + /** + * @deprecated since Struts 6.5.0, do not use as it will be removed in Struts 7.0.0 + */ + @Deprecated + public static final String KEY_HASHMODEL_PRIVATE = "__FreeMarkerManager.Request__"; + /** * Adds individual settings. * From 98f2e68e0bdc0417352013f4a1d25162cbd2bff9 Mon Sep 17 00:00:00 2001 From: stefansielaff Date: Tue, 2 Jul 2024 13:24:57 +0200 Subject: [PATCH 09/16] "Swap order of sysStrSubstitutor and envStrSubstitutor in substitute method" --- .../xwork2/config/providers/EnvsValueSubstitutor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/EnvsValueSubstitutor.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/EnvsValueSubstitutor.java index 5fc1809b3..9e58fdeba 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/EnvsValueSubstitutor.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/EnvsValueSubstitutor.java @@ -46,7 +46,7 @@ public class EnvsValueSubstitutor implements ValueSubstitutor { public String substitute(String value) { LOG.debug("Substituting value {} with proper System variable or environment variable", value); - String substituted = sysStrSubstitutor.replace(value); - return envStrSubstitutor.replace(substituted); + String substituted = envStrSubstitutor.replace(value); + return sysStrSubstitutor.replace(substituted); } } From 2f814186c8f3d7beb8200285c94db67a7ac50edd Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 17 Jun 2024 21:02:49 +1000 Subject: [PATCH 10/16] WW-5428 Allowlist capability should resolve Hibernate proxies when disableProxyObjects is not set --- .../xwork2/ognl/SecurityMemberAccess.java | 12 +++++++ .../opensymphony/xwork2/util/ProxyUtil.java | 33 +++++++++++++++++++ 2 files changed, 45 insertions(+) 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 f882b2c58..fc74f9a14 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -209,6 +209,18 @@ public class SecurityMemberAccess implements MemberAccess { * @return {@code true} if member access is allowed */ protected boolean checkAllowlist(Object target, Member member) { + if (!disallowProxyObjectAccess && target != null && ProxyUtil.isProxy(target)) { + // If `disallowProxyObjectAccess` is not set, allow resolving Hibernate entities to their underlying + // classes/members. This allows the allowlist capability to continue working and offer some level of + // protection in applications where the developer has accepted the risk of allowing OGNL access to Hibernate + // entities. This is preferred to having to disable the allowlist capability entirely. + Object newTarget = ProxyUtil.getHibernateProxyTarget(target); + if (newTarget != target) { + target = newTarget; + member = ProxyUtil.resolveTargetMember(member, newTarget); + } + } + Class memberClass = member.getDeclaringClass(); if (!enforceAllowlistEnabled) { return true; diff --git a/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java b/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java index c169af20b..895cfb7ee 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java @@ -24,6 +24,7 @@ import com.opensymphony.xwork2.ognl.OgnlCacheFactory; import org.apache.commons.lang3.reflect.ConstructorUtils; import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.commons.lang3.reflect.MethodUtils; +import org.hibernate.Hibernate; import org.hibernate.proxy.HibernateProxy; import java.lang.reflect.Constructor; @@ -33,6 +34,8 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.lang.reflect.Proxy; +import static java.lang.reflect.Modifier.isPublic; + /** * ProxyUtil *

@@ -255,4 +258,34 @@ public class ProxyUtil { return false; } + + /** + * @return the target instance of the given object if it is a Hibernate proxy object, otherwise the given object + */ + public static Object getHibernateProxyTarget(Object object) { + try { + return Hibernate.unproxy(object); + } catch (NoClassDefFoundError ignored) { + return object; + } + } + + /** + * @return matching member on target object if one exists, otherwise the same member + */ + public static Member resolveTargetMember(Member proxyMember, Object target) { + int mod = proxyMember.getModifiers(); + if (proxyMember instanceof Method) { + if (isPublic(mod)) { + return MethodUtils.getMatchingAccessibleMethod(target.getClass(), proxyMember.getName(), ((Method) proxyMember).getParameterTypes()); + } else { + return MethodUtils.getMatchingMethod(target.getClass(), proxyMember.getName(), ((Method) proxyMember).getParameterTypes()); + } + } else if (proxyMember instanceof Field) { + return FieldUtils.getField(target.getClass(), proxyMember.getName(), isPublic(mod)); + } else if (proxyMember instanceof Constructor && isPublic(mod)) { + return ConstructorUtils.getMatchingAccessibleConstructor(target.getClass(), ((Constructor) proxyMember).getParameterTypes()); + } + return proxyMember; + } } From abf03fdccc9f25a52abeb1c24d944b6b1bf74b72 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 8 Jul 2024 15:56:04 +1000 Subject: [PATCH 11/16] WW-5428 Clean up SecurityMemberAccessProxyTest --- .../ognl/SecurityMemberAccessProxyTest.java | 104 +++++++++--------- 1 file changed, 50 insertions(+), 54 deletions(-) diff --git a/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java b/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java index 3838ca9ae..885665a12 100644 --- a/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java +++ b/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java @@ -19,76 +19,72 @@ package com.opensymphony.xwork2.ognl; import com.opensymphony.xwork2.ActionProxy; -import com.opensymphony.xwork2.XWorkTestCase; +import com.opensymphony.xwork2.XWorkJUnit4TestCase; import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider; import org.apache.struts2.config.StrutsXmlConfigurationProvider; +import org.junit.Before; +import org.junit.Test; import java.lang.reflect.Member; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; -public class SecurityMemberAccessProxyTest extends XWorkTestCase { +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class SecurityMemberAccessProxyTest extends XWorkJUnit4TestCase { + + private static final String PROXY_MEMBER_METHOD = "isExposeProxy"; + private static final String TEST_SUB_BEAN_CLASS_METHOD = "getIssueId"; + private Map context; private ActionProxy proxy; - private Map members; - private final SecurityMemberAccess sma = new SecurityMemberAccess(true); - private final String PROXY_MEMBER_METHOD = "isExposeProxy"; - private final String TEST_SUB_BEAN_CLASS_METHOD = "setIssueId"; + private final SecurityMemberAccess sma = new SecurityMemberAccess(null, null); + private Member proxyObjectProxyMember; + private Member proxyObjectNonProxyMember; + + @Before @Override public void setUp() throws Exception { - super.setUp(); - - context = new HashMap<>(); - // Set up XWork XmlConfigurationProvider provider = new StrutsXmlConfigurationProvider("com/opensymphony/xwork2/spring/actionContext-xwork.xml"); - container.inject(provider); loadConfigurationProviders(provider); - // Setup proxy object - setupProxy(); - } - - public void testProxyAccessIsBlocked() throws Exception { - members.values().forEach(member -> { - // When disallowProxyObjectAccess is set to true, and disallowProxyMemberAccess is set to false, the proxy access is blocked - sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString()); - sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString()); - assertFalse(sma.isAccessible(context, proxy.getAction(), member, "")); - - // When disallowProxyObjectAccess is set to true, and disallowProxyMemberAccess is set to true, the proxy access is blocked - sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString()); - sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString()); - assertFalse(sma.isAccessible(context, proxy.getAction(), member, "")); - }); - - // When disallowProxyObjectAccess is set to false, and disallowProxyMemberAccess is set to true, the proxy member access is blocked - sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString()); - sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString()); - assertFalse(sma.isAccessible(context, proxy.getAction(), members.get(PROXY_MEMBER_METHOD), "")); - } - - public void testProxyAccessIsAccessible() throws Exception { - members.values().forEach(member -> { - // When disallowProxyObjectAccess is set to false, and disallowProxyMemberAccess is set to false, the proxy access is allowed - sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString()); - sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString()); - assertTrue(sma.isAccessible(context, proxy.getAction(), member, "")); - }); - - // When disallowProxyObjectAccess is set to false, and disallowProxyMemberAccess is set to true, the original class member access is allowed - sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString()); - sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString()); - assertTrue(sma.isAccessible(context, proxy.getAction(), members.get(TEST_SUB_BEAN_CLASS_METHOD), "")); - } - - private void setupProxy() throws NoSuchMethodException { + context = new HashMap<>(); proxy = actionProxyFactory.createActionProxy(null, "chaintoAOPedTestSubBeanAction", null, context); + proxyObjectProxyMember = proxy.getAction().getClass().getMethod(PROXY_MEMBER_METHOD); + proxyObjectNonProxyMember = proxy.getAction().getClass().getMethod(TEST_SUB_BEAN_CLASS_METHOD); + } - members = new HashMap<>(); - // method is proxy member - members.put(PROXY_MEMBER_METHOD, proxy.getAction().getClass().getMethod(PROXY_MEMBER_METHOD)); - // method is not proxy member but from POJO class - members.put(TEST_SUB_BEAN_CLASS_METHOD, proxy.getAction().getClass().getMethod(TEST_SUB_BEAN_CLASS_METHOD, String.class)); + /** + * When {@code disallowProxyObjectAccess} is {@code true}, proxy access is blocked irrespective of + * {@code disallowProxyMemberAccess} value and irrespective of whether the member itself originates from the proxy. + */ + @Test + public void disallowProxyObjectAccess() { + sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString()); + Arrays.asList(proxyObjectProxyMember, proxyObjectNonProxyMember).forEach(member -> + Arrays.asList(Boolean.TRUE, Boolean.FALSE).forEach(disallowProxyMemberAccess -> { + sma.useDisallowProxyMemberAccess(disallowProxyMemberAccess.toString()); + assertFalse(sma.isAccessible(context, proxy.getAction(), member, "")); + }) + ); + } + + @Test + public void disallowProxyMemberAccess() { + sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString()); + sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString()); + assertFalse(sma.isAccessible(context, proxy.getAction(), proxyObjectProxyMember, "")); + assertTrue(sma.isAccessible(context, proxy.getAction(), proxyObjectNonProxyMember, "")); + } + + @Test + public void allowAllProxyAccess() { + sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString()); + sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString()); + assertTrue(sma.isAccessible(context, proxy.getAction(), proxyObjectProxyMember, "")); + assertTrue(sma.isAccessible(context, proxy.getAction(), proxyObjectNonProxyMember, "")); } } From c965812ffeb87692089fa2174bea2f209b2ed8f4 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 8 Jul 2024 16:47:14 +1000 Subject: [PATCH 12/16] WW-5428 Add unit test coverage for Hibernate proxy resolution --- .../xwork2/ognl/SecurityMemberAccess.java | 7 +- .../xwork2/ognl/SecurityMemberAccessTest.java | 81 ++++++++++++++++++- 2 files changed, 83 insertions(+), 5 deletions(-) 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 fc74f9a14..af95a37df 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -209,6 +209,10 @@ public class SecurityMemberAccess implements MemberAccess { * @return {@code true} if member access is allowed */ protected boolean checkAllowlist(Object target, Member member) { + if (!enforceAllowlistEnabled) { + return true; + } + if (!disallowProxyObjectAccess && target != null && ProxyUtil.isProxy(target)) { // If `disallowProxyObjectAccess` is not set, allow resolving Hibernate entities to their underlying // classes/members. This allows the allowlist capability to continue working and offer some level of @@ -222,9 +226,6 @@ public class SecurityMemberAccess implements MemberAccess { } Class memberClass = member.getDeclaringClass(); - if (!enforceAllowlistEnabled) { - return true; - } if (!isClassAllowlisted(memberClass)) { LOG.warn(format("Declaring class [{0}] of member type [{1}] is not allowlisted!", memberClass, member)); return false; 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 381b7d0ad..d508ef99d 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java @@ -26,12 +26,16 @@ import ognl.MemberAccess; import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.struts2.ognl.ProviderAllowlist; import org.apache.struts2.ognl.ThreadAllowlist; +import org.hibernate.proxy.HibernateProxy; +import org.hibernate.proxy.LazyInitializer; import org.junit.Before; import org.junit.Test; import java.lang.reflect.Field; +import java.lang.reflect.InvocationHandler; import java.lang.reflect.Member; import java.lang.reflect.Method; +import java.lang.reflect.Proxy; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; @@ -853,9 +857,11 @@ public class SecurityMemberAccessTest { assertTrue("package java.lang. is accessible!", actual); } + /** + * Test that the allowlist is enforced correctly for classes. + */ @Test public void classInclusion() throws Exception { - sma.useEnforceAllowlistEnabled(Boolean.TRUE.toString()); TestBean2 bean = new TestBean2(); @@ -868,6 +874,9 @@ public class SecurityMemberAccessTest { assertTrue(sma.checkAllowlist(bean, method)); } + /** + * Test that the allowlist is enforced correctly for packages. + */ @Test public void packageInclusion() throws Exception { sma.useEnforceAllowlistEnabled(Boolean.TRUE.toString()); @@ -882,6 +891,9 @@ public class SecurityMemberAccessTest { assertTrue(sma.checkAllowlist(bean, method)); } + /** + * Test that the allowlist doesn't allow inherited methods unless the declaring class is also allowlisted. + */ @Test public void classInclusion_subclass() throws Exception { sma.useEnforceAllowlistEnabled(Boolean.TRUE.toString()); @@ -893,6 +905,9 @@ public class SecurityMemberAccessTest { assertFalse(sma.checkAllowlist(bean, method)); } + /** + * Test that the allowlist allows inherited methods when both the target and declaring class are allowlisted. + */ @Test public void classInclusion_subclass_both() throws Exception { sma.useEnforceAllowlistEnabled(Boolean.TRUE.toString()); @@ -904,6 +919,10 @@ public class SecurityMemberAccessTest { assertTrue(sma.checkAllowlist(bean, method)); } + /** + * Test that the allowlist doesn't allow inherited methods unless the package of the declaring class is also + * allowlisted. + */ @Test public void packageInclusion_subclass() throws Exception { sma.useEnforceAllowlistEnabled(Boolean.TRUE.toString()); @@ -915,6 +934,37 @@ public class SecurityMemberAccessTest { assertFalse(sma.checkAllowlist(bean, method)); } + /** + * When the allowlist is enabled and proxy object access is disallowed, Hibernate proxies should not be allowed. + */ + @Test + public void classInclusion_hibernateProxy_disallowProxyObjectAccess() throws Exception { + FooBarInterface proxyObject = mockHibernateProxy(new FooBar(), FooBarInterface.class); + Method proxyMethod = proxyObject.getClass().getMethod("fooLogic"); + + sma.useEnforceAllowlistEnabled(Boolean.TRUE.toString()); + sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString()); + sma.useAllowlistClasses(FooBar.class.getName()); + + assertFalse(sma.checkAllowlist(proxyObject, proxyMethod)); + } + + /** + * When the allowlist is enabled and proxy object access is allowed, Hibernate proxies should be allowlisted based + * on their underlying target object. Class allowlisting should work as expected. + */ + @Test + public void classInclusion_hibernateProxy_allowProxyObjectAccess() throws Exception { + FooBarInterface proxyObject = mockHibernateProxy(new FooBar(), FooBarInterface.class); + Method proxyMethod = proxyObject.getClass().getMethod("fooLogic"); + + sma.useEnforceAllowlistEnabled(Boolean.TRUE.toString()); + sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString()); + sma.useAllowlistClasses(FooBar.class.getName()); + + assertTrue(sma.checkAllowlist(proxyObject, proxyMethod)); + } + @Test public void packageInclusion_subclass_both() throws Exception { sma.useEnforceAllowlistEnabled(Boolean.TRUE.toString()); @@ -931,6 +981,15 @@ public class SecurityMemberAccessTest { private static String formGetterName(String propertyName) { return "get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1); } + + @SuppressWarnings("unchecked") + private static T mockHibernateProxy(T originalObject, Class proxyInterface) { + return (T) Proxy.newProxyInstance( + proxyInterface.getClassLoader(), + new Class[]{proxyInterface, HibernateProxy.class}, + new DummyHibernateProxyHandler(originalObject) + ); + } } class FooBar implements FooBarInterface { @@ -1042,10 +1101,28 @@ class StaticTester { } protected static Field getFieldByName(String fieldName) throws NoSuchFieldException { - if (fieldName != null && fieldName.length() > 0) { + if (fieldName != null && !fieldName.isEmpty()) { return StaticTester.class.getDeclaredField(fieldName); } else { throw new NoSuchFieldException("field: " + fieldName + " does not exist"); } } } + +class DummyHibernateProxyHandler implements InvocationHandler { + private final Object instance; + + public DummyHibernateProxyHandler(Object instance) { + this.instance = instance; + } + + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + if (HibernateProxy.class.getMethod("getHibernateLazyInitializer").equals(method)) { + LazyInitializer initializer = mock(LazyInitializer.class); + when(initializer.getImplementation()).thenReturn(instance); + return initializer; + } + return method.invoke(instance, args); + } +} From c6f394a0e83caaa2168c1f1cf9ae34dd4f9cd960 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 8 Jul 2024 19:44:05 +1000 Subject: [PATCH 13/16] WW-5428 Add log warning for Hibernate entities --- .../xwork2/ognl/SecurityMemberAccess.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) 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 af95a37df..a0c048a46 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -77,16 +77,23 @@ 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(); + private Set excludedClasses = unmodifiableSet(new HashSet<>(singletonList(Object.class.getName()))); private Set excludedPackageNamePatterns = emptySet(); private Set excludedPackageNames = emptySet(); private Set excludedPackageExemptClasses = emptySet(); + + private boolean isDevMode; + private boolean enforceAllowlistEnabled = false; private Set> allowlistClasses = emptySet(); private Set allowlistPackageNames = emptySet(); + private boolean disallowProxyObjectAccess = false; private boolean disallowProxyMemberAccess = false; private boolean disallowDefaultPackageAccess = false; @@ -220,6 +227,7 @@ public class SecurityMemberAccess implements MemberAccess { // entities. This is preferred to having to disable the allowlist capability entirely. Object newTarget = ProxyUtil.getHibernateProxyTarget(target); if (newTarget != target) { + logAllowlistHibernateEntity(target, newTarget); target = newTarget; member = ProxyUtil.resolveTargetMember(member, newTarget); } @@ -241,6 +249,21 @@ public class SecurityMemberAccess implements MemberAccess { return true; } + private void logAllowlistHibernateEntity(Object original, Object resolved) { + if (!isDevMode && !LOG.isDebugEnabled()) { + return; + } + String msg = "Hibernate entity [{}] resolved to [{}] for purpose of OGNL allowlisting." + + " We don't recommend executing OGNL expressions against Hibernate entities, you may disallow this behaviour using the configuration `{}=true`."; + Object[] args = {original, resolved, StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS}; + if (isDevMode) { + LOG.warn(msg, args); + } else { + LOG.debug(msg, args); + } + + } + protected boolean isClassAllowlisted(Class clazz) { return allowlistClasses.contains(clazz) || ALLOWLIST_REQUIRED_CLASSES.contains(clazz) @@ -473,4 +496,9 @@ public class SecurityMemberAccess implements MemberAccess { public void useDisallowDefaultPackageAccess(String disallowDefaultPackageAccess) { this.disallowDefaultPackageAccess = BooleanUtils.toBoolean(disallowDefaultPackageAccess); } + + @Inject(StrutsConstants.STRUTS_DEVMODE) + protected void useDevMode(String devMode) { + this.isDevMode = BooleanUtils.toBoolean(devMode); + } } From 8555dc266ef4a1013a651aaaa04f4894df4ffbc6 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 8 Jul 2024 19:52:22 +1000 Subject: [PATCH 14/16] WW-5428 Add log warning for allowlist disabled --- .../xwork2/ognl/SecurityMemberAccess.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) 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 a0c048a46..97b9cf952 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -217,6 +217,7 @@ public class SecurityMemberAccess implements MemberAccess { */ protected boolean checkAllowlist(Object target, Member member) { if (!enforceAllowlistEnabled) { + logAllowlistDisabled(); return true; } @@ -249,6 +250,21 @@ public class SecurityMemberAccess implements MemberAccess { return true; } + private void logAllowlistDisabled() { + if (!isDevMode && !LOG.isDebugEnabled()) { + return; + } + String msg = "OGNL allowlist is disabled!" + + " We strongly recommend keeping it enabled to protect against critical vulnerabilities." + + " Set the configuration `{0}=true` to enable it."; + Object[] args = {StrutsConstants.STRUTS_ALLOWLIST_ENABLE}; + if (isDevMode) { + LOG.warn(msg, args); + } else { + LOG.debug(msg, args); + } + } + private void logAllowlistHibernateEntity(Object original, Object resolved) { if (!isDevMode && !LOG.isDebugEnabled()) { return; @@ -261,7 +277,6 @@ public class SecurityMemberAccess implements MemberAccess { } else { LOG.debug(msg, args); } - } protected boolean isClassAllowlisted(Class clazz) { From 05680d78271f967e04192c221e7144258524e0fe Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 8 Jul 2024 19:57:36 +1000 Subject: [PATCH 15/16] WW-5428 Amend log warning for missing allowlist entry --- .../xwork2/ognl/SecurityMemberAccess.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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 97b9cf952..0c16ca1a6 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -51,6 +51,8 @@ import static java.text.MessageFormat.format; import static java.util.Collections.emptySet; import static java.util.Collections.singletonList; import static java.util.Collections.unmodifiableSet; +import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_CLASSES; +import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES; /** * Allows access decisions to be made on the basis of whether a member is static or not. @@ -236,7 +238,8 @@ public class SecurityMemberAccess implements MemberAccess { Class memberClass = member.getDeclaringClass(); if (!isClassAllowlisted(memberClass)) { - LOG.warn(format("Declaring class [{0}] of member type [{1}] is not allowlisted!", memberClass, member)); + LOG.warn("Declaring class [{}] of member type [{}] is not allowlisted! Add to '{}' or '{}' configuration.", + memberClass, member, STRUTS_ALLOWLIST_CLASSES, STRUTS_ALLOWLIST_PACKAGE_NAMES); return false; } if (target == null || target.getClass() == memberClass) { @@ -244,7 +247,8 @@ public class SecurityMemberAccess implements MemberAccess { } Class targetClass = target.getClass(); if (!isClassAllowlisted(targetClass)) { - LOG.warn(format("Target class [{0}] of target [{1}] is not allowlisted!", targetClass, target)); + LOG.warn("Target class [{}] of target [{}] is not allowlisted! Add to '{}' or '{}' configuration.", + targetClass, target, STRUTS_ALLOWLIST_CLASSES, STRUTS_ALLOWLIST_PACKAGE_NAMES); return false; } return true; @@ -487,12 +491,12 @@ public class SecurityMemberAccess implements MemberAccess { this.enforceAllowlistEnabled = BooleanUtils.toBoolean(enforceAllowlistEnabled); } - @Inject(value = StrutsConstants.STRUTS_ALLOWLIST_CLASSES, required = false) + @Inject(value = STRUTS_ALLOWLIST_CLASSES, required = false) public void useAllowlistClasses(String commaDelimitedClasses) { this.allowlistClasses = toClassObjectsSet(commaDelimitedClasses); } - @Inject(value = StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES, required = false) + @Inject(value = STRUTS_ALLOWLIST_PACKAGE_NAMES, required = false) public void useAllowlistPackageNames(String commaDelimitedPackageNames) { this.allowlistPackageNames = toPackageNamesSet(commaDelimitedPackageNames); } From 81b49431764d88e2f26ae3f82b372a104d15cbf4 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 8 Jul 2024 18:42:06 +1000 Subject: [PATCH 16/16] WW-5439 Move Dev Mode security configuration --- .../opensymphony/xwork2/ognl/OgnlUtil.java | 53 +++++++++---------- .../xwork2/ognl/SecurityMemberAccess.java | 38 +++++++++++++ .../xwork2/ognl/OgnlUtilTest.java | 36 ++++++++----- 3 files changed, 86 insertions(+), 41 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index 681aac57d..78cada96d 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -47,7 +47,6 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.regex.Pattern; import static com.opensymphony.xwork2.util.ConfigParseUtil.toClassesSet; @@ -68,9 +67,6 @@ public class OgnlUtil { private static final Logger LOG = LogManager.getLogger(OgnlUtil.class); - // Flag used to reduce flooding logs with WARNs about using DevMode excluded packages - private final AtomicBoolean warnReported = new AtomicBoolean(false); - private final OgnlCache expressionCache; private final OgnlCache, BeanInfo> beanInfoCache; private TypeConverter defaultConverter; @@ -80,11 +76,6 @@ public class OgnlUtil { private boolean enableExpressionCache = true; private boolean enableEvalExpression; - private String devModeExcludedClasses = ""; - private String devModeExcludedPackageNamePatterns = ""; - private String devModeExcludedPackageNames = ""; - private String devModeExcludedPackageExemptClasses = ""; - private Container container; /** @@ -164,9 +155,12 @@ public class OgnlUtil { // Must be set directly on SecurityMemberAccess } - @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, required = false) + /** + * @deprecated since 6.5.0, no replacement. + */ + @Deprecated protected void setDevModeExcludedClasses(String commaDelimitedClasses) { - this.devModeExcludedClasses = commaDelimitedClasses; + // Must be set directly on SecurityMemberAccess } /** @@ -177,9 +171,12 @@ public class OgnlUtil { // Must be set directly on SecurityMemberAccess } - @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false) + /** + * @deprecated since 6.5.0, no replacement. + */ + @Deprecated protected void setDevModeExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) { - this.devModeExcludedPackageNamePatterns = commaDelimitedPackagePatterns; + // Must be set directly on SecurityMemberAccess } /** @@ -190,9 +187,12 @@ public class OgnlUtil { // Must be set directly on SecurityMemberAccess } - @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, required = false) + /** + * @deprecated since 6.5.0, no replacement. + */ + @Deprecated protected void setDevModeExcludedPackageNames(String commaDelimitedPackageNames) { - this.devModeExcludedPackageNames = commaDelimitedPackageNames; + // Must be set directly on SecurityMemberAccess } /** @@ -203,9 +203,12 @@ public class OgnlUtil { // Must be set directly on SecurityMemberAccess } - @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false) + /** + * @deprecated since 6.5.0, no replacement. + */ + @Deprecated public void setDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) { - this.devModeExcludedPackageExemptClasses = commaDelimitedClasses; + // Must be set directly on SecurityMemberAccess } /** @@ -856,6 +859,11 @@ public class OgnlUtil { return createDefaultContext(root, null); } + /** + * Note that the allowlist capability is not enforced by the {@link OgnlContext} returned by this method. Currently, + * this context is only leveraged by some public methods on {@link OgnlUtil} which are called by + * {@link OgnlReflectionProvider}. + */ protected Map createDefaultContext(Object root, ClassResolver resolver) { if (resolver == null) { resolver = container.getInstance(RootAccessor.class); @@ -867,17 +875,6 @@ public class OgnlUtil { SecurityMemberAccess memberAccess = container.getInstance(SecurityMemberAccess.class); memberAccess.useEnforceAllowlistEnabled(Boolean.FALSE.toString()); - if (devMode) { - if (!warnReported.get()) { - warnReported.set(true); - LOG.warn("Working in devMode, using devMode excluded classes and packages!"); - } - memberAccess.useExcludedClasses(devModeExcludedClasses); - memberAccess.useExcludedPackageNamePatterns(devModeExcludedPackageNamePatterns); - memberAccess.useExcludedPackageNames(devModeExcludedPackageNames); - memberAccess.useExcludedPackageExemptClasses(devModeExcludedPackageExemptClasses); - } - return Ognl.createDefaultContext(root, memberAccess, resolver, defaultConverter); } 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 0c16ca1a6..f225b3c89 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -90,7 +90,12 @@ public class SecurityMemberAccess implements MemberAccess { private Set excludedPackageNames = emptySet(); private Set excludedPackageExemptClasses = emptySet(); + private volatile boolean isDevModeInit; private boolean isDevMode; + private Set devModeExcludedClasses = unmodifiableSet(new HashSet<>(singletonList(Object.class.getName()))); + private Set devModeExcludedPackageNamePatterns = emptySet(); + private Set devModeExcludedPackageNames = emptySet(); + private Set devModeExcludedPackageExemptClasses = emptySet(); private boolean enforceAllowlistEnabled = false; private Set> allowlistClasses = emptySet(); @@ -296,6 +301,7 @@ public class SecurityMemberAccess implements MemberAccess { * @return {@code true} if member access is allowed */ protected boolean checkExclusionList(Object target, Member member) { + useDevModeConfiguration(); Class memberClass = member.getDeclaringClass(); if (isClassExcluded(memberClass)) { LOG.warn("Declaring class of member type [{}] is excluded!", memberClass); @@ -520,4 +526,36 @@ public class SecurityMemberAccess implements MemberAccess { protected void useDevMode(String devMode) { this.isDevMode = BooleanUtils.toBoolean(devMode); } + + @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, required = false) + public void useDevModeExcludedClasses(String commaDelimitedClasses) { + this.devModeExcludedClasses = toNewClassesSet(devModeExcludedClasses, commaDelimitedClasses); + } + + @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false) + public void useDevModeExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) { + this.devModeExcludedPackageNamePatterns = toNewPatternsSet(devModeExcludedPackageNamePatterns, commaDelimitedPackagePatterns); + } + + @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, required = false) + public void useDevModeExcludedPackageNames(String commaDelimitedPackageNames) { + this.devModeExcludedPackageNames = toNewPackageNamesSet(devModeExcludedPackageNames, commaDelimitedPackageNames); + } + + @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false) + public void useDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) { + this.devModeExcludedPackageExemptClasses = toClassesSet(commaDelimitedClasses); + } + + private void useDevModeConfiguration() { + if (!isDevMode || isDevModeInit) { + return; + } + isDevModeInit = true; + LOG.warn("Working in devMode, using devMode excluded classes and packages!"); + excludedClasses = devModeExcludedClasses; + excludedPackageNamePatterns = devModeExcludedPackageNamePatterns; + excludedPackageNames = devModeExcludedPackageNames; + excludedPackageExemptClasses = devModeExcludedPackageExemptClasses; + } } diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index b8e100ee9..27a0d0f33 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -80,6 +80,11 @@ public class OgnlUtilTest extends XWorkTestCase { ognlUtil = container.getInstance(OgnlUtil.class); } + private void resetOgnlUtil(Map properties) { + loadButSet(properties); + ognlUtil = container.getInstance(OgnlUtil.class); + } + public void testCanSetADependentObject() { String dogName = "fido"; @@ -1152,8 +1157,8 @@ public class OgnlUtilTest extends XWorkTestCase { Exception expected = null; try { - ognlUtil.setExcludedClasses(Object.class.getName()); - ognlUtil.setValue("class.classLoader.defaultAssertionStatus", ognlUtil.createDefaultContext(foo), foo, true); + // Object.class is excluded by default + ognlUtil.setValue("class.classLoader", ognlUtil.createDefaultContext(foo), foo, true); fail(); } catch (OgnlException e) { expected = e; @@ -1166,9 +1171,11 @@ public class OgnlUtilTest extends XWorkTestCase { public void testAllowCallingMethodsOnObjectClassInDevModeTrue() { Exception expected = null; try { - ognlUtil.setExcludedClasses(Foo.class.getName()); - ognlUtil.setDevModeExcludedClasses(""); - ognlUtil.setDevMode(Boolean.TRUE.toString()); + Map properties = new HashMap<>(); + properties.put(StrutsConstants.STRUTS_EXCLUDED_CLASSES, Foo.class.getName()); + properties.put(StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, ""); + properties.put(StrutsConstants.STRUTS_DEVMODE, Boolean.TRUE.toString()); + resetOgnlUtil(properties); Foo foo = new Foo(); String result = (String) ognlUtil.getValue("toString", ognlUtil.createDefaultContext(foo), foo, String.class); @@ -1180,14 +1187,18 @@ public class OgnlUtilTest extends XWorkTestCase { } public void testExclusionListDevModeOnOff() throws Exception { - ognlUtil.setDevModeExcludedClasses(Foo.class.getName()); Foo foo = new Foo(); - ognlUtil.setDevMode(Boolean.TRUE.toString()); + Map properties = new HashMap<>(); + properties.put(StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, Foo.class.getName()); + properties.put(StrutsConstants.STRUTS_DEVMODE, Boolean.TRUE.toString()); + resetOgnlUtil(properties); + OgnlException e = assertThrows(OgnlException.class, () -> ognlUtil.getValue("toString", ognlUtil.createDefaultContext(foo), foo, String.class)); assertThat(e).hasMessageContaining("com.opensymphony.xwork2.util.Foo.toString"); - ognlUtil.setDevMode(Boolean.FALSE.toString()); + properties.put(StrutsConstants.STRUTS_DEVMODE, Boolean.FALSE.toString()); + resetOgnlUtil(properties); assertEquals("Foo", (String) ognlUtil.getValue("toString", ognlUtil.createDefaultContext(foo), foo, String.class)); } @@ -1196,7 +1207,7 @@ public class OgnlUtilTest extends XWorkTestCase { Exception expected = null; try { - ognlUtil.setExcludedClasses(Object.class.getName()); + // Object.class is excluded by default ognlUtil.setValue("Class.ClassLoader.DefaultAssertionStatus", ognlUtil.createDefaultContext(foo), foo, true); fail(); } catch (OgnlException e) { @@ -1212,7 +1223,7 @@ public class OgnlUtilTest extends XWorkTestCase { Exception expected = null; try { - ognlUtil.setExcludedClasses(Object.class.getName()); + // Object.class is excluded by default ognlUtil.setValue("class['classLoader']['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true); fail(); } catch (OgnlException e) { @@ -1243,7 +1254,7 @@ public class OgnlUtilTest extends XWorkTestCase { Exception expected = null; try { - ognlUtil.setExcludedClasses(Object.class.getName()); + // Object.class is excluded by default ognlUtil.setValue("class[\"classLoader\"]['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true); fail(); } catch (OgnlException e) { @@ -1284,12 +1295,11 @@ public class OgnlUtilTest extends XWorkTestCase { assertEquals(expected.getMessage(), "Inappropriate OGNL expression: toString()"); } - public void testAvoidCallingSomeClasses() { + public void testStaticMethodBlocked() { Foo foo = new Foo(); Exception expected = null; try { - ognlUtil.setExcludedClasses(Runtime.class.getName()); ognlUtil.setValue("@java.lang.Runtime@getRuntime().exec('mate')", ognlUtil.createDefaultContext(foo), foo, true); fail(); } catch (OgnlException e) {