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

Joseph Wolschon commented on WW-5194:
-------------------------------------

Hi [~yasserzamani] :)

It's not that it returns a non-null object when it doesn't contain the key, 
it's that it tries to grab the nonce out of an invalidated session which causes 
the IllegalArgumentException. If the session wasn't 
invalidated,`session.get("nonce")` would return the value (or null if it didn't 
contain the key).

See docs:

{noformat}
/**
 * Returns the object bound with the specified name in this session, or
 * <code>null</code> if no object is bound under the name.
 *
 * @param name    a string specifying the name of the object
 *
 * @return       the object with the specified name
 *
 * @exception IllegalStateException    if this method is called on an
 *             invalidated session
 */
public Object getAttribute(String name); 

{noformat}
And package org.eclipse.jetty.server.session.Session:
{noformat}
 /**
     * @see javax.servlet.http.HttpSession#getAttribute(java.lang.String)
     */
    @Override
    public Object getAttribute(String name)
    {
        try (Lock lock = _lock.lock())
        {
            checkValidForRead();
            return _sessionData.getAttribute(name);
        }
    }

/**
 * Chech that the session data can be read.
 *
 * @throws IllegalStateException if the session is invalid
 */
protected void checkValidForRead() throws IllegalStateException
{
    checkLocked();

    if (_state == State.INVALID)
        throw new IllegalStateException("Invalid for read: id=" + 
_sessionData.getId() +
            " created=" + _sessionData.getCreated() +
            " accessed=" + _sessionData.getAccessed() +
            " lastaccessed=" + _sessionData.getLastAccessed() +
            " maxInactiveMs=" + _sessionData.getMaxInactiveMs() +
            " expiry=" + _sessionData.getExpiry());

    if (_state == State.INVALIDATING)
        return;

    if (!isResident())
        throw new IllegalStateException("Invalid for read: id=" + 
_sessionData.getId() + " not resident");
} 
{noformat}

I'm not sure what the expected behavior should be when trying to grab a nonce 
out of an invalidated session. The previous revision worked under our use case 
because the session doesn't even have a nonce to begin with because we're not 
using content security policy (CSP) feature. Perhaps it _should_ throw an 
exception if the session has been invalidated? If that is the case, the code 
block that gets the nonce should not execute if someone is not using the CSP 
feature, but maybe that was the intent of the `if 
(session.containsKey("nonce"))` block that was there before?

> UIBean.evaluateParams() throws an IllegalStateException when getting the 
> nonce out of a session that has been invalidated.
> --------------------------------------------------------------------------------------------------------------------------
>
>                 Key: WW-5194
>                 URL: https://issues.apache.org/jira/browse/WW-5194
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 6.0.0
>            Reporter: Joseph Wolschon
>            Assignee: Yasser Zamani
>            Priority: Minor
>              Labels: UIBean
>             Fix For: 6.0.1
>
>
> h2. Summary
> UIBean.evaluateParams() grabs the nonce out of the session without first 
> checking that it exists, causing an IllegalStateException to be thrown if the 
> session has been invalidated. This breaks our use case where we invalidate a 
> session, but still want to use ActionError to convey information to the user. 
> It doesn't appear that this change relates to removing double evaluations, so 
> I would consider this a regression.
> h2. Triage
> This was introduced when [refactoring to fix double 
> evaluations|https://github.com/apache/struts/commit/b2bfdc5c88a13e82d647e7ae836089a12ce001fe#diff-cfe644a2b24b492d6835fa1f38e7a770dad354b286cbe6b056a5fe7e80e669caL900]:
> {noformat}
> Object nonceValue = session != null ? session.get("nonce") : null;
> if (nonceValue != null){ 
>     addParameter("nonce", nonceValue.toString()); 
> }{noformat}
> The previous previous revision first checks that the key exists before 
> attempting to pull it out:
> {noformat}
> if (session.containsKey("nonce")) {               
>    String nonceValue = session.get("nonce").toString();
>    addParameter("nonce", nonceValue);           
> }
> {noformat}
> h2. Proposed Fix
> Revert to the previous revision and first check that the session contains the 
> nonce before getting it from the session.



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

Reply via email to