Skip to content

Commit 4e80f26

Browse files
lukaszlenartlukaszlenart
authored
Merge pull request #1187 from apache/fix/WW-5517-debug
WW-5517 Fixes <s:debug/> to be compatible with allowlist capability
2 parents 9ba5c09 + 8f4fa06 commit 4e80f26

File tree

10 files changed

+755
-148
lines changed

10 files changed

+755
-148
lines changed

‎core/src/main/java/org/apache/struts2/components/Debug.java

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
*/
1919
package org.apache.struts2.components;
2020

21+
import org.apache.commons.lang3.ClassUtils;
2122
import org.apache.struts2.inject.Inject;
23+
import org.apache.struts2.ognl.ThreadAllowlist;
24+
import org.apache.struts2.util.CompoundRoot;
2225
import org.apache.struts2.util.ValueStack;
2326
import org.apache.struts2.util.reflection.ReflectionProvider;
2427
import jakarta.servlet.http.HttpServletRequest;
@@ -40,6 +43,7 @@ public class Debug extends UIBean {
4043

4144
protected ReflectionProvider reflectionProvider;
4245

46+
private ThreadAllowlist threadAllowlist;
4347

4448
public Debug(ValueStack stack, HttpServletRequest request, HttpServletResponse response) {
4549
super(stack, request, response);
@@ -50,6 +54,11 @@ public void setReflectionProvider(ReflectionProvider prov) {
5054
this.reflectionProvider = prov;
5155
}
5256

57+
@Inject
58+
public void setThreadAllowlist(ThreadAllowlist threadAllowlist) {
59+
this.threadAllowlist = threadAllowlist;
60+
}
61+
5362
protected String getDefaultTemplate() {
5463
return TEMPLATE;
5564
}
@@ -59,16 +68,19 @@ public boolean start(Writer writer) {
5968

6069
if (showDebug()) {
6170
ValueStack stack = getStack();
62-
Iterator iter = stack.getRoot().iterator();
63-
List stackValues = new ArrayList(stack.getRoot().size());
71+
allowList(stack.getRoot());
72+
73+
Iterator<Object> iter = stack.getRoot().iterator();
74+
List<Object> stackValues = new ArrayList<>(stack.getRoot().size());
6475
while (iter.hasNext()) {
6576
Object o = iter.next();
66-
Map values;
77+
Map<String, Object> values;
6778
try {
6879
values = reflectionProvider.getBeanMap(o);
6980
} catch (Exception e) {
7081
throw new StrutsException("Caught an exception while getting the property values of " + o, e);
7182
}
83+
allowListClass(o);
7284
stackValues.add(new DebugMapEntry(o.getClass().getName(), values));
7385
}
7486

@@ -77,6 +89,16 @@ public boolean start(Writer writer) {
7789
return result;
7890
}
7991

92+
private void allowList(CompoundRoot root) {
93+
root.forEach(this::allowListClass);
94+
}
95+
96+
private void allowListClass(Object o) {
97+
threadAllowlist.allowClass(o.getClass());
98+
ClassUtils.getAllSuperclasses(o.getClass()).forEach(threadAllowlist::allowClass);
99+
ClassUtils.getAllInterfaces(o.getClass()).forEach(threadAllowlist::allowClass);
100+
}
101+
80102
@Override
81103
public boolean end(Writer writer, String body) {
82104
if (showDebug()) {
@@ -91,17 +113,17 @@ protected boolean showDebug() {
91113
return (devMode || Boolean.TRUE == PrepareOperations.getDevModeOverride());
92114
}
93115

94-
private static class DebugMapEntry implements Map.Entry {
95-
private final Object key;
116+
private static class DebugMapEntry implements Map.Entry<String, Object> {
117+
private final String key;
96118
private Object value;
97119

98-
DebugMapEntry(Object key, Object value) {
120+
DebugMapEntry(String key, Object value) {
99121
this.key = key;
100122
this.value = value;
101123
}
102124

103125
@Override
104-
public Object getKey() {
126+
public String getKey() {
105127
return key;
106128
}
107129

‎core/src/main/java/org/apache/struts2/interceptor/ExceptionMappingInterceptor.java

Lines changed: 55 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@
1818
*/
1919
package org.apache.struts2.interceptor;
2020

21+
import org.apache.logging.log4j.Level;
2122
import org.apache.logging.log4j.LogManager;
2223
import org.apache.logging.log4j.Logger;
2324
import org.apache.struts2.ActionInvocation;
2425
import org.apache.struts2.config.entities.ExceptionMappingConfig;
2526
import org.apache.struts2.dispatcher.HttpParameters;
27+
import org.apache.struts2.inject.Inject;
28+
import org.apache.struts2.ognl.ThreadAllowlist;
2629

2730
import java.util.List;
2831
import java.util.Map;
@@ -42,11 +45,11 @@
4245
* you make this interceptor the first interceptor on the stack, ensuring that it has full access to catch any
4346
* exception, even those caused by other interceptors.
4447
* </p>
45-
*
48+
* <p>
4649
* <!-- END SNIPPET: description -->
4750
*
4851
* <p><u>Interceptor parameters:</u></p>
49-
*
52+
* <p>
5053
* <!-- START SNIPPET: parameters -->
5154
*
5255
* <ul>
@@ -64,11 +67,11 @@
6467
* The parameters above enables us to log all thrown exceptions with stacktace in our own logfile,
6568
* and present a friendly webpage (with no stacktrace) to the end user.
6669
* </p>
67-
*
70+
* <p>
6871
* <!-- END SNIPPET: parameters -->
6972
*
7073
* <p><u>Extending the interceptor:</u></p>
71-
*
74+
* <p>
7275
* <!-- START SNIPPET: extending -->
7376
* <p>
7477
* If you want to add custom handling for publishing the Exception, you may override
@@ -158,11 +161,17 @@ public class ExceptionMappingInterceptor extends AbstractInterceptor {
158161

159162
private static final Logger LOG = LogManager.getLogger(ExceptionMappingInterceptor.class);
160163

164+
private transient ThreadAllowlist threadAllowlist;
165+
161166
protected Logger categoryLogger;
162167
protected boolean logEnabled = false;
163168
protected String logCategory;
164169
protected String logLevel;
165170

171+
@Inject
172+
public void setThreadAllowlist(ThreadAllowlist threadAllowlist) {
173+
this.threadAllowlist = threadAllowlist;
174+
}
166175

167176
public boolean isLogEnabled() {
168177
return logEnabled;
@@ -173,20 +182,20 @@ public void setLogEnabled(boolean logEnabled) {
173182
}
174183

175184
public String getLogCategory() {
176-
return logCategory;
177-
}
185+
return logCategory;
186+
}
178187

179-
public void setLogCategory(String logCatgory) {
180-
this.logCategory = logCatgory;
181-
}
188+
public void setLogCategory(String logCategory) {
189+
this.logCategory = logCategory;
190+
}
182191

183-
public String getLogLevel() {
184-
return logLevel;
185-
}
192+
public String getLogLevel() {
193+
return logLevel;
194+
}
186195

187-
public void setLogLevel(String logLevel) {
188-
this.logLevel = logLevel;
189-
}
196+
public void setLogLevel(String logLevel) {
197+
this.logLevel = logLevel;
198+
}
190199

191200
@Override
192201
public String intercept(ActionInvocation invocation) throws Exception {
@@ -200,13 +209,16 @@ public String intercept(ActionInvocation invocation) throws Exception {
200209
}
201210
List<ExceptionMappingConfig> exceptionMappings = invocation.getProxy().getConfig().getExceptionMappings();
202211
ExceptionMappingConfig mappingConfig = this.findMappingFromExceptions(exceptionMappings, e);
203-
if (mappingConfig != null && mappingConfig.getResult()!=null) {
212+
if (mappingConfig != null && mappingConfig.getResult() != null) {
204213
Map<String, String> mappingParams = mappingConfig.getParams();
205214
// create a mutable HashMap since some interceptors will remove parameters, and parameterMap is immutable
206215
HttpParameters parameters = HttpParameters.create(mappingParams).build();
207216
invocation.getInvocationContext().withParameters(parameters);
208217
result = mappingConfig.getResult();
209-
publishException(invocation, new ExceptionHolder(e));
218+
ExceptionHolder holder = new ExceptionHolder(e);
219+
threadAllowlist.allowClass(holder.getClass());
220+
threadAllowlist.allowClass(e.getClass());
221+
publishException(invocation, holder);
210222
} else {
211223
throw e;
212224
}
@@ -221,55 +233,45 @@ public String intercept(ActionInvocation invocation) throws Exception {
221233
* @param e the exception to log.
222234
*/
223235
protected void handleLogging(Exception e) {
224-
if (logCategory != null) {
225-
if (categoryLogger == null) {
226-
// init category logger
227-
categoryLogger = LogManager.getLogger(logCategory);
228-
}
229-
doLog(categoryLogger, e);
230-
} else {
231-
doLog(LOG, e);
232-
}
236+
if (logCategory != null) {
237+
if (categoryLogger == null) {
238+
// init category logger
239+
categoryLogger = LogManager.getLogger(logCategory);
240+
}
241+
doLog(categoryLogger, e);
242+
} else {
243+
doLog(LOG, e);
244+
}
233245
}
234246

235247
/**
236248
* Performs the actual logging.
237249
*
238-
* @param logger the provided logger to use.
239-
* @param e the exception to log.
250+
* @param logger the provided logger to use.
251+
* @param e the exception to log.
240252
*/
241253
protected void doLog(Logger logger, Exception e) {
242-
if (logLevel == null) {
243-
logger.debug(e.getMessage(), e);
244-
return;
245-
}
254+
if (logLevel == null) {
255+
logger.debug(e.getMessage(), e);
256+
return;
257+
}
246258

247-
if ("trace".equalsIgnoreCase(logLevel)) {
248-
logger.trace(e.getMessage(), e);
249-
} else if ("debug".equalsIgnoreCase(logLevel)) {
250-
logger.debug(e.getMessage(), e);
251-
} else if ("info".equalsIgnoreCase(logLevel)) {
252-
logger.info(e.getMessage(), e);
253-
} else if ("warn".equalsIgnoreCase(logLevel)) {
254-
logger.warn(e.getMessage(), e);
255-
} else if ("error".equalsIgnoreCase(logLevel)) {
256-
logger.error(e.getMessage(), e);
257-
} else if ("fatal".equalsIgnoreCase(logLevel)) {
258-
logger.fatal(e.getMessage(), e);
259-
} else {
260-
throw new IllegalArgumentException("LogLevel [" + logLevel + "] is not supported");
261-
}
259+
Level level = Level.getLevel(logLevel);
260+
if (level == null) {
261+
throw new IllegalArgumentException("LogLevel [" + logLevel + "] is not supported");
262+
}
263+
logger.log(level, e.getMessage(), e);
262264
}
263265

264266
/**
265267
* Try to find appropriate {@link ExceptionMappingConfig} based on provided Throwable
266268
*
267269
* @param exceptionMappings list of defined exception mappings
268-
* @param t caught exception
270+
* @param t caught exception
269271
* @return appropriate mapping or null
270272
*/
271273
protected ExceptionMappingConfig findMappingFromExceptions(List<ExceptionMappingConfig> exceptionMappings, Throwable t) {
272-
ExceptionMappingConfig config = null;
274+
ExceptionMappingConfig config = null;
273275
// Check for specific exception mappings.
274276
if (exceptionMappings != null) {
275277
int deepest = Integer.MAX_VALUE;
@@ -288,15 +290,15 @@ protected ExceptionMappingConfig findMappingFromExceptions(List<ExceptionMapping
288290
* Return the depth to the superclass matching. 0 means ex matches exactly. Returns -1 if there's no match.
289291
* Otherwise, returns depth. Lowest depth wins.
290292
*
291-
* @param exceptionMapping the mapping classname
292-
* @param t the cause
293+
* @param exceptionMapping the mapping classname
294+
* @param t the cause
293295
* @return the depth, if not found -1 is returned.
294296
*/
295297
public int getDepth(String exceptionMapping, Throwable t) {
296298
return getDepth(exceptionMapping, t.getClass(), 0);
297299
}
298300

299-
private int getDepth(String exceptionMapping, Class exceptionClass, int depth) {
301+
private int getDepth(String exceptionMapping, Class<?> exceptionClass, int depth) {
300302
if (exceptionClass.getName().contains(exceptionMapping)) {
301303
// Found it!
302304
return depth;
@@ -312,7 +314,7 @@ private int getDepth(String exceptionMapping, Class exceptionClass, int depth) {
312314
* Default implementation to handle ExceptionHolder publishing. Pushes given ExceptionHolder on the stack.
313315
* Subclasses may override this to customize publishing.
314316
*
315-
* @param invocation The invocation to publish Exception for.
317+
* @param invocation The invocation to publish Exception for.
316318
* @param exceptionHolder The exceptionHolder wrapping the Exception to publish.
317319
*/
318320
protected void publishException(ActionInvocation invocation, ExceptionHolder exceptionHolder) {

0 commit comments

Comments
 (0)