Hi Christoph, Ideally, you register your custom wrappers to a singleton ObjectWrapper instance, which you get from the also singleton freemarker.template.Configuration instance. I'm not sure if you already have some shared container object (like a Spring context) that exposes singletons. If not, you can share the singleton Configuration through a static field though. My point is that this is the duty of the ObjectWrapper instance, not of a global static registry. Well, the static registry surely works in your application. But, the idea of registering custom "sub-wrappers" under DefaultObjectWrapper came up in the past, although not to work around OSGi complications, but so that people can extend DefaultObjectWrapper (or some other stock subclass of it - ExtendableDefaultObjectWrapper or such) without actually extending the class. Then it's possible to "extend" the stock ObjectWrapper on runtime. That could also help with this OSGi issue.
One issue I spotted in above (possibly simplified) code is that you just look up the concrete class in the Map of the registry. Subclasses can break that lookup. It's more likely that you want to look up the class, and if that fails then the superclass of it, and so on. And then there are also the implemented interfaces that should be looked up. As of the need for custom classes like JsonPrimitiveFreemarkerWrapper, I think that's only necessary if you need some special functionality, like exposing a JsonObject as a hash (consider XML DOM wrapper in FreeMarker as an example). Otherwise white-listing should be able to take care of the security aspect. I guess at least. That though will still require the bundle to add its own white-list rules to the ObjectWrapper, but that's not a "sub-wrapper", just some list of methods. Actually, I'm not sure what JsonPrimitiveFreemarkerWrapper does. Is it a TemplateModel? Note that my goal is that commonly needed functionality goes into FreeMarker, and being able to add wrapping rules dynamically (i.e. without subclassing the stock ObjectWrapper) is a such thing. White-listing based security is too, though that's a different topic. BTW, you did write to the mailing list already (if you check the address you will see). No problem, just saying. On Wed, Oct 30, 2019 at 11:40 AM Christoph Rüger <[email protected]> wrote: > Hi Daniel, > based on our recent conversation about the security issue, we have > continued adding some things and I wanted to bring this up. Maybe something > is worth considering to add it to Freemarker... > > > *OSGI and Model Wrapping* > In an OSGI Application each bundle has a different classloader. In our case > BundleA can objects to the Freemarker Context and theoretically an object > of ClassA could become accessible (through a chain of getters when using > BeanModel). Our ObjectWrapper resides in BundleB and cannot see ClassA of > BundleA. Our CustomObjectWrapper lives in BundleB. > > That's why we looked for a way how BundleA can register a custom > WrappingCallback which is used by our CustomObjectWrapper in BundleB. > > Here is what we came up with: > > BundleA -> calls FMRegistry.registerCustomObjectWrapper(new > CustomFreemarkerJsonWrapper()) > > BundleB.CustomObjectWrapper --> calls FMRegistry.wrap(object) > > BundleB FMRegistry looks like this: > > > /** > > * Registers a custom Object Wrapper for Freemarker > > * object wrapping. > > * This can be used by other bundles to provide > > * custom wrappers for classes which are not known or visible here. > > * > > * *@param* clazz > > * *@param* customObjectWrapper > > */ > > *public* static *void* > registerCustomObjectWrapper(CustomFreemarkerObjectWrapperCallback > customObjectWrapperCallback) { > > List<Class> supportedTypes = customObjectWrapperCallback > .getSupportedTypes(); // e.g. Arrays.asList(JsonPrimitive.class, > JsonArray.class); > > *for* (Class clazz : supportedTypes) { > > *if*(!*this*.customFreemarkerObjectWrappers.containsKey(clazz)) { > > *this*.customFreemarkerObjectWrappers.put(clazz, > customObjectWrapperCallback > ); > > } > > } > > } > > > > /** > > * Used by {@link MyObjectWrapper} to wrap > > * unknown types which were registered by {@link > SynestyTemplateMethodModelWithCustomObjectWrapper} > > * > > * *@param* obj > > * *@return* a callback if found, or <code>null</code> if no callback was > registered for the given class (by obj.getClass()) > > */ > > *public* CustomFreemarkerObjectWrapperCallback wrap(Object obj) { > > *if*(obj == *null*) *return* *null*; > > *return* *this*.customFreemarkerObjectWrappers.get(obj.getClass()); > > } > > > Then in our CustomObjectWrapper in BundleB we do something like this > .wrap(obj) or .handleUnknownType(obj) > > // check if a customObjectWrapper is registered > > BiFunction<Object, BeansWrapper, Object> customWrapper = > FMRegistry.wrap(object); > > *if*(customWrapper != *null*) { > > > > Object wrappedObject = customWrapper.apply(object, *this*); > > *if*(wrappedObject != *null*) { > > *return* wrap(wrappedObject); > > } > > } > > /* ... */ > return super.wrap(object) // or > > > Here is an example of such a CallbackWrapper in BundleA: > > /** > > * A custom ObjectWrapper Callback for GSON Json Objects. Callback because > it > > * will be called by our {@link MyObjectWrapper} during wrapping phase on > the > > * fly. > > * > > * e.g. to be able to convert gson-JsonArray into a ArrayList (instead of > > * Iterable) so that we can access it in Freemarker as a sequence e.g. > seq[0], > > * seq[1] etc. > > * > > */ > > *public* *class* CustomFreemarkerJsonWrapper *implements* > CustomFreemarkerObjectWrapperCallback { > > > /** > > * Returns the classes which this wrapper supports. > > * > > * *@return* > > */ > > @Override > > *public* List<Class> getSupportedTypes() { > > *return* Arrays.*asList*(JsonPrimitive.*class*, JsonArray.*class*); > > } > > > @Override > > *public* Object apply(Object t, BeansWrapper beansWrapper) { > > *if* (t *instanceof* JsonArray) { > > > JsonArray asJsonArray = (JsonArray) t; > > *return* *new* JsonArrayFreemarkerWrapper(asJsonArray, beansWrapper); > > } *else* *if* (t *instanceof* JsonPrimitive) { > > *return* *new* JsonPrimitiveFreemarkerWrapper((JsonPrimitive) t, > beansWrapper); > > } *else* { > > *return* t; > > } > > } > > > } > > *To conclude:* > FMRegistry allows several bundles to register a custom objectWrapping > method for different classes. > Our custom ObjectWrapper can use those callbacks to wrap the objects which > are unknown to its own Classloader. > > This allows us to control ObjectWrapping in different bundles and solve > some classloader issues and avoid direct dependencies of bundles. > > This is also needed to solve our security issue, as you suggested to always > build own WrapperModels for "dangerous" objects. > > I will write another mail regarding more the security aspect, which is > related but would be too much for this part. > > > Let me know if you think the above makes sense. I can also bring it up in > the mailing list, but first wanted to check with you directly. > > Thanks > Christoph > > > > > -- > Christoph Rüger, Geschäftsführer > Synesty <https://synesty.com/> - Anbinden und Automatisieren ohne > Programmieren > > -- > Synesty GmbH > Moritz-von-Rohr-Str. 1a > 07745 Jena > Tel.: +49 3641 > 5596493Internet: https://synesty.com <https://synesty.com> > Informationen > zum Datenschutz: https://synesty.com/datenschutz > <https://synesty.com/datenschutz> > > Geschäftsführer: Christoph Rüger > > Unternehmenssitz: Jena > Handelsregister B beim Amtsgericht: Jena > > Handelsregister-Nummer: HRB 508766 > Ust-IdNr.: DE287564982 > -- Best regards, Daniel Dekany
