You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by yasserzamani <gi...@git.apache.org> on 2017/06/13 14:28:04 UTC
[GitHub] struts pull request #142: Blocks ognl access to class members of Spring prox...
GitHub user yasserzamani opened a pull request:
https://github.com/apache/struts/pull/142
Blocks ognl access to class members of Spring proxy
Fixes what I sent to security@struts.apache.org (3/15/2017)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yasserzamani/struts S2-047
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/struts/pull/142.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #142
----
commit 2a8a6869d0d8ed5c9a4a49d036a5e3c6d837fada
Author: Yasser Zamani <ya...@live.com>
Date: 2017-06-13T14:18:23Z
Blocks ognl access to class members of Spring proxy
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the issue:
https://github.com/apache/struts/pull/142
@yasserzamani do you want to port some of those changes to 2.3.33? Or at least implement what @aleksandr-m mentioned in a comment?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by yasserzamani <gi...@git.apache.org>.
Github user yasserzamani commented on the issue:
https://github.com/apache/struts/pull/142
@lukaszlenart , Yes with pleasure. I should come with a new PR but on branch support-2-3, right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the issue:
https://github.com/apache/struts/pull/142
The case with those changes is that they will affect everyone even if you don't use Spring so its scope should be narrowed just to the Spring Plugin. The simplest solution is to add a flag, a constant that by default should turn off this check, but the Spring Plugin should have this flag set on to enabled additional scanning.
The ultimate solution would be a voter mechanism injectable by the internal DI mechanism but this requires a bit more work.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: Blocks ognl access to class members of Spring proxy
Posted by yasserzamani <gi...@git.apache.org>.
Github user yasserzamani commented on the issue:
https://github.com/apache/struts/pull/142
*I forgot to emphasis that after my third commit, `isProxyMember` just have to search two Spring interfaces which includes a few members.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: Blocks ognl access to class members of Spring proxy
Posted by yasserzamani <gi...@git.apache.org>.
Github user yasserzamani commented on the issue:
https://github.com/apache/struts/pull/142
However, in my next commit, I try to check if `member` is belong to Spring, which decreases search space and improves performance. Currently it checks if `member` is not belong to user's config time action class.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: Blocks ognl access to class members of Spring proxy
Posted by yasserzamani <gi...@git.apache.org>.
Github user yasserzamani commented on the issue:
https://github.com/apache/struts/pull/142
> But isAccessible is called quite often even for rather simple actions.
So what do you recommend? Adding a cache? or something else you think about.
> Also it would be nice to have some property which allows to access proxy members.
It's not hard to have but I could not think about any use case of modifying proxy members by USER. Have you any specific use case in your mind? If no, I think we can postpone this until users demand.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: Blocks ognl access to class members of Spring proxy
Posted by aleksandr-m <gi...@git.apache.org>.
Github user aleksandr-m commented on the issue:
https://github.com/apache/struts/pull/142
What about performance?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: Blocks ognl access to class members of Spring proxy
Posted by aleksandr-m <gi...@git.apache.org>.
Github user aleksandr-m commented on the issue:
https://github.com/apache/struts/pull/142
But `isAccessible` is called quite often even for rather simple actions.
Also it would be nice to have some property which allows to access proxy members.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by aleksandr-m <gi...@git.apache.org>.
Github user aleksandr-m commented on the issue:
https://github.com/apache/struts/pull/142
@lukaszlenart Sounds good. Still, it would be nice to allow to turn this checking completely off even when spring plugin is presented. The issue then can be avoided with addition of a simple pattern which should be faster.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by aleksandr-m <gi...@git.apache.org>.
Github user aleksandr-m commented on the issue:
https://github.com/apache/struts/pull/142
@lukaszlenart
> The simplest solution is to add a flag, a constant that by default should turn off this check, but the Spring Plugin should have this flag set on to enable additional scanning.
I'll create a PR implementing this in a few days.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by yasserzamani <gi...@git.apache.org>.
Github user yasserzamani commented on the issue:
https://github.com/apache/struts/pull/142
Thank you @lukaszlenart , I got your point but what about when user uses Spring but not S2's Spring Plugin? i.e. when user does not want to define his/her actions as Spring beans but wants to use AOP on them.
However, now, after my forth commit, I don't think we should be worry. I tested WW-4805's scenario heavily with hundreds concurrent users via JMeter while profiling via YourKit. All of `ProxyUtil` methods just consume 186ms of the whole execution time, 137000ms, i.e. 0.001% ~= 0%. Before caching it was 7%.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the issue:
https://github.com/apache/struts/pull/142
With the flag in place you can always disable it in your {{struts.xml}} event it the Spring Plugin is present.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the issue:
https://github.com/apache/struts/pull/142
Yeah I understand but still this affects non-Spring users. And I think this can go in as is and we can improve and think about the Voters mechanism in 2.6.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by cnenning <gi...@git.apache.org>.
Github user cnenning commented on the issue:
https://github.com/apache/struts/pull/142
IMO this can be merged
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: Blocks ognl access to class members of Spring proxy
Posted by yasserzamani <gi...@git.apache.org>.
Github user yasserzamani commented on the issue:
https://github.com/apache/struts/pull/142
I think it's not a problem. It's complexity is O(n) where n is MAX(number of inheritances, interfaces or inner proxies) which are usually small.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request #142: WW-4805 Blocks ognl access to class members of Spr...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/struts/pull/142
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the issue:
https://github.com/apache/struts/pull/142
Yes, you must branch off from the `support-2-3` branch and open a PR against that branch after all
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts issue #142: WW-4805 Blocks ognl access to class members of Spring pro...
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the issue:
https://github.com/apache/struts/pull/142
If no objections I am going to merge this PR, btw. I have created a task to implement Voters
https://issues.apache.org/jira/browse/WW-4807
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org