kotlinx.serialization icon indicating copy to clipboard operation
kotlinx.serialization copied to clipboard

The JPMS boundary should be concerned during access to serializer

Open iseki0 opened this issue 2 years ago • 11 comments

Describe the bug When we create a hand-written serializer, it can be linked with some type by @Serializable annotation. The hand-written serializer could be placed to another package which is not exported by module-info.class. The Json.decodeFromString<Type>(data) will be failed with an IllegalAccessException because of the JPMS edge.

Expected behavior Could we add some warning for it?

Environment

  • Kotlin version: 1.8.22
  • Library version: 1.8.22
  • Kotlin platforms: JVM
  • Gradle version: 8.2

iseki0 avatar Sep 01 '23 03:09 iseki0

Before that, I'm trying use JPMS to hide serializers in Java side. (The internal visibility modifier is not working in Java.)

iseki0 avatar Sep 01 '23 03:09 iseki0

Because serialization is intended to be statically resolved the serializer type is part of the ABI. As such it being (potentially) hidden by JPMS is expected behaviour.

ps. if you want to hide it from Java you can try the @JvmSynthetic annotation as tools will ignore synthetic types/members. pps. there is nothing that prevents serialization to be used from Java or makes it awkward (it isn't like inline classes or coroutines), so exposing the serializer types is valid.

pdvrieze avatar Sep 01 '23 08:09 pdvrieze

@pdvrieze I means there's no compilation error if the serializaer is hidden. As a instead there're runtime exception thrown during try to serialize/deserialize. It's a very bad development experience.

iseki0 avatar Sep 01 '23 09:09 iseki0

🤣I change the title, make it more clearly

iseki0 avatar Sep 01 '23 09:09 iseki0

@iseki0 I see, I assume that you use the Type.serializer() function. This is however an intrinsic now that will silently be replaced with the actual serializer. Your issue is probably with the intrinsic not observing JPMS.

pdvrieze avatar Sep 01 '23 11:09 pdvrieze

Although I'm not using it.(I'm prefer to using the standalone function serializer<Type>() instead). But the problem you said is correct. The intrinsic ignored the JPMS boundray during compilation. Sorry for my bad English, I use a wrong word. @pdvrieze

iseki0 avatar Sep 01 '23 13:09 iseki0

Hmm, interesting issue. Shouldn't just using a class reference (e.g. println(MyFooSerializer::class)) also trigger a JPMS error? In that case, this is a missing functionality in Kotlin or IDE, rather than a serialization plugin.

sandwwraith avatar Sep 11 '23 15:09 sandwwraith

@sandwwraith emmm Do you meaning to export a type with an internal serializer is a problem and it should be treat as an error reported by IDE and Kotlin compiler?

iseki0 avatar Sep 12 '23 10:09 iseki0

No, I mean referencing non-exported type should be an error in the IDE regardless whether it is used in serialization annotations or not.

Or, maybe I misunderstood you and you have a situation like this:

// File a.kt
package my

@Serializable(my.internal.MySerializer::class)
class MyClass

// File b.kt
package my.internal
class MySerializer: KSerializer<MyClass>

// module-info.java
exports my;

sandwwraith avatar Sep 12 '23 11:09 sandwwraith

@sandwwraith The way I understand the bug is that (given your example) serializer<MyClass>() compiles (and thus also Json.decodeFromString<Type>(data) which is just an inline function calling serializer()). But as the function is an intrinsic this transforms into a direct reference to MySerializer that is not public. But even if not an intrinsic, there would be an annotation that refers to a non-accessible type (I'm not sure what the correct behaviour is for that).

  • The intrinsic should check that the type that replaces the call is actually accessible (as if the code was written directly in the source file) rather than assume that it is.
  • For the non-intrinsic case it may be worthwhile to add a check where the type is statically known, but this would effectively be an intrinsic-light approach. Nothing in JPMS appears to be checking a package against its own package-info.java (in other words, a type inside a module can publicly expose an internal type - this type is just not accessible to users of the module to the related function is effectively internal). I don't think serialization should add module verification on module creation.

pdvrieze avatar Sep 12 '23 13:09 pdvrieze

I think the problem is clarificated. :)🥰

iseki0 avatar Nov 03 '23 19:11 iseki0