[jira] [Work logged] (WW-5408) Add option to NOT fallback to empty namespace when unresolved
[
https://issues.apache.org/jira/browse/WW-5408?focusedWorklogId=913816=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913816
]
ASF GitHub Bot logged work on WW-5408:
--
Author: ASF GitHub Bot
Created on: 10/Apr/24 05:10
Start Date: 10/Apr/24 05:10
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on code in PR #912:
URL: https://github.com/apache/struts/pull/912#discussion_r1558851276
##
core/src/main/java/org/apache/struts2/StrutsConstants.java:
##
@@ -230,6 +230,8 @@ public final class StrutsConstants {
public static final String STRUTS_XWORKCONVERTER = "struts.xworkConverter";
public static final String STRUTS_ALWAYS_SELECT_FULL_NAMESPACE =
"struts.mapper.alwaysSelectFullNamespace";
+/** Disable fallback to empty namespace when request namespace didn't
match any in action configuration */
+public static final String STRUTS_DISABLE_EMPTY_NAMESPACE_FALLBACK =
"struts.disableActionConfigFallbackToEmptyNamespace";
Review Comment:
Wouldn't be better to have `struts.actionConfig.fallbackToEmptyNamespace`
with default value set to `true` instead?
Issue Time Tracking
---
Worklog Id: (was: 913816)
Time Spent: 0.5h (was: 20m)
> Add option to NOT fallback to empty namespace when unresolved
> -
>
> Key: WW-5408
> URL: https://issues.apache.org/jira/browse/WW-5408
> Project: Struts 2
> Issue Type: Improvement
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Minor
> Fix For: 6.5.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Currently, when a namespace cannot be resolved from a request URL, it falls
> back to the empty namespace.
> This effectively allows all Actions which are defined for the empty namespace
> to be accessed from an infinite number of endpoints.
> For example, you may have an Action defined in the empty namespace, intended
> for access at:
> {{www.domain.com/login.action}}
> However, due to the current fallback behaviour, this Action can actually be
> accessed at any non-resolving namespace, eg.:
> {{www.domain.com/what/about/this/login.action}}
> This behaviour is not usually beneficial and could lead to bugs if a
> developer only expects their Action to be accessible at a very specific URL.
> Many developers may not be aware of these Action resolving quirks of Struts.
> As far as I can tell, there is not currently an option to prevent this
> behaviour, so I propose we add one.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (WW-5406) Action excluded patterns are not updated following a configuration reload
[
https://issues.apache.org/jira/browse/WW-5406?focusedWorklogId=913820=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913820
]
ASF GitHub Bot logged work on WW-5406:
--
Author: ASF GitHub Bot
Created on: 10/Apr/24 05:53
Start Date: 10/Apr/24 05:53
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on code in PR #910:
URL: https://github.com/apache/struts/pull/910#discussion_r1558883281
##
core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareFilter.java:
##
@@ -43,6 +43,8 @@ public class StrutsPrepareFilter implements StrutsStatics,
Filter {
protected static final String REQUEST_EXCLUDED_FROM_ACTION_MAPPING =
StrutsPrepareFilter.class.getName() + ".REQUEST_EXCLUDED_FROM_ACTION_MAPPING";
protected PrepareOperations prepare;
+
+@Deprecated
Review Comment:
We can have one general ticket to remove all the deprecated options, just to
be sure we won't miss this one at some point (so `since` would be needed)
Issue Time Tracking
---
Worklog Id: (was: 913820)
Time Spent: 1h 20m (was: 1h 10m)
> Action excluded patterns are not updated following a configuration reload
> -
>
> Key: WW-5406
> URL: https://issues.apache.org/jira/browse/WW-5406
> Project: Struts 2
> Issue Type: Bug
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Minor
> Fix For: 6.5.0
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> If {{struts.action.excludePattern}} or
> {{struts.action.excludePattern.separator}} are updated during runtime, the
> changes are not reflected in the application behaviour due to these constants
> only being read exactly once. This is not consistent with all other
> configuration which is re-injected following a configuration reload.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (WW-5408) Add option to NOT fallback to empty namespace when unresolved
[
https://issues.apache.org/jira/browse/WW-5408?focusedWorklogId=913818=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913818
]
ASF GitHub Bot logged work on WW-5408:
--
Author: ASF GitHub Bot
Created on: 10/Apr/24 05:19
Start Date: 10/Apr/24 05:19
Worklog Time Spent: 10m
Work Description: kusalk commented on code in PR #912:
URL: https://github.com/apache/struts/pull/912#discussion_r1558857753
##
core/src/main/java/org/apache/struts2/StrutsConstants.java:
##
@@ -230,6 +230,8 @@ public final class StrutsConstants {
public static final String STRUTS_XWORKCONVERTER = "struts.xworkConverter";
public static final String STRUTS_ALWAYS_SELECT_FULL_NAMESPACE =
"struts.mapper.alwaysSelectFullNamespace";
+/** Disable fallback to empty namespace when request namespace didn't
match any in action configuration */
+public static final String STRUTS_DISABLE_EMPTY_NAMESPACE_FALLBACK =
"struts.disableActionConfigFallbackToEmptyNamespace";
Review Comment:
Sounds reasonable to me - @jefferyxhy could you please update?
Issue Time Tracking
---
Worklog Id: (was: 913818)
Time Spent: 40m (was: 0.5h)
> Add option to NOT fallback to empty namespace when unresolved
> -
>
> Key: WW-5408
> URL: https://issues.apache.org/jira/browse/WW-5408
> Project: Struts 2
> Issue Type: Improvement
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Minor
> Fix For: 6.5.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Currently, when a namespace cannot be resolved from a request URL, it falls
> back to the empty namespace.
> This effectively allows all Actions which are defined for the empty namespace
> to be accessed from an infinite number of endpoints.
> For example, you may have an Action defined in the empty namespace, intended
> for access at:
> {{www.domain.com/login.action}}
> However, due to the current fallback behaviour, this Action can actually be
> accessed at any non-resolving namespace, eg.:
> {{www.domain.com/what/about/this/login.action}}
> This behaviour is not usually beneficial and could lead to bugs if a
> developer only expects their Action to be accessible at a very specific URL.
> Many developers may not be aware of these Action resolving quirks of Struts.
> As far as I can tell, there is not currently an option to prevent this
> behaviour, so I propose we add one.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (WW-5406) Action excluded patterns are not updated following a configuration reload
[
https://issues.apache.org/jira/browse/WW-5406?focusedWorklogId=913817=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913817
]
ASF GitHub Bot logged work on WW-5406:
--
Author: ASF GitHub Bot
Created on: 10/Apr/24 05:19
Start Date: 10/Apr/24 05:19
Worklog Time Spent: 10m
Work Description: kusalk commented on code in PR #910:
URL: https://github.com/apache/struts/pull/910#discussion_r1558857128
##
core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareFilter.java:
##
@@ -43,6 +43,8 @@ public class StrutsPrepareFilter implements StrutsStatics,
Filter {
protected static final String REQUEST_EXCLUDED_FROM_ACTION_MAPPING =
StrutsPrepareFilter.class.getName() + ".REQUEST_EXCLUDED_FROM_ACTION_MAPPING";
protected PrepareOperations prepare;
+
+@Deprecated
Review Comment:
Yep let me rectify
Regarding a ticket for the deletion - my assumption would be that all
deprecated elements are deleted in the next major version. Do we still require
individual follow-up issues in the case of a deprecation?
##
core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java:
##
@@ -340,6 +348,27 @@ public void setMultipartValidationRegex(String
multipartValidationRegex) {
this.multipartValidationPattern =
Pattern.compile(multipartValidationRegex);
}
+@Inject(value = StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN_SEPARATOR,
required = false)
+public void setActionExcludedPatternsSeparator(String separator) {
+this.actionExcludedPatternsSeparator = separator;
+}
+
+@Inject(value = StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN, required =
false)
+public void setActionExcludedPatterns(String excludedPatterns) {
+this.actionExcludedPatterns =
buildExcludedPatternsList(excludedPatterns, actionExcludedPatternsSeparator);
+}
+
+private static List buildExcludedPatternsList(String patterns,
String separator) {
+if (patterns == null || patterns.trim().isEmpty()) {
+return emptyList();
+}
+return
unmodifiableList(Arrays.stream(patterns.split(separator)).map(String::trim).map(Pattern::compile).collect(toList()));
+}
+
+public List getActionExcludedPatterns() {
+return actionExcludedPatterns;
+}
Review Comment:
Yep I think that's a fair call, I'll create an issue for it
Issue Time Tracking
---
Worklog Id: (was: 913817)
Time Spent: 1h (was: 50m)
> Action excluded patterns are not updated following a configuration reload
> -
>
> Key: WW-5406
> URL: https://issues.apache.org/jira/browse/WW-5406
> Project: Struts 2
> Issue Type: Bug
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Minor
> Fix For: 6.5.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> If {{struts.action.excludePattern}} or
> {{struts.action.excludePattern.separator}} are updated during runtime, the
> changes are not reflected in the application behaviour due to these constants
> only being read exactly once. This is not consistent with all other
> configuration which is re-injected following a configuration reload.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (WW-5407) Extend SecurityMemberAccess proxy detection to Hibernate proxies
[
https://issues.apache.org/jira/browse/WW-5407?focusedWorklogId=913813=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913813
]
ASF GitHub Bot logged work on WW-5407:
--
Author: ASF GitHub Bot
Created on: 10/Apr/24 04:56
Start Date: 10/Apr/24 04:56
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on code in PR #911:
URL: https://github.com/apache/struts/pull/911#discussion_r1558841813
##
core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java:
##
@@ -96,12 +105,41 @@ public static boolean isProxyMember(Member member, Object
object) {
return flag;
}
-boolean isProxyMember = isSpringProxyMember(member);
+boolean isProxyMember = isSpringProxyMember(member) ||
isHibernateProxyMember(member);
isProxyMemberCache.put(member, isProxyMember);
return isProxyMember;
}
+/**
+ * Check whether the given object is a Hibernate proxy.
+ *
+ * @param object the object to check
+ */
+public static boolean isHibernateProxy(Object object) {
+try {
+return HibernateProxy.class.isAssignableFrom(object.getClass());
+} catch (NoClassDefFoundError ignored) {
+return false;
+}
+}
+
+/**
+ * Check whether the given member is a member of a Hibernate proxy.
+ *
+ * @param member the member to check
+ */
+public static boolean isHibernateProxyMember(Member member) {
+try {
+Class clazz =
ClassLoaderUtil.loadClass(HIBERNATE_HIBERNATEPROXY_CLASS_NAME, ProxyUtil.class);
+if (hasMember(clazz, member))
+return true;
Review Comment:
Shouldn't be inlined?
```java
return hasMember(clazz, member);
```
Issue Time Tracking
---
Worklog Id: (was: 913813)
Time Spent: 1.5h (was: 1h 20m)
> Extend SecurityMemberAccess proxy detection to Hibernate proxies
>
>
> Key: WW-5407
> URL: https://issues.apache.org/jira/browse/WW-5407
> Project: Struts 2
> Issue Type: Improvement
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Minor
> Fix For: 6.5.0
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> The current option {{struts.disallowProxyMemberAccess}} does not have any
> logic to detect Hibernate proxies which may also present a security risk.
> Additionally, the current option only forbids access to members which
> originate from a proxy. However, it makes more sense to forbid access to
> proxy objects entirely. This is because proxying is often used for sensitive
> instances, application beans or Hibernate objects. None of which is safe to
> be accessed or manipulated via OGNL. Thus, let's introduce an additional
> option {{struts.disallowProxyObjectAccess}} which will offer stronger
> protection.
> Finally, the caching mechanism in the ProxyUtil class uses an unbounded map,
> this can potentially be attacked and lead to a memory leak or DoS. Let's
> replace it with a Caffeine cache as we have done previously for the OGNL
> expression cache.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (WW-5406) Action excluded patterns are not updated following a configuration reload
[
https://issues.apache.org/jira/browse/WW-5406?focusedWorklogId=913812=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913812
]
ASF GitHub Bot logged work on WW-5406:
--
Author: ASF GitHub Bot
Created on: 10/Apr/24 04:53
Start Date: 10/Apr/24 04:53
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on code in PR #910:
URL: https://github.com/apache/struts/pull/910#discussion_r1558837094
##
core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java:
##
@@ -340,6 +348,27 @@ public void setMultipartValidationRegex(String
multipartValidationRegex) {
this.multipartValidationPattern =
Pattern.compile(multipartValidationRegex);
}
+@Inject(value = StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN_SEPARATOR,
required = false)
+public void setActionExcludedPatternsSeparator(String separator) {
+this.actionExcludedPatternsSeparator = separator;
+}
+
+@Inject(value = StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN, required =
false)
+public void setActionExcludedPatterns(String excludedPatterns) {
+this.actionExcludedPatterns =
buildExcludedPatternsList(excludedPatterns, actionExcludedPatternsSeparator);
+}
+
+private static List buildExcludedPatternsList(String patterns,
String separator) {
+if (patterns == null || patterns.trim().isEmpty()) {
+return emptyList();
+}
+return
unmodifiableList(Arrays.stream(patterns.split(separator)).map(String::trim).map(Pattern::compile).collect(toList()));
+}
+
+public List getActionExcludedPatterns() {
+return actionExcludedPatterns;
+}
Review Comment:
It's getting crowd'y here, what about moving all the injectable options into
`DispatcherOptions`? It doesn't have to happen now, but I can create a ticket
to address that later
##
core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareFilter.java:
##
@@ -43,6 +43,8 @@ public class StrutsPrepareFilter implements StrutsStatics,
Filter {
protected static final String REQUEST_EXCLUDED_FROM_ACTION_MAPPING =
StrutsPrepareFilter.class.getName() + ".REQUEST_EXCLUDED_FROM_ACTION_MAPPING";
protected PrepareOperations prepare;
+
+@Deprecated
Review Comment:
Please document why it's deprecated and what to use instead. Also `since`
should be added, and feel free to create a ticket in JIRA to remove this
deprecated element in Struts 7. [An example
deprecation](https://github.com/apache/struts/blob/master/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java#L176)
Issue Time Tracking
---
Worklog Id: (was: 913812)
Time Spent: 50m (was: 40m)
> Action excluded patterns are not updated following a configuration reload
> -
>
> Key: WW-5406
> URL: https://issues.apache.org/jira/browse/WW-5406
> Project: Struts 2
> Issue Type: Bug
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Minor
> Fix For: 6.5.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> If {{struts.action.excludePattern}} or
> {{struts.action.excludePattern.separator}} are updated during runtime, the
> changes are not reflected in the application behaviour due to these constants
> only being read exactly once. This is not consistent with all other
> configuration which is re-injected following a configuration reload.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Created] (WW-5410) Extract Dispatcher configuration into separate injectable class
Kusal Kithul-Godage created WW-5410:
---
Summary: Extract Dispatcher configuration into separate injectable
class
Key: WW-5410
URL: https://issues.apache.org/jira/browse/WW-5410
Project: Struts 2
Issue Type: Improvement
Components: Core
Reporter: Kusal Kithul-Godage
Fix For: 7.0.0
Currently {{core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java}}
contains a number of methods that exist purely for injecting and preparing
configuration. Let's extract such methods into a separate class to keep the
Dispatcher class clean.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (WW-5406) Action excluded patterns are not updated following a configuration reload
[
https://issues.apache.org/jira/browse/WW-5406?focusedWorklogId=913819=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913819
]
ASF GitHub Bot logged work on WW-5406:
--
Author: ASF GitHub Bot
Created on: 10/Apr/24 05:50
Start Date: 10/Apr/24 05:50
Worklog Time Spent: 10m
Work Description: kusalk commented on code in PR #910:
URL: https://github.com/apache/struts/pull/910#discussion_r1558881309
##
core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java:
##
@@ -340,6 +348,27 @@ public void setMultipartValidationRegex(String
multipartValidationRegex) {
this.multipartValidationPattern =
Pattern.compile(multipartValidationRegex);
}
+@Inject(value = StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN_SEPARATOR,
required = false)
+public void setActionExcludedPatternsSeparator(String separator) {
+this.actionExcludedPatternsSeparator = separator;
+}
+
+@Inject(value = StrutsConstants.STRUTS_ACTION_EXCLUDE_PATTERN, required =
false)
+public void setActionExcludedPatterns(String excludedPatterns) {
+this.actionExcludedPatterns =
buildExcludedPatternsList(excludedPatterns, actionExcludedPatternsSeparator);
+}
+
+private static List buildExcludedPatternsList(String patterns,
String separator) {
+if (patterns == null || patterns.trim().isEmpty()) {
+return emptyList();
+}
+return
unmodifiableList(Arrays.stream(patterns.split(separator)).map(String::trim).map(Pattern::compile).collect(toList()));
+}
+
+public List getActionExcludedPatterns() {
+return actionExcludedPatterns;
+}
Review Comment:
WW-5410
Issue Time Tracking
---
Worklog Id: (was: 913819)
Time Spent: 1h 10m (was: 1h)
> Action excluded patterns are not updated following a configuration reload
> -
>
> Key: WW-5406
> URL: https://issues.apache.org/jira/browse/WW-5406
> Project: Struts 2
> Issue Type: Bug
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Minor
> Fix For: 6.5.0
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> If {{struts.action.excludePattern}} or
> {{struts.action.excludePattern.separator}} are updated during runtime, the
> changes are not reflected in the application behaviour due to these constants
> only being read exactly once. This is not consistent with all other
> configuration which is re-injected following a configuration reload.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Work logged] (WW-5407) Extend SecurityMemberAccess proxy detection to Hibernate proxies
[
https://issues.apache.org/jira/browse/WW-5407?focusedWorklogId=913815=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913815
]
ASF GitHub Bot logged work on WW-5407:
--
Author: ASF GitHub Bot
Created on: 10/Apr/24 05:02
Start Date: 10/Apr/24 05:02
Worklog Time Spent: 10m
Work Description: jefferyxhy commented on code in PR #911:
URL: https://github.com/apache/struts/pull/911#discussion_r1558846553
##
core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java:
##
@@ -96,12 +105,41 @@ public static boolean isProxyMember(Member member, Object
object) {
return flag;
}
-boolean isProxyMember = isSpringProxyMember(member);
+boolean isProxyMember = isSpringProxyMember(member) ||
isHibernateProxyMember(member);
isProxyMemberCache.put(member, isProxyMember);
return isProxyMember;
}
+/**
+ * Check whether the given object is a Hibernate proxy.
+ *
+ * @param object the object to check
+ */
+public static boolean isHibernateProxy(Object object) {
+try {
+return HibernateProxy.class.isAssignableFrom(object.getClass());
+} catch (NoClassDefFoundError ignored) {
+return false;
+}
+}
+
+/**
+ * Check whether the given member is a member of a Hibernate proxy.
+ *
+ * @param member the member to check
+ */
+public static boolean isHibernateProxyMember(Member member) {
+try {
+Class clazz =
ClassLoaderUtil.loadClass(HIBERNATE_HIBERNATEPROXY_CLASS_NAME, ProxyUtil.class);
+if (hasMember(clazz, member))
+return true;
Review Comment:
@lukaszlenart updated. Thanks
Issue Time Tracking
---
Worklog Id: (was: 913815)
Time Spent: 1h 40m (was: 1.5h)
> Extend SecurityMemberAccess proxy detection to Hibernate proxies
>
>
> Key: WW-5407
> URL: https://issues.apache.org/jira/browse/WW-5407
> Project: Struts 2
> Issue Type: Improvement
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Minor
> Fix For: 6.5.0
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> The current option {{struts.disallowProxyMemberAccess}} does not have any
> logic to detect Hibernate proxies which may also present a security risk.
> Additionally, the current option only forbids access to members which
> originate from a proxy. However, it makes more sense to forbid access to
> proxy objects entirely. This is because proxying is often used for sensitive
> instances, application beans or Hibernate objects. None of which is safe to
> be accessed or manipulated via OGNL. Thus, let's introduce an additional
> option {{struts.disallowProxyObjectAccess}} which will offer stronger
> protection.
> Finally, the caching mechanism in the ProxyUtil class uses an unbounded map,
> this can potentially be attacked and lead to a memory leak or DoS. Let's
> replace it with a Caffeine cache as we have done previously for the OGNL
> expression cache.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Created] (WW-5411) Delete deprecated classes and members
Kusal Kithul-Godage created WW-5411: --- Summary: Delete deprecated classes and members Key: WW-5411 URL: https://issues.apache.org/jira/browse/WW-5411 Project: Struts 2 Issue Type: Task Components: Core Reporter: Kusal Kithul-Godage Fix For: 7.0.0 -- This message was sent by Atlassian Jira (v8.20.10#820010)
