[WW-5190] Fixes StackOverflowException when dispatching request #571

lukaszlenart · 2022-06-15T09:08:55Z

This PR addresses a problem with forwarding to another JSP or action, but only in case if StrutsExecuteAndPrepareFilter is used - when combination of StrutsPrepareFilter & StrutsExecuteFilter is used; forwarding to actions won't work - it's a large problem and will be addressed separately, this never worked tbh.

Fixes WW-5190

coveralls · 2022-06-15T11:43:35Z

Coverage increased (+0.03%) to 50.658% when pulling cb59974 on WW-5190-match-action-proxy into 64d0542 on master.

core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java

yasserzamani · 2022-06-17T06:39:50Z

Is it simple to have a testWW5190 test method which tests that this issue is fixed? Sorry I couldn't understand the root cause to write a test. My best guess is that it's because of Struts FORWARD happens in same thread, is it?

lukaszlenart · 2022-06-17T07:10:39Z

Is it simple to have a testWW5190 test method which tests that this issue is fixed? Sorry I couldn't understand the root cause to write a test. My best guess is that it's because of Struts FORWARD happens in same thread, is it?

The simplest way is to define a dispatcher example in Showcase app and define an integration test for that.

…ests

lukaszlenart · 2022-06-21T08:44:23Z

There is some discrepancy between StrutsPrepareAndExcuteFilter and StrutsPrepareFilter & StrutsExcuteFilter which I'm trying to figure out

yasserzamani · 2022-06-21T10:49:32Z

There is some discrepancy between StrutsPrepareAndExcuteFilter and StrutsPrepareFilter & StrutsExcuteFilter which I'm trying to figure out

Maybe we can merge and postpone this to another release? just to make sure that the release has minimal required changes to fix regressions.

BTW how much time we do have do you know please? I like to find some free cycles and add an integration test to showcase app as you suggested , thanks!

lukaszlenart · 2022-06-21T10:50:40Z

I have integration test but it won't work with the Showcase App because of those problems.

yasserzamani · 2022-06-21T11:03:57Z

So it makes sense to have those in this PR please. awesome!! thank you very much!

…e and execute filters

lukaszlenart · 2022-06-27T07:54:06Z

@yasserzamani I extend description of the PR as the problem is a bit more complicated. My fix addresses only the broken support for forwarding to an another action when using StrutsPrepareAndExecuteFilter, yet the Showcase App is using combination of StrutsPrepareFilter/StrutsExecuteFilter and this never worked properly - I created a dedicated ticket to fix this with the next Minor release.

burtonrhodes · 2022-10-10T15:52:01Z

This change is breaking my ExecuteAndWait implementations using the recommended refresh implementation. The resulting url appends the file with "!methodName" making the url invalid.

A real world example is below:

<action name="LetterMergeBulk_contactList" class="com.afs.web.struts.action.letters.LetterMergeBulkAction" method="contactList">
    <interceptor-ref name="executeAndWaitInterceptor" />
    <result name="wait">/struts/letters/letterMergeBulkWait_popup.jsp</result>
    <result>/struts/letters/letterMerge_popup.jsp</result>
</action>

Results in (v6.0.3)

/afs/app/LetterMergeBulk_contactList!contactList.action?struts.token.name=token&token=A9SIKBABK17IZFM6AHSP0QSFIJHLZKJB&letterTemplateId=14773

Instead of (v6.0.0)

/afs/app/LetterMergeBulk_contactList.action?struts.token.name=token&token=A9SIKBABK17IZFM6AHSP0QSFIJHLZKJB&letterTemplateId=14773

Using HTML Header

<head>
   <title>Please wait</title>
   <meta http-equiv="refresh" content="5;url=<s:url includeParams="all" />"/>
</head>

lukaszlenart · 2022-10-13T06:27:30Z

@burtonrhodes do you use DMI?

burtonrhodes · 2022-10-13T11:21:34Z

I don't use DMI, but have no specific setting in my struts.xml file regarding it. However, given your suggestion, I specifically disabled DMI to see if that would help - but the result is still the same.

<constant name="struts.enable.DynamicMethodInvocation" value="false"/>

lukaszlenart · 2022-10-13T11:24:48Z

Ok, so please register a new JIRA ticket and we can start working on solution after figuring out what's wrong.

burtonrhodes · 2022-10-13T12:07:58Z

Done! https://issues.apache.org/jira/browse/WW-5241

lukaszlenart added 3 commits June 15, 2022 11:06

WW-5190 Extracts method name from config for known action name

5af840b

WW-5190 Reuses action proxy if namespace, name and method are match

1c2b491

WW-5190 Uses wrapped request only when processing request by Struts

69102e9

lukaszlenart added the Lazy Consensus label Jun 15, 2022

yasserzamani reviewed Jun 17, 2022

View reviewed changes

core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java Show resolved Hide resolved

lukaszlenart added 6 commits June 18, 2022 07:25

Fixes logging in Showcase app

a6a4529

Fixes showing configuration of an action in Showcase app

06290ce

Simplifies logging pattern in Showcase app

957292e

WW-5190 Adds a new test to cover new logic of comparing ActionProxies

ebe1b80

WW-5190 Defines two examples with Dispatcher result and integration t…

8e468a3

…ests

Merge branch 'master' into WW-5190-match-action-proxy

d639b59

lukaszlenart added 3 commits June 22, 2022 07:58

WW-5190 Uses proper order of servlets and filters

8c37b15

WW-5190 Disables test which won't pass without refactoring the prepar…

ecfdee1

…e and execute filters

WW-5190 Marks all Struts related filters with <async-supported/>

cb59974

lukaszlenart requested a review from yasserzamani July 8, 2022 10:54

lukaszlenart merged commit f3cb892 into master Jul 11, 2022

lukaszlenart deleted the WW-5190-match-action-proxy branch July 11, 2022 08:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WW-5190] Fixes StackOverflowException when dispatching request #571

[WW-5190] Fixes StackOverflowException when dispatching request #571

lukaszlenart commented Jun 15, 2022 •

edited

Loading

coveralls commented Jun 15, 2022 •

edited

Loading

yasserzamani commented Jun 17, 2022

lukaszlenart commented Jun 17, 2022 •

edited

Loading

lukaszlenart commented Jun 21, 2022

yasserzamani commented Jun 21, 2022

lukaszlenart commented Jun 21, 2022

yasserzamani commented Jun 21, 2022

lukaszlenart commented Jun 27, 2022

burtonrhodes commented Oct 10, 2022 •

edited

Loading

lukaszlenart commented Oct 13, 2022

burtonrhodes commented Oct 13, 2022

lukaszlenart commented Oct 13, 2022

burtonrhodes commented Oct 13, 2022

[WW-5190] Fixes StackOverflowException when dispatching request #571

[WW-5190] Fixes StackOverflowException when dispatching request #571

Conversation

lukaszlenart commented Jun 15, 2022 • edited Loading

coveralls commented Jun 15, 2022 • edited Loading

yasserzamani commented Jun 17, 2022

lukaszlenart commented Jun 17, 2022 • edited Loading

lukaszlenart commented Jun 21, 2022

yasserzamani commented Jun 21, 2022

lukaszlenart commented Jun 21, 2022

yasserzamani commented Jun 21, 2022

lukaszlenart commented Jun 27, 2022

burtonrhodes commented Oct 10, 2022 • edited Loading

lukaszlenart commented Oct 13, 2022

burtonrhodes commented Oct 13, 2022

lukaszlenart commented Oct 13, 2022

burtonrhodes commented Oct 13, 2022

lukaszlenart commented Jun 15, 2022 •

edited

Loading

coveralls commented Jun 15, 2022 •

edited

Loading

lukaszlenart commented Jun 17, 2022 •

edited

Loading

burtonrhodes commented Oct 10, 2022 •

edited

Loading