[jira] [Work logged] (WW-5406) Action excluded patterns are not updated following a configuration reload
[ https://issues.apache.org/jira/browse/WW-5406?focusedWorklogId=913825&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913825 ] ASF GitHub Bot logged work on WW-5406: -- Author: ASF GitHub Bot Created on: 10/Apr/24 06:08 Start Date: 10/Apr/24 06:08 Worklog Time Spent: 10m Work Description: sonarcloud[bot] commented on PR #910: URL: https://github.com/apache/struts/pull/910#issuecomment-2046603099 ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=910) **Quality Gate failed** Failed conditions ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [B Maintainability Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=910) (required ≥ A) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=910) ## ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/light_bulb-16px.png '') Catch issues before they fail your Quality Gate with our IDE extension ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/sonarlint-16px.png '') [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request) Issue Time Tracking --- Worklog Id: (was: 913825) Time Spent: 2h 10m (was: 2h) > 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: 2h 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-5406) Action excluded patterns are not updated following a configuration reload
[ https://issues.apache.org/jira/browse/WW-5406?focusedWorklogId=913824&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913824 ] ASF GitHub Bot logged work on WW-5406: -- Author: ASF GitHub Bot Created on: 10/Apr/24 06:04 Start Date: 10/Apr/24 06:04 Worklog Time Spent: 10m Work Description: kusalk commented on PR #910: URL: https://github.com/apache/struts/pull/910#issuecomment-2046598907 Done Issue Time Tracking --- Worklog Id: (was: 913824) Time Spent: 2h (was: 1h 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: 2h > 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-5406) Action excluded patterns are not updated following a configuration reload
[ https://issues.apache.org/jira/browse/WW-5406?focusedWorklogId=913823&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913823 ] ASF GitHub Bot logged work on WW-5406: -- Author: ASF GitHub Bot Created on: 10/Apr/24 06:02 Start Date: 10/Apr/24 06:02 Worklog Time Spent: 10m Work Description: sonarcloud[bot] commented on PR #910: URL: https://github.com/apache/struts/pull/910#issuecomment-2046596843 ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=910) **Quality Gate failed** Failed conditions ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [B Maintainability Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=910) (required ≥ A) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=910) ## ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/light_bulb-16px.png '') Catch issues before they fail your Quality Gate with our IDE extension ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/sonarlint-16px.png '') [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request) Issue Time Tracking --- Worklog Id: (was: 913823) Time Spent: 1h 50m (was: 1h 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: 1h 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] [Work logged] (WW-5406) Action excluded patterns are not updated following a configuration reload
[ https://issues.apache.org/jira/browse/WW-5406?focusedWorklogId=913822&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913822 ] ASF GitHub Bot logged work on WW-5406: -- Author: ASF GitHub Bot Created on: 10/Apr/24 05:59 Start Date: 10/Apr/24 05:59 Worklog Time Spent: 10m Work Description: kusalk commented on PR #910: URL: https://github.com/apache/struts/pull/910#issuecomment-2046593552 Ah wait, just realised I made a breaking change in PrepareOperations.java, let me fix that Issue Time Tracking --- Worklog Id: (was: 913822) Time Spent: 1h 40m (was: 1.5h) > 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 40m > 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-5406) Action excluded patterns are not updated following a configuration reload
[ https://issues.apache.org/jira/browse/WW-5406?focusedWorklogId=913821&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913821 ] ASF GitHub Bot logged work on WW-5406: -- Author: ASF GitHub Bot Created on: 10/Apr/24 05:59 Start Date: 10/Apr/24 05:59 Worklog Time Spent: 10m Work Description: kusalk commented on code in PR #910: URL: https://github.com/apache/struts/pull/910#discussion_r155068 ## 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: Fixed and created this general issue - WW-5411 Issue Time Tracking --- Worklog Id: (was: 913821) Time Spent: 1.5h (was: 1h 20m) > 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: 1.5h > 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-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)
[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&page=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-5406) Action excluded patterns are not updated following a configuration reload
[ https://issues.apache.org/jira/browse/WW-5406?focusedWorklogId=913819&page=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] [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=913817&page=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-5408) Add option to NOT fallback to empty namespace when unresolved
[ https://issues.apache.org/jira/browse/WW-5408?focusedWorklogId=913818&page=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-5408) Add option to NOT fallback to empty namespace when unresolved
[ https://issues.apache.org/jira/browse/WW-5408?focusedWorklogId=913816&page=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-5407) Extend SecurityMemberAccess proxy detection to Hibernate proxies
[ https://issues.apache.org/jira/browse/WW-5407?focusedWorklogId=913815&page=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] [Work logged] (WW-5407) Extend SecurityMemberAccess proxy detection to Hibernate proxies
[ https://issues.apache.org/jira/browse/WW-5407?focusedWorklogId=913813&page=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&page=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)