[jira] [Comment Edited] (WW-5070) JSONResult default root object should be set explicitly, rather than from result of ValueStack.peek()

["Michael Hum (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/WW-5070?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17090729#comment-17090729 ] 

Michael Hum edited comment on WW-5070 at 4/23/20, 4:13 PM:
-----------------------------------------------------------

Ah, fair enough. I admit I assumed model here meant the action and kind of didn't give it a second thought as I was going off the setter documentation. I guess here then it's important to identify what the default should be? As in both cases currently it's not strictly true that the top of the stack be the model (the inline comment) or action (the setter javadoc). Even documenting that the default be the top of the stack would be clearer at that point, in case others ever run into this situation :)




was (Author: mikehum1):
Ah, fair enough. I admit I assumed model here meant the action and kind of didn't give it a second thought as I was going off the setter documentation. I guess here then it's important to identify what the default should be? As in both cases currently it's not strictly true that the top of the stack be the model or action. Even documenting that the default be the top of the stack would be clearer at that point, in case others ever run into this situation :)



> JSONResult default root object should be set explicitly, rather than from result of ValueStack.peek()
> -----------------------------------------------------------------------------------------------------
>
>                 Key: WW-5070
>                 URL: https://issues.apache.org/jira/browse/WW-5070
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Results
>    Affects Versions: 2.5.17
>         Environment: Struts 2.5.17
> Tomcat 8.0.24
>            Reporter: Michael Hum
>            Priority: Minor
>             Fix For: 2.6
>
>
> JSONResult#setRoot() is documented as follows:
> {quote}
> Sets the root object to be serialized, defaults to the Action
> {quote}
> This is implemented in {{org.apache.struts2.json.JSONResult#findRootObject}}:
> {code:java}
>         if (this.root != null) {
>             ValueStack stack = invocation.getStack();
>             rootObject = stack.findValue(root);
>         } else {
>             rootObject = invocation.getStack().peek(); // model overrides action
>         }
> {code}
> We have just run into an issue with our application where this expectation did not turn out to be true, due to a race condition in some of our custom results/interceptors (triggered by multiple requests) that results in the top of the stack not being the action. We've mitigated the issue by explicitly setting the root for serialization as the action:
> {code:java}
> @Result(name = FooAction.JSON, type = "json", params = {"root", "#action", "noCache", "true", "ignoreHierarchy", "false"}),
> {code}
> While not a bug triggered by any struts code, it would make more sense in this case to explicitly find the action instead of assuming that the top of the stack is the action. The ValueStack is able to be manipulated freely by developers, so this would guarantee that the default be correct regardless of external manipulation.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)