Hi Radu,

In general I agree, that it is too easy to leak implementation details
here. Maybe I can use these mixins in the first way to write a lot of WARN
messages in such a case, plus the option not to serialize them (by default
turned off, so serializing them nevertheless).

But my priority is not the prevention of these leaks, but rather mitigating
the.risk of breaking the serialization process. In that case I did not
experience issues with serializing the list of resources.

Jörg


Am Di., 27. Juni 2023 um 19:12 Uhr schrieb Radu Cotescu <r...@apache.org>:

> Hi Jörg,
>
> I think you would have the same problem with that model given the other
> property, namely that list of resources.
>
> Unfortunately I don’t see an easy way out other than documenting this
> aspect and advising developers to avoid exposing implementation details.
> Whatever you’d change in the implementation to prevent this, it would be at
> least a behaviour change, potentially causing regressions similar to
> https://xkcd.com/1172/.
>
> Regards,
> Radu
>
> On Tue, 27 Jun 2023 at 18:35, Jörg Hoh <jhoh...@googlemail.com.invalid>
> wrote:
>
> > HI Stefan,
> >
> > I agree, but:
> >
> > I don't have control over the code, which causes the ResourceResolver to
> be
> > serialized. Also I cannot enforce, that such code is NOT written and
> > deployed (there are too many ways to make it wrong). Also because it
> > currently works, I would have to counter the "but it worked yesterday,
> and
> > I did not change anything since then" argument, in case it fails with an
> > exception like above.
> >
> > So I have to find another way to prevent the ResourceResolver from being
> > serialized ...
> >
> > Jörg
> >
> >
> >
> > Am Di., 27. Juni 2023 um 13:38 Uhr schrieb Stefan Seifert
> > <stefan.seif...@diva-e.com.invalid>:
> >
> > > well, using lombok with such a result is a bad idea. there should not
> be
> > a
> > > getResolver() method, this is an implementation detail and no getter
> > should
> > > be provided for it.
> > >
> > > so to put it another way: good that the serialization fails, so you can
> > > fix the calls to not add a getResolver() method!
> > >
> > > stefan
> > >
> > > p.s. i'm not a fan of lombok in general and have no experience with it,
> > > but I assume it can be configured to not expose the resolver.
> > >
> > > > -----Original Message-----
> > > > From: Jörg Hoh <jhoh...@googlemail.com.INVALID>
> > > > Sent: Tuesday, June 27, 2023 1:28 PM
> > > > To: Sling Developers List <dev@sling.apache.org>
> > > > Subject: Sling Model Exporter: Prevent serializing of a
> > ResourceResolver
> > > >
> > > > Hi,
> > > >
> > > > Assuming this Sling Model (using Lombok's @Getter annotation)
> > > >
> > > > @Getter
> > > > @Model(
> > > >         adaptables = { SlingHttpServletRequest.class },
> > > >         adapters = { MyModel.class, ComponentExporter.class },
> > > >         resourceType = MyModel.RESOURCE_TYPE) @Exporter(
> > > >         name = ExporterConstants.SLING_MODEL_EXPORTER_NAME,
> > > >         extensions = ExporterConstants.SLING_MODEL_EXTENSION)
> > > > public class MyModel implements ComponentExporter {
> > > >
> > > >         static final String RESOURCE_TYPE =
> "myapp/components/mymodel";
> > > >
> > > >         @Inject
> > > >         private ResourceResolver resolver;
> > > >
> > > >         @ChildResource
> > > >         private List<Resource> items;
> > > >
> > > > }
> > > >
> > > > When it this model is serialized via SlingModelExporter / Jackson,
> the
> > > > resolver field is also exported via the created getResolver())
> method.
> > > >
> > > > But serializing that does not always work:
> > > >
> > > > org.apache.sling.models.factory.ExportException:
> > > > com.fasterxml.jackson.databind.exc.InvalidDefinitionException: No
> > > > serializer found for class
> > > > com.day.cq.wcm.core.impl.policies.ContentPolicyManagerImpl and no
> > > > properties discovered to create BeanSerializer (to avoid exception,
> > > > disable
> > > > SerializationFeature.FAIL_ON_EMPTY_BEANS) (through reference chain:
> > > > com.myapp.PageImpl[":items"]> [...] > com.myapp.MyModel["resolver"]
> > > >
> > >org.apache.sling.resourceresolver.impl.ResourceResolverImpl["propertyMa
> > > > >p"]
> > > >
> > >java.util.HashMap["com.day.cq.wcm.core.impl.policies.ContentPolicyAdapt
> > > > >erFactory.ContentPolicy"])
> > > >     at
> > > >
> > >
> >
> org.apache.sling.models.jacksonexporter.impl.JacksonExporter.export(Jackso
> > > > nExporter.java:138)
> > > > [org.apache.sling.models.jacksonexporter:1.1.2]
> > > >     at
> > > >
> > >
> >
> org.apache.sling.models.impl.ModelAdapterFactory.exportModel(ModelAdapterF
> > > > actory.java:1333)
> > > > [org.apache.sling.models.impl:1.5.4]
> > > >
> > > >
> > > > I don't want to check each class I want to add to the propertyMap if
> it
> > > > can be serialized or not; and a more serious problem is that
> > serializing
> > > > the resourceResolver and it's properyMap can leak a lot of
> information,
> > > > which should be not get public.
> > > >
> > > > Do you see a way to prevent serialization of the ResourceResolver
> (and
> > > > potentially other types as well) without touching the model classes?
> > > >
> > > > Jörg
> > > >
> > > > --
> > > > Cheers,
> > > > Jörg Hoh,
> > > >
> > > > https://cqdump.joerghoh.de
> > > > Twitter: @joerghoh
> > >
> >
> >
> > --
> > Cheers,
> > Jörg Hoh,
> >
> > https://cqdump.joerghoh.de
> > Twitter: @joerghoh
> >
>


-- 
Cheers,
Jörg Hoh,

https://cqdump.joerghoh.de
Twitter: @joerghoh

Reply via email to