If we're confident that OGNL 3.5 is highly compatible then yeah I think it should be fine. And it will need a stringent code review - the draft PR already has a few snippets that make me wary such as allowing the construction of OGNL contexts without injecting the SecurityMemberAccess policy.
There's also still the issue of breaking extension point API by migrating from the Map parameter type to another. I know we've had SemVer complaints in the past - https://issues.apache.org/jira/browse/WW-5251?focusedCommentId=17830550 . But perhaps with clear communication of these breaking changes, that can also be overcome. On Thu, Apr 3, 2025 at 11:38 PM Lukasz Lenart <lukaszlen...@apache.org> wrote: > > śr., 2 kwi 2025 o 02:12 Kusal Kithul-Godage > <kusal.kithulgod...@gmail.com> napisał(a): > > It seems the surface area of changes is large and has an API breaking > > impact on multiple extension points. I expect the OGNL upgrade itself > > could also cause breaking behavioural changes. If we're to abide by > > SemVer here, we should probably release this change as Struts 8.0 > > rather than 7.1 as many existing applications may not be able to > > upgrade without making code changes of their own. > > That's true, the impact of this change is wide, yet there is no change > in functionality. It even brings more safety as you cannot use an > arbitrary Map<String, Object> as a context for OGNL expression > evaluation - it must be StrutsContext, which basically is a Map after > all, see the structure of inheritance: > StrutsContext ->OgnlContext -> Map<String, Object> > > My first thought was the same, this should happen in Struts 8.x. Yet > we are falling behind in upgrading OGNL and it will be only harder to > upgrade in the future. Especially if a new security vulnerability > appears. > > In my opinion there is no big risk in introducing this change in 7.1 > while Struts 7 adoption is still low. > > > Compatibility aside, making major changes to OGNL expression handling > > has security implications and I feel it might be better to signify > > this with a major release. It'd be unfortunate to introduce a new > > security regression in 7.1 after all the work we've done ensuring 7.0 > > is the most secure Struts release to date. > > There are no changes in this area, as I already mentioned, this only > narrows the used type of context, which makes the code even safer. I'm > going to update tests now and if there be any regression in > expressions handling I will stop to see what we can do about it. > > > Cheers > Łukasz > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org > For additional commands, e-mail: dev-h...@struts.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org
