Right now I stuck with migration as I'm getting class cast exceptions when creating contexts using Ognl.createDefaultContext() - I assume it can be related to "static" nature of the whole OGNL and maybe it won't be possible to migrate the way I thought :\
I will try to dig a bit more, yet I'm considering reverting the idea and to use some kind of "bridge" between pure Map<> in Struts and OgnlContext in OGNL to keep backward compatibility. Yet this means I would have to convert OGNL from being a "static" driven tool to be an "instantiable" expression language. Cheers Lukasz niedz., 13 kwi 2025 o 03:46 Kusal Kithul-Godage <kusal.kithulgod...@gmail.com> napisał(a): > > 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org
