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

ASF GitHub Bot logged work on WW-5070:
--------------------------------------

                Author: ASF GitHub Bot
            Created on: 27/Apr/20 06:14
            Start Date: 27/Apr/20 06:14
    Worklog Time Spent: 10m 
      Work Description: lukaszlenart commented on a change in pull request #406:
URL: https://github.com/apache/struts/pull/406#discussion_r415537558



##########
File path: plugins/json/src/main/java/org/apache/struts2/json/JSONResult.java
##########
@@ -68,8 +69,6 @@
  */
 public class JSONResult implements Result {
 
-    private static final long serialVersionUID = 8624350183189931165L;

Review comment:
       Yeah, the idea was as this is a larger change to the class the ID should 
be changed, then I thought _why do we need the ID at all here, nobody should 
ever serialize a instance of `Result`_. So probably the main case is to stop 
extending `Serializable` by `Result`. I will put back a new ID.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 427474)
    Time Spent: 40m  (was: 0.5h)

> 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
>
>          Time Spent: 40m
>  Remaining Estimate: 0h
>
> 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)

Reply via email to