Critical bug, prickle unpickles scala object from anything

[l15k4]

case class PipelineState(pipelineId: String, nodes: Seq[(String, Map[String, String])], edges: Set[(String, String)])
case object KeepWsAlive

Unpickle[KeepWsAlive.type].fromString(Pickle.intoString(PipelineState("x", Seq.empty[(String, Map[String, String])], Set.empty)))
--------------------------------
Success(KeepWsAlive)

This happens on both JVM and JS.

[benhutchison]

I don't see a bug here. Extra unneeded fields in the Json shouldn't prevent unpickling. Case objects require zero info to unpickle beyond their type.

On 19 Sep. 2017 1:34 am, "Jakub Liska" notifications@github.com wrote:

case class PipelineState(pipelineId: String, nodes: Seq[(String, Map[String, String])], edges: Set[(String, String)]) case object KeepWsAlive

Unpickle[KeepWsAlive.type].fromString(Pickle.intoString(PipelineState("x", Seq.empty[(String, Map[String, String])], Set.empty)))

Success(KeepWsAlive)

This happens on both JVM and JS.

— 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/44, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF05LSYKBUOYz0N445uGEHle84eabacks5sjo18gaJpZM4PbGU8 .

[l15k4]

Yeah, but it leads to very nasty bugs in user code base like the one that happened to me with websockets, I spent like 4 hours by chasing a connection problem because browser reported the classic upgrade protocol "101 Switching Protocols" error which in fact wasn't a problem, the problem was :

    ws.onmessage = (x: MessageEvent) => {
      x.data.toString match {
        case msg if Unpickle[KeepWsAlive.type].fromString(msg).isSuccess =>
          keepAlive()
        ......
      }
    }

:-)

[benhutchison]

I'll try to document this surprising behavior to warn users this can happen

On 19 Sep. 2017 7:39 am, "Jakub Liska" notifications@github.com wrote:

Yeah, but it leads to very nasty bugs in user code base like the one that happened to me with websockets, I spent like 4 hours by chasing a connection problem because browser reported the classic upgrade protocol "101 Switching Protocols" error which in fact wasn't a problem, the problem was :

ws.onmessage = (x: MessageEvent) => {
  x.data.toString match {
    case msg if Unpickle[KeepWsAlive.type].fromString(msg).isSuccess =>
      keepAlive()
    ......
  }
}

:-)

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

[benhutchison]

I'd recommend using a CompositePickler for signal objects eg

trait Signal case object KeepWsAlive extends Signal

That will ensure only the correct json will unpickle

On Tue, Sep 19, 2017 at 7:45 AM, Ben Hutchison brhutchison@gmail.com wrote:

I'll try to document this surprising behavior to warn users this can happen

On 19 Sep. 2017 7:39 am, "Jakub Liska" notifications@github.com wrote:

Yeah, but it leads to very nasty bugs in user code base like the one that happened to me with websockets, I spent like 4 hours by chasing a connection problem because browser reported the classic upgrade protocol "101 Switching Protocols" error which in fact wasn't a problem, the problem was :

ws.onmessage = (x: MessageEvent) => {
  x.data.toString match {
    case msg if Unpickle[KeepWsAlive.type].fromString(msg).isSuccess =>
      keepAlive()
    ......
  }
}

:-)

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

[l15k4]

Yeah, that's how I fixed it. Thanks Ben

[l15k4]

Yo Ben, there is an unexpected behavior even if I use the composite pickler, see the screen shot :

import prickle.{CompositePickler, Pickle, PicklerPair, Unpickle}

sealed trait Signal
case object KeepWsAlive extends Signal
case class PipelineState(
      pipelineId: String,
      partitionStates: Seq[(String, Map[String, String])],
      edges: Set[(String, String)]
    )

val pipelineState =
  PipelineState("x", Seq.empty[(String, Map[String, String])], Set.empty)

implicit val signalPickler: PicklerPair[Signal] =
  CompositePickler[Signal]
    .concreteType[KeepWsAlive.type]

Unpickle[Signal].fromString(Pickle.intoString(KeepWsAlive))
Unpickle[Signal].fromString(Pickle.intoString(pipelineState))
Unpickle[KeepWsAlive.type].fromString(Pickle.intoString(KeepWsAlive))
Unpickle[KeepWsAlive.type].fromString(Pickle.intoString(pipelineState))

[benhutchison]

I think final paragraph is relevant to this situation: https://github.com/benhutchison/prickle#support-for-class-hierarchies-and-sum-types

On Wed, Sep 20, 2017 at 6:55 AM, Jakub Liska notifications@github.com wrote:

Yo Ben, there is an unexpected behavior even if I use the composite pickler, see the screen shot :

import prickle.{CompositePickler, Pickle, PicklerPair, Unpickle}

sealed trait Signal case object KeepWsAlive extends Signal case class PipelineState( pipelineId: String, partitionStates: Seq[(String, Map[String, String])], edges: Set[(String, String)] )

val pipelineState = PipelineState("x", Seq.empty[(String, Map[String, String])], Set.empty)

implicit val signalPickler: PicklerPair[Signal] = CompositePickler[Signal] .concreteType[KeepWsAlive.type]

Unpickle[Signal].fromString(Pickle.intoString(KeepWsAlive)) Unpickle[Signal].fromString(Pickle.intoString(pipelineState)) Unpickle[KeepWsAlive.type].fromString(Pickle.intoString(KeepWsAlive)) Unpickle[KeepWsAlive.type].fromString(Pickle.intoString(pipelineState))

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