Skip to content
This repository was archived by the owner on May 31, 2022. It is now read-only.

Serialization of Session/Request Scope Beans should not rely on BeanFactory #705

Closed
rwinch opened this issue Feb 29, 2016 · 12 comments
Closed
Assignees

Comments

@rwinch
Copy link
Contributor

rwinch commented Feb 29, 2016

Currently Spring Security OAuth adds Session/Request Scope Beans in Session. These beans also serialize the BeanFactory associated to them. This can cause problems when the BeanFactory serializationId is different between serialization and deserialization.

Consider a single application that serializes OAuth2ClientContext. The application is updated to include one more bean name. The is then started up again and tries to deserialize the previous OAuth2ClientContext. The deserialization will now fail (if the serialization id changes based upon the names).

I think that Spring Security OAuth should serialize OAuth2ClientContext in a way that it is not impacted by the underlying BeanFactory.

See spring-cloud/spring-cloud-commons#93

@dsyer
Copy link
Contributor

dsyer commented Feb 29, 2016

These beans also serialize the BeanFactory associated to them.

Spring does this for scoped proxies. I don't think it needs to in this case, so I'd like to think about how we can solve it in Spring Core first. To change the behaviour here we'd have to give up having scoped proxies (which isn't a huge change, but requires a change in the contract for users, so we can 't really do it before 3.0).

The "one more bean name" thing is a red herring as well: it applies only if you are using the bean names to generate a serialization ID (Spring Cloud does this by default, but it's not an OAuth2 feature, so there's no point discussing it here really).

@rwinch
Copy link
Contributor Author

rwinch commented Feb 29, 2016

@dsyer

The "one more bean name" thing is a red herring as well: it applies only if you are using the bean names to generate a serialization ID (Spring Cloud does this by default, but it's not an OAuth2 feature, so there's no point discussing it here really).

Fair enough. Does this mean that Spring Cloud will stop generating the serialization id based on the beans?

@dsyer
Copy link
Contributor

dsyer commented Feb 29, 2016

Does this mean that Spring Cloud will stop generating the serialization id based on the beans?

Not necessarily. It's just an honest attempt at a default value that ought to work a lot of the time (and it does work more often than the default that you get from Spring Core). We seem to be discussing it in the wrong tracker though.

@rwinch
Copy link
Contributor Author

rwinch commented Feb 29, 2016

@dsyer Thanks for the response.

From my perspective this should work one way or another. So if Spring Cloud is not going to change, then this issue needs to be alive. If Spring Cloud is going to change, we can close this issue.

@dsyer
Copy link
Contributor

dsyer commented Feb 29, 2016

I don't think either issue should be closed. But the best solution might be in Spring Core, so we should probably open up the discussion there as well?

@rwinch
Copy link
Contributor Author

rwinch commented Feb 29, 2016

@dsyer That's probably a good idea. I'm not sure I understand your point about how to fix the issue in core though. Perhaps you can write the ticket up and cc me on it?

@joaotab
Copy link

joaotab commented Mar 7, 2016

I'm having an issue with Spring Boot + Spring Session + Hazelcast + Spring Security OAuth2 that seems to be related. Everything works fine when accessing the server that generated the OAuth2 context but as soon as I switch to another server it throws a deserialization exception:

com.hazelcast.nio.serialization.HazelcastSerializationException: java.lang.IllegalStateException: Cannot deserialize BeanFactory with id <artifact-id>:<profile>:<port>: no factory registered for this id
at com.hazelcast.nio.serialization.SerializationServiceImpl.handleException(SerializationServiceImpl.java:380) ~[hazelcast-3.5.4.jar:3.5.4]
[...]

@dsyer
Copy link
Contributor

dsyer commented Mar 7, 2016

I guess you need to set the serialization id then, for now? There's a code snippet in the spring cloud issue I think (but it's really easy).

@joaotab
Copy link

joaotab commented Mar 7, 2016

Yes, that's the workaround I'm currently using, found it somewhere on stackoverflow. Found this while checking if a better solution existed and tought you might like to know. Thanks anyway for the sugestion! Is there any nasty side effect around setting a fixed serialization id?

@dsyer
Copy link
Contributor

dsyer commented Apr 5, 2016

I created a Spring Framework issue here: https://jira.spring.io/browse/SPR-14117

@rwinch
Copy link
Contributor Author

rwinch commented Apr 5, 2016

@dsyer Thanks! Looks like Juergen has already gotten a fix too :)

@rwinch
Copy link
Contributor Author

rwinch commented May 12, 2016

Closing since this is fixed in Spring Framework

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants