Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NullPointerException on DefaultCallbackLogic #22

Closed
halflite opened this Issue Jun 15, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@halflite
Copy link

halflite commented Jun 15, 2017

jersey-mvc-freemarker 2.25.1
jersey-container-grizzly2-servlet 2.25.1
grizzly-http-servlet 2.3.28
jax-rs-pac4j 2.0.0
pac4j-oauth 2.0.0

https://github.com/pac4j/pac4j/blob/master/pac4j-core/src/main/java/org/pac4j/core/engine/DefaultCallbackLogic.java

Method redirectToOriginallyRequestedUrl

context.setSessionAttribute(Pac4jConstants.REQUESTED_URL, null);

https://github.com/javaee/grizzly/blob/master/modules/http-server/src/main/java/org/glassfish/grizzly/http/server/Session.java

Grizzly SessionStore cache attribute uses ConcurrentHashMap. Null value is denied.

@victornoel

This comment has been minimized.

Copy link
Member

victornoel commented Jun 17, 2017

Good find, so I will commit a simple fix soon, but until it is released, you can workaround this problem by doing the following:

Override GrizzlySessionStore to fix the problem:

    public class FixGrizzlySessionStore extends GrizzlySessionStore {
        @Override
        public void set(JaxRsContext context, String key, Object value) {
            if (value == null) {
                getSession(context).removeAttribute(key);
            } else {
                super.set(context, key, value);
            }
        }
    }

And use it instead of the original one by setting it on the pac4j Config:

        config.setSessionStore(new FixGrizzlySessionStore());

And it should be good :)

@victornoel victornoel added the bug label Jun 17, 2017

@victornoel victornoel added this to the 2.0.1 milestone Jun 17, 2017

@victornoel victornoel self-assigned this Jun 17, 2017

@leleuj

This comment has been minimized.

Copy link
Member

leleuj commented Jun 19, 2017

I remember some discussion on this kind of issue for the ProfileManager used in Play (as the profiles are saved in the Play Cache which does not support null in some implementations).
We ended up putting empty values: https://github.com/pac4j/pac4j/blob/master/pac4j-core/src/main/java/org/pac4j/core/client/IndirectClient.java#L102

@victornoel

This comment has been minimized.

Copy link
Member

victornoel commented Jun 19, 2017

@leleuj in this case it is clearly not needed as there is a removeAttribute method exactly for this purpose…

@victornoel

This comment has been minimized.

Copy link
Member

victornoel commented Jun 19, 2017

@leleuj also I think it is the context or the session store that should bother with that, not their clients… it's not their problem how things are stored.

@leleuj

This comment has been minimized.

Copy link
Member

leleuj commented Jun 19, 2017

Yes, it was a specific store and thus you need to handle that by yourself...

@victornoel

This comment has been minimized.

Copy link
Member

victornoel commented Jun 19, 2017

ah ok, but it's ok right now, it's fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.