[jboss-jira] [JBoss JIRA] (WFLY-6808) DistributableSession validate method throw misleading exception message

[Mathieu Lachance (JIRA)]

     [ https://issues.jboss.org/browse/WFLY-6808?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mathieu Lachance updated WFLY-6808:
-----------------------------------
    Description: 
In DistributableSession the validate method is getting called for any underlying undertow session access to make sure we are not touching an already invalidated session (which totally make sense):
{code}
public class DistributableSession implements io.undertow.server.session.Session {

    private static void validate(Session<LocalSessionContext> session) {
        if (!session.isValid()) {
            throw UndertowMessages.MESSAGES.sessionNotFound(session.getId());
        }
    }
}
{code}

The problem though is the exception message that is thrown is really misleading because in reality the session actually exists but is currently invalid and/or getting invalidated. This can happen especially when running in optimistic mode where we can have many differents threads accessing the very same session.

I would recommend we do instead:
{code}
        if (!session.isValid()) {
            throw UndertowMessages.MESSAGES.sessionAlreadyInvalidated();
        }
{code}
or even better:
{code}
        if (!session.isValid()) {
            throw UndertowMessages.MESSAGES.sessionAlreadyInvalidated(session.getId());
        }
{code}
but it will require also a change in Undertow to actually template/parametize the sessionAlreadyInvalidated message.

Thanks,

  was:
In DistributableSession the validate method is getting called for any underlying undertow session access to make sure we are not touching an already invalidated session (which totally make sense):
{code}
public class DistributableSession implements io.undertow.server.session.Session {
    // These mechanisms can auto-reauthenticate and thus use local context (instead of replicating)
    private static final Set<String> AUTO_REAUTHENTICATING_MECHANISMS = new HashSet<>(Arrays.asList(HttpServletRequest.BASIC_AUTH, HttpServletRequest.DIGEST_AUTH, HttpServletRequest.CLIENT_CERT_AUTH));

    private static void validate(Session<LocalSessionContext> session) {
        if (!session.isValid()) {
            throw UndertowMessages.MESSAGES.sessionNotFound(session.getId());
        }
    }
}
{code}

The problem though is the exception message that is thrown is really misleading because in reality the session actually exists but is currently invalid and/or getting invalidated. This can happen especially when running in optimistic mode where we can have many differents threads accessing the very same session.

I would recommend we do instead:
{code}
        if (!session.isValid()) {
            throw UndertowMessages.MESSAGES.sessionAlreadyInvalidated();
        }
{code}
or even better:
{code}
        if (!session.isValid()) {
            throw UndertowMessages.MESSAGES.sessionAlreadyInvalidated(session.getId());
        }
{code}
but it will require also a change in Undertow to actually template/parametize the sessionAlreadyInvalidated message.

Thanks,



> DistributableSession validate method throw misleading exception message
> -----------------------------------------------------------------------
>
>                 Key: WFLY-6808
>                 URL: https://issues.jboss.org/browse/WFLY-6808
>             Project: WildFly
>          Issue Type: Enhancement
>          Components: Clustering
>    Affects Versions: 10.0.0.Final
>            Reporter: Mathieu Lachance
>            Assignee: Paul Ferraro
>
> In DistributableSession the validate method is getting called for any underlying undertow session access to make sure we are not touching an already invalidated session (which totally make sense):
> {code}
> public class DistributableSession implements io.undertow.server.session.Session {
>     private static void validate(Session<LocalSessionContext> session) {
>         if (!session.isValid()) {
>             throw UndertowMessages.MESSAGES.sessionNotFound(session.getId());
>         }
>     }
> }
> {code}
> The problem though is the exception message that is thrown is really misleading because in reality the session actually exists but is currently invalid and/or getting invalidated. This can happen especially when running in optimistic mode where we can have many differents threads accessing the very same session.
> I would recommend we do instead:
> {code}
>         if (!session.isValid()) {
>             throw UndertowMessages.MESSAGES.sessionAlreadyInvalidated();
>         }
> {code}
> or even better:
> {code}
>         if (!session.isValid()) {
>             throw UndertowMessages.MESSAGES.sessionAlreadyInvalidated(session.getId());
>         }
> {code}
> but it will require also a change in Undertow to actually template/parametize the sessionAlreadyInvalidated message.
> Thanks,



--
This message was sent by Atlassian JIRA
(v6.4.11#64026)