spring-projects / spring-session

Spring Session
https://spring.io/projects/spring-session
Apache License 2.0
1.86k stars 1.11k forks source link

Provide reliable serialization mechanism (not objectstream) #348

Open pcornelissen opened 8 years ago

pcornelissen commented 8 years ago

The current way to handle serialization via objectstream may be the solution that was easiest to implement (and fastest at runtime?), but it has some serious disadvantages. The negative effects are also visible in issues like #320 and #200.

I think this is rather unfortunate, because the current approach makes the whole microservice based application potentially a deployment monolith. When you change a class in one service then you also need to change all other services that fetch the underlying data. The current approach is very very fragile :-(

I was first made aware of this problem when I tried to upgrade one service from spring boot 1.2.x to 1.3.x. Due to the fact that spring security changed, the UserDetails implementation changed too and the whole app broke apart. This is really bad, because now I have to do a big bang release of all services that use the session at once.

What I would expect to be possible: Microservice 1 handles the user authentication, as it does more than just displaying it, it has it's own UserDetails Implementation. Microservice 2 just uses a simple POJO to read the UserDetails from the session to display it and make security related decisions

This doesn't work at all at the moment, as both services need to use exactly the same implementation.

Wouldn't it be much more robust to use something like json/jsonb or something like that? Then you even could use non java clients to read the same session info.

283 seems to allow a better serialization strategy, but every spring-session user has to come up with his own implementation and I think this is a very common use-case and it would be great it spring would ship with a good solution.

rwinch commented 8 years ago

@pcornelissen Thank you for your feedback!

JDK Serialization makes sense as a default for a few reasons.

Wouldn't it be much more robust to use something like json/jsonb or something like that? Then you even could use non java clients to read the same session info.

I'm not sure I understand this statement. You can expose a Jackson2JsonRedisSerializer which should allow writing your session attributes with JSON.

Issue #283 is the first step to providing users with alternatives, but there is certainly room for improvement. As I see it, we need to provide an easy way to serialize Spring related objects (i.e. Spring Security's SecurityContext) in alternative formats.

However, if you have any custom objects being written to session there is likely going to be work from your side to ensure they are mapped properly.

Of course you can further insulate yourself by exposing sessions via a service. Then user's would not be directly interacting with the data store. This adds additional overhead, but it isn't really the microservice way to share a datastore directly.

Does this help or do you have something else that you think needs to be in place?

pcornelissen commented 8 years ago

Thanks for your reply!

I understand the reason for jdk serialization as "works everywhere" solution. The problem that the objects in the session need to be serializable in the defined serialization mechanism is always present, just in another form.

During my initial investigation I also found the Jackson2JsonRedisSerializer, but it needs a class to serialize to and I didn't had the time to find out whether this might me the solution.

What do I have to do to configure this and get it picked up? So, the target class should be HttpSession and I just create a Bean in my context or is there something else to do?

rwinch commented 8 years ago

@pcornelissen There in lies the problem. You cannot have arbitrary deserialization without including the class names (which is Java specific) in the output of serializing the classes.

For what its worth this is certainly something that we need to experiment with to provide more prescriptive solution.

For now you should be able to customize serialization by exposing a RedisSerializer with the name defaultRedisSerializer in 1.1.0.M1. This name will change to springSessionDefaultRedisSerializer in #350

pcornelissen commented 8 years ago

Hmm, well regular json based communication doesn't include a classname either, but I see that this would be a problem as you can't provide the type when you try to fetch it from the session as the interface there is already established.

I think I'll try to switch to json based serialization, to at least get rid of the deployment monolith effect. So I just have to make sure that the bean itself doesn't change too much to be able to deserialize it, which is ok.

pcornelissen commented 8 years ago

I had tome to switch to the Jackson2JsonRedisSerializer and it looks good.

For documentation purposes, this is how I've enabled it after switching to springsession and -data-redis to the 1.1.0.M1:

   @Bean(name = {"defaultRedisSerializer","springSessionDefaultRedisSerializer"})
    RedisSerializer<Object> defaultRedisSerializer(){
        return new Jackson2JsonRedisSerializer(Object.class);
    }

It's important to use Object as generic parameter, otherwise it won't be picked up!

Maybe this should be stated more visible in the spring-session documentation? Because as I said, as son as you have different codebases that try to access the session you'll get in trouble.

rwinch commented 8 years ago

@pcornelissen Thanks for the feedback. You can see it is documented here http://docs.spring.io/spring-session/docs/current-SNAPSHOT/reference/html5/#custom-redisserializer Does that help?

pcornelissen commented 8 years ago

@rwinch That's a start :-)

It helps that this is mentioned, but it doesn't say why you might want to do that. If didn't knew about the implications of the default serialization scheme, I wouldn't have searched in the docs for it. I would suggest to make the documentation a little bit more verbose like:

Serialization

The standard serialization format is plain java serialization using ObjectStream. This implies that you need the exact same implementation of the saved objects on all services that access the session. If you need to be able to have different implementations or versions on your services, you need to use a different serializer. You can customize the serialization by creating a Bean named springSessionDefaultRedisSerializer that implements RedisSerializer. To use Json to serialize the objects in the session you can use for example the Jackson2JsonRedisSerializer.

chrylis commented 8 years ago

You don't necessarily need to have the exact same implementation, just compatible ones; the serialVersionUID is there particularly to handle that case. IMO, pointing out that Java serialization is used should be sufficient to alert Java programmers to format issues. That said, given the compelling use cases of Spring Session in microservices and continuous delivery, it would be nice to have some easier mechanism for forward compatibility, though JSON makes me a little edgy for performance reasons.

pcornelissen commented 8 years ago

I think redis stores binary data anyway, so Bson would be possible as well.

Regarding the serialVersionUUID, you are right you can influence this a bit if what you are saving is written by you, but in my case for example, the spring security version changed, so did the User class which lead to the problem that the serialization broke when I tried to use spring boot 1.2 and 1.3 services at once. And the larger your software ecosystem is, the larger is the possibility that this bites you in the "you know what".

pcornelissen commented 8 years ago

I am not sure if I am still doing something wrong...

All my Services have this Bean:

    @Bean(name = {"defaultRedisSerializer", "springSessionDefaultRedisSerializer"})
    RedisSerializer<Object> defaultRedisSerializer() {
        return new Jackson2JsonRedisSerializer(Object.class);
    }

And I have a UserDetailsService that returns an instance of org.springframework.security.core.userdetails.User

Each time I log in and to a redirect to the homepage where the principal is used to show the log in name I get this in the log and the user appears to be not logged in:

2016-02-08 04:21:32.839  WARN 28029 --- [nio-8086-exec-6] w.c.HttpSessionSecurityContextRepository : SPRING_SECURITY_CONTEXT did not contain a SecurityContext but contained: 
'{authentication={details={remoteAddress=0:0:0:0:0:0:0:1, sessionId=null}, authorities=[{authority=ROLE_USER}], authenticated=true, 
principal={password=null, username=testuser@orchit.de, authorities=[{authority=ROLE_USER}], accountNonExpired=true, accountNonLocked=true, credentialsNonExpired=true, enabled=true},
 credentials=null, name=testuser@orchit.de}}'; 
are you improperly modifying the HttpSession directly (you should always use SecurityContextHolder) or using the HttpSession attribute reserved for this class?

(I have added a few line breaks as this log line is rather long...) Needless to say, I don't fiddle with the session, I have no direct contact with the session anywhere besides trying to get the principal from the security context via thymeleaf-spring-security integration or controller methods like:

    @RequestMapping(value = "edit", method = RequestMethod.GET)
    public String edit(Model model, Principal principal) {
        ....
        return "user/edit";
    }

I assume that spring security is not (yet?) capable of working with the jackson serializer?

The session in Redis looks like this:

lastAccessedTime: 1454901692859
sessionAttr:SPRING_SECURITY_LAST_EXCEPTION: ""
maxInactiveInterval: 1800
sessionAttr:SPRING_SECURITY_CONTEXT: {
    "authentication": {
        "details": {
            "remoteAddress": "0:0:0:0:0:0:0:1",
            "sessionId": null
        },
        "authorities": [
            {
                "authority": "ROLE_USER"
            }
        ],
        "authenticated": true,
        "principal": {
            "password": null,
            "username": "testuser@orchit.de",
            "authorities": [
                {
                    "authority": "ROLE_USER"
                }
            ],
            "accountNonExpired": true,
            "accountNonLocked": true,
            "credentialsNonExpired": true,
            "enabled": true
        },
        "credentials": null,
        "name": "testuser@orchit.de"
    }
}
creationTime: 1454901692641

So basically this should work, because as far as I can see, all the info that spring security needs is there, but it fails to get the proper class from the serializer.

    // We now have the security context object from the session.
    if (!(contextFromSession instanceof SecurityContext)) {
             ...
    }

fails because the contextFromSession in the HttpSessionSecurityContextRepository when I am not in debug mode (wtf?).

pcornelissen commented 8 years ago

OK, doing some further debugging why there is a difference between running the code fro console or from inside intelliJ...

I just got this exception when running from console: (I used this: mvn clean spring-boot:run)

2016-02-08 06:32:32.285  WARN 2015 --- [nerContainer-21] o.s.d.r.l.RedisMessageListenerContainer  : Execution of JMS message listener failed, and no ErrorHandler has been set.

org.springframework.data.redis.serializer.SerializationException: Cannot deserialize; nested exception is org.springframework.core.serializer.support.SerializationFailedException: Failed to deserialize payload. Is the byte array a result of corresponding serialization for DefaultDeserializer?; nested exception is java.io.EOFException
    at org.springframework.data.redis.serializer.JdkSerializationRedisSerializer.deserialize(JdkSerializationRedisSerializer.java:41) ~[spring-data-redis-1.6.2.RELEASE.jar:1.6.2.RELEASE]
    at org.springframework.session.data.redis.RedisOperationsSessionRepository.onMessage(RedisOperationsSessionRepository.java:450) ~[spring-session-1.1.0.M1.jar:na]
    at org.springframework.data.redis.listener.RedisMessageListenerContainer.executeListener(RedisMessageListenerContainer.java:245) [spring-data-redis-1.6.2.RELEASE.jar:1.6.2.RELEASE]
    at org.springframework.data.redis.listener.RedisMessageListenerContainer.processMessage(RedisMessageListenerContainer.java:235) [spring-data-redis-1.6.2.RELEASE.jar:1.6.2.RELEASE]
    at org.springframework.data.redis.listener.RedisMessageListenerContainer$1.run(RedisMessageListenerContainer.java:960) [spring-data-redis-1.6.2.RELEASE.jar:1.6.2.RELEASE]
    at java.lang.Thread.run(Thread.java:745) [na:1.8.0_66]
Caused by: org.springframework.core.serializer.support.SerializationFailedException: Failed to deserialize payload. Is the byte array a result of corresponding serialization for DefaultDeserializer?; nested exception is java.io.EOFException
    at org.springframework.core.serializer.support.DeserializingConverter.convert(DeserializingConverter.java:78) ~[spring-core-4.2.4.RELEASE.jar:4.2.4.RELEASE]
    at org.springframework.core.serializer.support.DeserializingConverter.convert(DeserializingConverter.java:36) ~[spring-core-4.2.4.RELEASE.jar:4.2.4.RELEASE]
    at org.springframework.data.redis.serializer.JdkSerializationRedisSerializer.deserialize(JdkSerializationRedisSerializer.java:39) ~[spring-data-redis-1.6.2.RELEASE.jar:1.6.2.RELEASE]
    ... 5 common frames omitted
Caused by: java.io.EOFException: null
    at java.io.ObjectInputStream$PeekInputStream.readFully(ObjectInputStream.java:2335) ~[na:1.8.0_66]
    at java.io.ObjectInputStream$BlockDataInputStream.readShort(ObjectInputStream.java:2804) ~[na:1.8.0_66]
    at java.io.ObjectInputStream.readStreamHeader(ObjectInputStream.java:803) ~[na:1.8.0_66]
    at java.io.ObjectInputStream.<init>(ObjectInputStream.java:299) ~[na:1.8.0_66]
    at org.springframework.core.ConfigurableObjectInputStream.<init>(ConfigurableObjectInputStream.java:64) ~[spring-core-4.2.4.RELEASE.jar:4.2.4.RELEASE]
    at org.springframework.core.ConfigurableObjectInputStream.<init>(ConfigurableObjectInputStream.java:50) ~[spring-core-4.2.4.RELEASE.jar:4.2.4.RELEASE]
    at org.springframework.core.serializer.DefaultDeserializer.deserialize(DefaultDeserializer.java:68) ~[spring-core-4.2.4.RELEASE.jar:4.2.4.RELEASE]
    at org.springframework.core.serializer.support.DeserializingConverter.convert(DeserializingConverter.java:73) ~[spring-core-4.2.4.RELEASE.jar:4.2.4.RELEASE]
    ... 7 common frames omitted

(Although I have defined the json serializer as stated above)

According to the /internal/beans endpoint, the bean is present:

   {
     "bean": "defaultRedisSerializer",
    "scope": "singleton",
    "type": "org.springframework.data.redis.serializer.Jackson2JsonRedisSerializer",
    "resource": "class path resource [de/orchit/kulinariweb/frontend/config/SecurityConfig.class]",
    "dependencies": [

    ]
  },

But it didn't register with it's second name "springSessionDefaultRedisSerializer".

According to the beans endpoint there is no bean named springSessionDefaultRedisSerializer in debug mode either, where it works. I have removed the defaultRedisSerializer bean name from the bean definition leavong it with only one name. Nor it works also from console. I think there may be a problem with bean aliases.

Sorry for highjacking this issue for this. Did I something wrong or should I create an issue for this so someone can investigate this later?

rwinch commented 8 years ago

@pcornelissen Thanks for the feedback on the documentation! Would you mind sending a PR with the suggested changes?

Sorry for highjacking this issue for this. Did I something wrong or should I create an issue for this so someone can investigate this later?

I think the problem is that the value is not being deserliazed into a SecurityContext object. This is likely because Jackson2JsonRedisSerializer only works on a single type (i.e. you would need to use):

    @Bean(name = {"defaultRedisSerializer", "springSessionDefaultRedisSerializer"})
    RedisSerializer<Object> defaultRedisSerializer() {
        Jackson2JsonRedisSerializer result = new Jackson2JsonRedisSerializer(SecurityContextImpl.class);
        result.setObjectMapper(createObjectMapper());
        return result;
    }

You would also need to ensure that the ObjectMapper injected into Jackson2JsonRedisSerializer can handle serializing and deserializing the SecurityContextimpl. I think this leads me back to the statement of:

There in lies the problem. You cannot have arbitrary deserialization without including the class names (which is Java specific) in the output of serializing the classes.

There is the possibility we could add some sort of hook to provide serialization by the name of the session attribute.

rwinch commented 8 years ago

@pcornelissen Looking into this a little more. Perhaps what we need is the ability to convert the delta map (that contains rich objects) into an arbitrary map instance. I created #359 to explore this further. Feel free to comment/discuss there. I'd love to see a PR if you can find the time :)

rwinch commented 8 years ago

@pcornelissen PS: I think the problem you are encountering is a real issue many users are running into. So I think it is important we flush it out. Thanks for raising this issue.

laurikimmel commented 7 years ago

I just ran into it and I can confirm - it's a real issue. Any plans to resolve it in the foreseeable future?

rwinch commented 7 years ago

Yes. Improving serialization was a theme in 1.3, but this issue just got dropped due to lack of time. Right now you can use Jackson, but you must include the class name in the JSON.

Eventually (next release) we will allow transforming based on the entire Map which would allow you to base the class name based on the session attribute name.

Maskime commented 7 years ago

I just ran in the same issue when trying to use mongodb as the session backend. I guess that since mongo is also using a JSON like structure the source of the problem is the same. I changed it to HashMap backend and it works correctly now.

Bessonov commented 6 years ago

I really like spring session approach, but I have same issue with it like many others. I see some issues related to this topic: https://github.com/spring-projects/spring-session/issues/280, https://github.com/spring-projects/spring-session/issues/361 and https://github.com/spring-projects/spring-session/issues/529.

@rwinch is there any plan (or a way) to fix it instead of moving from milestone to milestone?

Which workarounds are safe? Is this Implementation safe for production use (except it should be a FindByIndexNameSessionRepository)?

vpavic commented 6 years ago

@Bessonov A while ago I've created a PR that was supposed to address #529 - see #646. However there were some shortcomings to that approach which I've documented in the comments on that PR so I'd encourage you to take a look and see if it fits you.

Bessonov commented 6 years ago

@vpavic yeah, I've seen it. The linked approach is similar to yours, but it don't just delegate to original delete function and uses redisTemplate.delete.

As I see from source there are some blocks which are not called anymore with this approach.

Would it be better to replace session with empty one and then call delege.delete?