Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arrow-integrations-jackson-module Either seems Incompatible with Jackson default typing #115

Open
christophejan opened this issue Dec 7, 2022 · 3 comments
Labels
bug Something isn't working enhancement New feature or request question Further information is requested

Comments

@christophejan
Copy link

Describe the bug

arrow-integrations-jackson-module for Either throw exception when Jackson default typing is activated

To Reproduce

data class MyDataClass(val myValue: Any)

val objectMapper = ObjectMapper().apply {
    activateDefaultTyping(polymorphicTypeValidator, DefaultTyping.NON_FINAL, As.PROPERTY)
    registerKotlinModule()
    registerArrowModule()
}
objectMapper.writeValueAsString(MyDataClass("my either right value".right()))

Output :

Type id handling not implemented for type arrow.core.Either (by serializer of type arrow.integrations.jackson.module.internal.UnionTypeSerializer) (through reference chain: fr.maif.epa.backmobile.commons.infrastructure.cache.CacheServiceTest$Arrow integration Either$MyDataClass["myValue"])
com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Type id handling not implemented for type arrow.core.Either (by serializer of type arrow.integrations.jackson.module.internal.UnionTypeSerializer) (through reference chain: fr.maif.epa.backmobile.commons.infrastructure.cache.CacheServiceTest$Arrow integration Either$MyDataClass["myValue"])
	at app//com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:77)

Expected behavior

arrow-integrations-jackson-module should be able de serialize/deserialize this kind of value with Jackson default typing

Environment (please complete the following information):

  • kotlin version : 1.7.0
  • arrow-core version : 1.1.3
  • arrow-integrations-jackson-module version : 0.14.0

Additional context

Note that I know there is workaround. Currently I don't use arrow-integrations-jackson-module to manage this kind of serialization/deserialization but this imply to filter some Either properties and I think it would be great if arrow-integrations-jackson-module is able to handle Jackson default typing out of the box.

@christophejan christophejan added the bug Something isn't working label Dec 7, 2022
@myuwono
Copy link
Collaborator

myuwono commented Dec 14, 2022

Thank you for reporting this @christophejan I wonder if we should classify this as a feature instead of a bug?

Sorry for the late response I was out sick in the last couple of weeks.

I wonder what's the serialization output that we would expect when this feature is enabled? In particular how should the type & generic type information serialize? My concern is if we want to support this we will need to find some way to make sure that serialization-deserialization of the class round trip. I've done some quick investigation and so far I haven't had much luck. I can't find any good way to tell jackson to preserve the runtime information of the generic types contained within Either.Left<L> or Either.Right<R> as L and R are gone due to type-erasure.

We need to understand that default typing may pose some security risk so many security critical production applications have that setting disabled by default.

@myuwono myuwono added enhancement New feature or request question Further information is requested labels Dec 14, 2022
@myuwono
Copy link
Collaborator

myuwono commented Dec 14, 2022

We can perhaps address this in two steps - maybe firstly just ignore serializing the type information for either when default typing is activated. This way we can close this bug first so to make sure serialization works without throwing exception. Deserialization would definitely not work as we lost the generic type information in the typeId. However, maybe we can address round-tripping in a separate issue.. I'm still not convinced if we should do that though. In my humble opinion a serialization/deserialization that does not round trip sounds pretty bad.

@christophejan
Copy link
Author

Hello @myuwono ! Thank you very much for your answer.

You raise a lot of points... I will try to explain my point of view on some of them.

First, I completely agree that the goal is to support serialization-deserialization round trip.

Regarding the generated json, I'm not sure we need informations on the generic type... I don't see generic type information on generated json for a list for example... List items themself seems to take care of that through default typing...

Regarding the serialization, to conform to Jackson default typing configuration, I think we have to implement serializeWithType method and use the given TypeSerializer typeIdmethod with current value and START_OBJECT as valueShape...

Regarding deserialization, I don't know without digging further...

I've considered this use-case as bug because it breaks a basic Jackson build-in feature and that's not the case with most frequently used Jackson modules.

At last, default typing expose indeed to some security risks when misused (see JacksonPolymorphicDeserialization/security-risks-using-global-default-typing on wiki). That's not the case if all the content proceed by the ObjectMapper come from trusted sources. For example, for caching purpose, when all the content has been written in a secured store (eg : application dedicated Redis) by the application itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants