materializeUnpickler no longer checks whether shared objects are supported

[rcano]

I implemented a custom PConfig and was that doesn't support shared objects. This PConfig has the behaviour that every field that is not present, returns null, instead of failing, but because the materialized unpickler is not checking whether this is supported or not, it always tries to get the ref field, and then the logic fails because it founds a null instead of a string. Shouldn't the materialized unpickler check this property?

[benhutchison]

The check is still there during Unpickling: https://github.com/benhutchison/prickle/blob/master/shared/src/main/scala/prickle/Unpickler.scala#L50

Here's what I need from you to investigate this issue:

• Version that you think worked
• Example of a payload that you are having trouble unpickling
• Relevant details of your PConfig.

On Sat, Jul 29, 2017 at 1:09 PM, rcano [email protected] wrote:

I implemented a custom PConfig and was that worked fine, until I updated to the latest version and started getting weird case of a ref field being accessed (which doesn't exists), comparing older version to current, I can see that the code that materializeUnpickler generates, no longer checks whether areSharedObjectsSupported is true, and hence always tries to go for that field, which messes with the behaviour of my PConfig where a missing field matches a null (instead of a Failure). Can this be reviewed? until then I'm stick to an older version.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/benhutchison/prickle/issues/43, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF05F1JBeKaTrFWKQB-NDzn2VrScw3sks5sSqJogaJpZM4OnPMU .

[rcano]

• Version that you think worked

Sorry if you read the issue text before I edited, I got confused with a fork I did of the project and a modification that I did on it. I don't think that there was a version where this worked.

• Example of a payload that you are having trouble unpickling

Any object payload where I try to load a case class from using the materialized unpickler.

• Relevant details of your PConfig.

It's a trivial PConfig using json4s's AST instead of microjson, it implements every method in the "obvious" way, and the only difference with JsConfig should be that when reading a field that is not present, my readObjectField returns JNull instead of throwing an exception when the field is not present.

This is the first line that a materialized unpickled does: https://github.com/benhutchison/prickle/blob/master/shared/src/main/scala/prickle/PicklerMaterializers.scala#L114 If I'm reading that correctly (I verified it in the generated AST too), the very first thing the generated unpickle method does is config.readObjectField(pickle, config.prefix + "ref") and that would be what is causing my issue. Right now I'm sidestepping this by special casing that key (prefix + "ref") in my PConf and always returning a cached exception, but I think that line in PicklerMaterializers should be changed to something like

      q"""
      if (config.areSharedObjectsSupported) config.readObjectField(pickle, config.prefix + "ref").transform({$unpickleRef}, _ => scala.util.Success($unpickleBody)).get
      else $unpickleBody
      """

to avoid querying for the ref field altogether when shared objects are not supported.

[benhutchison]

Ok, that make sense, I'll look into making that change

On Sun, Jul 30, 2017 at 1:43 AM, rcano [email protected] wrote:

• Version that you think worked

Sorry if you read the issue text before I edited, I got confused with a fork I did of the project and a modification that I did on it. I don't think that there was a version where this worked.

• Example of a payload that you are having trouble unpickling

Any object payload where I try to load a case class from using the materialized unpickler.

• Relevant details of your PConfig.

It's a trivial PConfig using json4s's AST instead of microjson, it implements every method in the "obvious" way, and the only difference with JsConfig should be that when readObjectField returns JNull instead of throwing an exception when the field is not present.

This is the first line that a materialized unpickled does: https://github.com/benhutchison/prickle/blob/master/shared/src/main/scala/ prickle/PicklerMaterializers.scala#L114 If I'm reading that correctly (I verified it in the generated AST too), the very first thing the generated unpickle method does is config.readObjectField(pickle, config.prefix + "ref") and that would be what is causing my issue. Right now I'm sidestepping this by special casing that key (prefix + "ref") in my PConf and always returning a cached exception, but I think that line in PicklerMaterializers should be changed to something like

  q"""      if (config.areSharedObjectsSupported) config.readObjectField(pickle, config.prefix + "ref").transform({$unpickleRef}, _ => scala.util.Success($unpickleBody)).get      else $unpickleBody      """

to avoid querying for the ref field altogether when shared objects are not supported.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/benhutchison/prickle/issues/43#issuecomment-318839149, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF05DmVnKxQulP_Kl4rY1Q-2IplHjaFks5sS1MPgaJpZM4OnPMU .