Hello,
While working on the Demo Request bug (https://osgeo-org.atlassian.net/browse/GEOS-7513), I came across an issue with Resources. The bug is caused by buggy serialisation of a Resource instance stored in a Wicket Page.
It was me who made the Resource interface extend Serializable so that all Resources could be used as serializables.
The idea behind that was that Files are serializable and that made it easier to replace the use of “File” by the use of “Resource” in the webui pages amongst other things.
However, now I realise that this was a wrong thing to do. Resources are usually implemented as non-static inner classes, and it is an anti-pattern to make those Serializable, see:
https://www.securecoding.cert.org/confluence/display/java/SER05-J.+Do+not+serialize+instances+of+inner+classes
Making the Resource implementation classes static is also not an option. They need to reference their store, because there can be different stores at once and resources need to call their store for certain operations. Changing that would require rethinking the resource API altogether. It can’t be the case that we serialize the store and get multiple copies of it. It is a spring bean that refers to other spring beans such as LockProvider, FileWatcher etc. We can’t make all of that serializable and create new copies from all of them, just because we are serializing a webui page.
Anyway, I want to roll back this change. As far as I can see there are only two places in which the serialisation of resources is relied upon:
- the Demo Request page as mentioned,
- the importer module.
The alternative solution is to use a String with the path instead and rely upon the Resources.fromPath method - which uses the datadirectorystore for relative paths and Files.asResource for absolute paths.
I had the idea to simplify the multiple use of this pattern by creating a Resources.serializable(Resource) method which wraps a serializable resource around a non-serializable resource, relying on the Resources.fromPath method to do the serialisation. This way we need minimal change to the aforementioned modules.
Does anyone object to any of my suggestions?
I assume I will need to make a proposal?
Regards
Niels