You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by MassimoH <Ma...@gmail.com> on 2007/10/02 23:52:50 UTC

struts/xwork patch submission. Please Review.

I've isolated a Struts 2 bug and I would like to suggest a code fix. The code
fix is in an xwork class. I'm don't fully understand the xwork/struts
distinction, but I assume I'm posting to the right list.

Here is the error scenario:

- DefaultActionInvocation calls all Interceptors which wind onto the Java
call stack. Then DefaultActionInvocation calls the actual action itself. The
action returns "success"
- DefaultActionInvocation creates result object from this result code and
executes the result
- There is a problem in my FTL template, so FreeMarker throws a
ParseException
- Call stack unwinds to ExceptionMappingInterceptor which catches the
exception, adds to OGNL value stack and returns "unhandledException"
- DefaultActionInvocation simply re-executes the already created result
object based on the original Action result.
- The caching mechanism within FreeMarker detects a current timestamp and
will either throw a FileNotFoundException (if the page isn't cached) or
returns cached and out-of-date results. In either case, the original
exception is lost and never logged. My exception action is also never
executed.

This seems to be...

A small FreeMarker bug: it should rethrow ParseException when called a
second time.
A larger Struts 2 (or xwork) bug: Struts shouldn't ignore the first
exception and simply rerun the result object. When Struts catches the
original exception from the result, it should call the defined exception
action.


I suggest that the following three lines be removed from
DefaultActionInvocation.java, line 174:

    	if (result != null) {
   		return result;
    	}

I've tested this and I get the desired behavior. My exception action is
called for the original exception. The xwork build reports no additional
automated test failures. I got four test failures trying to build the
standard xwork 2.0.4 distribution before making any changes, however my code
change didn't affect these results. 


Version Info
Java 1.6.0_02
Struts 2.0.9
xwork 2.0.4
-- 
View this message in context: http://www.nabble.com/struts-xwork-patch-submission.-Please-Review.-tf4558415.html#a13008842
Sent from the Struts - Dev mailing list archive at Nabble.com.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Re: struts/xwork patch submission. Please Review.

Posted by Don Brown <mr...@twdata.org>.
The ticket has been fixed.  Basically, the patch provided would have
broken another feature, so I fixed it in such a way that everyone
wins.  I didn't put it in 2.1 as I'd like to see it get more testing
before going in a stable branch, but if someone else reviews the
change and is ok with it, feel free to backport it.

Don

On 10/4/07, MassimoH <Ma...@gmail.com> wrote:
>
> Yes, that is the exact same issue and fix.
>
> I take it that this issue wasn't high enough priority to be included in
> previous xwork patch releases?
>
>
> Aymeric Levaux wrote:
> >
> > I reported a similar issue in Xwork JIRA months ago :
> > http://jira.opensymphony.com/browse/XW-521
> > The fix is identical.
> >
> > --
> > Aymeric
> >
> > PS: I'm not able to reply threads from my mail account, they're just
> > ignored  (no errors). Any suggestions?
> >
> >
> > MassimoH wrote:
> >>
> >> I've isolated a Struts 2 bug and I would like to suggest a code fix. The
> >> code fix is in an xwork class. I'm don't fully understand the
> >> xwork/struts distinction, but I assume I'm posting to the right list.
> >>
> >> Here is the error scenario:
> >>
> >> - DefaultActionInvocation calls all Interceptors which wind onto the Java
> >> call stack. Then DefaultActionInvocation calls the actual action itself.
> >> The action returns "success"
> >> - DefaultActionInvocation creates result object from this result code and
> >> executes the result
> >> - There is a problem in my FTL template, so FreeMarker throws a
> >> ParseException
> >> - Call stack unwinds to ExceptionMappingInterceptor which catches the
> >> exception, adds to OGNL value stack and returns "unhandledException"
> >> - DefaultActionInvocation simply re-executes the already created result
> >> object based on the original Action result.
> >> - The caching mechanism within FreeMarker detects a current timestamp and
> >> will either throw a FileNotFoundException (if the page isn't cached) or
> >> returns cached and out-of-date results. In either case, the original
> >> exception is lost and never logged. My exception action is also never
> >> executed.
> >>
> >> This seems to be...
> >>
> >> A small FreeMarker bug: it should rethrow ParseException when called a
> >> second time.
> >> A larger Struts 2 (or xwork) bug: Struts shouldn't ignore the first
> >> exception and simply rerun the result object. When Struts catches the
> >> original exception from the result, it should call the defined exception
> >> action.
> >>
> >>
> >> I suggest that the following three lines be removed from
> >> DefaultActionInvocation.java, line 174:
> >>
> >>      if (result != null) {
> >>              return result;
> >>      }
> >>
> >> I've tested this and I get the desired behavior. My exception action is
> >> called for the original exception. The xwork build reports no additional
> >> automated test failures. I got four test failures trying to build the
> >> standard xwork 2.0.4 distribution before making any changes, however my
> >> code change didn't affect these results.
> >>
> >>
> >> Version Info
> >> Java 1.6.0_02
> >> Struts 2.0.9
> >> xwork 2.0.4
> >>
> >
> >
>
> --
> View this message in context: http://www.nabble.com/struts-xwork-patch-submission.-Please-Review.-tf4558415.html#a13020278
> Sent from the Struts - Dev mailing list archive at Nabble.com.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Re: struts/xwork patch submission. Please Review.

Posted by MassimoH <Ma...@gmail.com>.
Yes, that is the exact same issue and fix.

I take it that this issue wasn't high enough priority to be included in
previous xwork patch releases?


Aymeric Levaux wrote:
> 
> I reported a similar issue in Xwork JIRA months ago :
> http://jira.opensymphony.com/browse/XW-521
> The fix is identical.
> 
> -- 
> Aymeric 
> 
> PS: I'm not able to reply threads from my mail account, they're just
> ignored  (no errors). Any suggestions?
> 
> 
> MassimoH wrote:
>> 
>> I've isolated a Struts 2 bug and I would like to suggest a code fix. The
>> code fix is in an xwork class. I'm don't fully understand the
>> xwork/struts distinction, but I assume I'm posting to the right list.
>> 
>> Here is the error scenario:
>> 
>> - DefaultActionInvocation calls all Interceptors which wind onto the Java
>> call stack. Then DefaultActionInvocation calls the actual action itself.
>> The action returns "success"
>> - DefaultActionInvocation creates result object from this result code and
>> executes the result
>> - There is a problem in my FTL template, so FreeMarker throws a
>> ParseException
>> - Call stack unwinds to ExceptionMappingInterceptor which catches the
>> exception, adds to OGNL value stack and returns "unhandledException"
>> - DefaultActionInvocation simply re-executes the already created result
>> object based on the original Action result.
>> - The caching mechanism within FreeMarker detects a current timestamp and
>> will either throw a FileNotFoundException (if the page isn't cached) or
>> returns cached and out-of-date results. In either case, the original
>> exception is lost and never logged. My exception action is also never
>> executed.
>> 
>> This seems to be...
>> 
>> A small FreeMarker bug: it should rethrow ParseException when called a
>> second time.
>> A larger Struts 2 (or xwork) bug: Struts shouldn't ignore the first
>> exception and simply rerun the result object. When Struts catches the
>> original exception from the result, it should call the defined exception
>> action.
>> 
>> 
>> I suggest that the following three lines be removed from
>> DefaultActionInvocation.java, line 174:
>> 
>>     	if (result != null) {
>>    		return result;
>>     	}
>> 
>> I've tested this and I get the desired behavior. My exception action is
>> called for the original exception. The xwork build reports no additional
>> automated test failures. I got four test failures trying to build the
>> standard xwork 2.0.4 distribution before making any changes, however my
>> code change didn't affect these results. 
>> 
>> 
>> Version Info
>> Java 1.6.0_02
>> Struts 2.0.9
>> xwork 2.0.4
>> 
> 
> 

-- 
View this message in context: http://www.nabble.com/struts-xwork-patch-submission.-Please-Review.-tf4558415.html#a13020278
Sent from the Struts - Dev mailing list archive at Nabble.com.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Re: struts/xwork patch submission. Please Review.

Posted by Aymeric Levaux <al...@javablackbelt.com>.
I reported a similar issue in Xwork JIRA months ago :
http://jira.opensymphony.com/browse/XW-521
The fix is identical.

-- 
Aymeric 

PS: I'm not able to reply threads from my mail account, they're just ignored 
(no errors). Any suggestions?


MassimoH wrote:
> 
> I've isolated a Struts 2 bug and I would like to suggest a code fix. The
> code fix is in an xwork class. I'm don't fully understand the xwork/struts
> distinction, but I assume I'm posting to the right list.
> 
> Here is the error scenario:
> 
> - DefaultActionInvocation calls all Interceptors which wind onto the Java
> call stack. Then DefaultActionInvocation calls the actual action itself.
> The action returns "success"
> - DefaultActionInvocation creates result object from this result code and
> executes the result
> - There is a problem in my FTL template, so FreeMarker throws a
> ParseException
> - Call stack unwinds to ExceptionMappingInterceptor which catches the
> exception, adds to OGNL value stack and returns "unhandledException"
> - DefaultActionInvocation simply re-executes the already created result
> object based on the original Action result.
> - The caching mechanism within FreeMarker detects a current timestamp and
> will either throw a FileNotFoundException (if the page isn't cached) or
> returns cached and out-of-date results. In either case, the original
> exception is lost and never logged. My exception action is also never
> executed.
> 
> This seems to be...
> 
> A small FreeMarker bug: it should rethrow ParseException when called a
> second time.
> A larger Struts 2 (or xwork) bug: Struts shouldn't ignore the first
> exception and simply rerun the result object. When Struts catches the
> original exception from the result, it should call the defined exception
> action.
> 
> 
> I suggest that the following three lines be removed from
> DefaultActionInvocation.java, line 174:
> 
>     	if (result != null) {
>    		return result;
>     	}
> 
> I've tested this and I get the desired behavior. My exception action is
> called for the original exception. The xwork build reports no additional
> automated test failures. I got four test failures trying to build the
> standard xwork 2.0.4 distribution before making any changes, however my
> code change didn't affect these results. 
> 
> 
> Version Info
> Java 1.6.0_02
> Struts 2.0.9
> xwork 2.0.4
> 

-- 
View this message in context: http://www.nabble.com/struts-xwork-patch-submission.-Please-Review.-tf4558415.html#a13014849
Sent from the Struts - Dev mailing list archive at Nabble.com.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Re: struts/xwork patch submission. Please Review.

Posted by Don Brown <mr...@twdata.org>.
Looks good, file a issue at http://issues.apache.org/struts

Don

On 10/3/07, MassimoH <Ma...@gmail.com> wrote:
>
> I've isolated a Struts 2 bug and I would like to suggest a code fix. The code
> fix is in an xwork class. I'm don't fully understand the xwork/struts
> distinction, but I assume I'm posting to the right list.
>
> Here is the error scenario:
>
> - DefaultActionInvocation calls all Interceptors which wind onto the Java
> call stack. Then DefaultActionInvocation calls the actual action itself. The
> action returns "success"
> - DefaultActionInvocation creates result object from this result code and
> executes the result
> - There is a problem in my FTL template, so FreeMarker throws a
> ParseException
> - Call stack unwinds to ExceptionMappingInterceptor which catches the
> exception, adds to OGNL value stack and returns "unhandledException"
> - DefaultActionInvocation simply re-executes the already created result
> object based on the original Action result.
> - The caching mechanism within FreeMarker detects a current timestamp and
> will either throw a FileNotFoundException (if the page isn't cached) or
> returns cached and out-of-date results. In either case, the original
> exception is lost and never logged. My exception action is also never
> executed.
>
> This seems to be...
>
> A small FreeMarker bug: it should rethrow ParseException when called a
> second time.
> A larger Struts 2 (or xwork) bug: Struts shouldn't ignore the first
> exception and simply rerun the result object. When Struts catches the
> original exception from the result, it should call the defined exception
> action.
>
>
> I suggest that the following three lines be removed from
> DefaultActionInvocation.java, line 174:
>
>         if (result != null) {
>                 return result;
>         }
>
> I've tested this and I get the desired behavior. My exception action is
> called for the original exception. The xwork build reports no additional
> automated test failures. I got four test failures trying to build the
> standard xwork 2.0.4 distribution before making any changes, however my code
> change didn't affect these results.
>
>
> Version Info
> Java 1.6.0_02
> Struts 2.0.9
> xwork 2.0.4
> --
> View this message in context: http://www.nabble.com/struts-xwork-patch-submission.-Please-Review.-tf4558415.html#a13008842
> Sent from the Struts - Dev mailing list archive at Nabble.com.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org