From cfff0b800cf67027b2362a887ce962e4a2a4f061 Mon Sep 17 00:00:00 2001 From: Patrik Nordwall Date: Tue, 25 Apr 2017 20:52:45 +0200 Subject: [PATCH] skip serialize-messages check for well known local messages, #22680 * when serialize-messages=on and allow-java-serialization=off --- .../DisabledJavaSerializerWarningSpec.scala | 46 +++++++++++++++++++ .../SerializationSetupSpec.scala | 15 ++++-- .../scala/akka/actor/dungeon/Dispatch.scala | 19 +++++--- .../akka/serialization/Serialization.scala | 6 ++- .../AllowJavaSerializationOffSpec.scala | 14 ++++-- 5 files changed, 84 insertions(+), 16 deletions(-) create mode 100644 akka-actor-tests/src/test/scala/akka/serialization/DisabledJavaSerializerWarningSpec.scala diff --git a/akka-actor-tests/src/test/scala/akka/serialization/DisabledJavaSerializerWarningSpec.scala b/akka-actor-tests/src/test/scala/akka/serialization/DisabledJavaSerializerWarningSpec.scala new file mode 100644 index 0000000000..a346d49efd --- /dev/null +++ b/akka-actor-tests/src/test/scala/akka/serialization/DisabledJavaSerializerWarningSpec.scala @@ -0,0 +1,46 @@ +/** + * Copyright (C) 2017 Lightbend Inc. + */ +package akka.serialization + +import scala.concurrent.duration._ +import akka.testkit._ +import akka.testkit.TestEvent._ + +object DisabledJavaSerializerWarningSpec { + final case class Msg(s: String) +} + +class DisabledJavaSerializerWarningSpec extends AkkaSpec( + """ + akka.actor { + allow-java-serialization = off + serialize-messages = on + # this is by default on, but tests are running with off + warn-about-java-serializer-usage = on + } + """) with ImplicitSender { + + import DisabledJavaSerializerWarningSpec._ + + "DisabledJavaSerializer warning" must { + + "be logged for suspicious messages" in { + EventFilter.warning(start = "Outgoing message attempted to use Java Serialization", occurrences = 1).intercept { + val echo = system.actorOf(TestActors.echoActorProps) + echo ! List("a") + expectNoMsg(300.millis) + } + + } + + "be skipped for well known local messages" in { + EventFilter.warning(start = "Outgoing message attempted to use Java Serialization", occurrences = 0).intercept { + val echo = system.actorOf(TestActors.echoActorProps) + echo ! Msg("a") // Msg is in the akka package + expectMsg(Msg("a")) + } + } + + } +} diff --git a/akka-actor-tests/src/test/scala/akka/serialization/SerializationSetupSpec.scala b/akka-actor-tests/src/test/scala/akka/serialization/SerializationSetupSpec.scala index 9d5f5d5278..adc4161252 100644 --- a/akka-actor-tests/src/test/scala/akka/serialization/SerializationSetupSpec.scala +++ b/akka-actor-tests/src/test/scala/akka/serialization/SerializationSetupSpec.scala @@ -5,6 +5,8 @@ package akka.serialization import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.AtomicInteger +import java.util.{ BitSet ⇒ ProgrammaticJavaDummy } +import java.util.{ Date ⇒ SerializableDummy } import akka.actor.setup.ActorSystemSetup import akka.actor.{ ActorSystem, BootstrapSetup, ExtendedActorSystem, Terminated } @@ -15,8 +17,6 @@ import scala.concurrent.duration._ class ConfigurationDummy class ProgrammaticDummy -case class ProgrammaticJavaDummy() -case class SerializableDummy() // since case classes are serializable /** * Keeps a registry of each object "serialized", returns an identifier, for every "deserialization" the original object @@ -57,6 +57,9 @@ object SerializationSetupSpec { actor { serialize-messages = off + # this is by default on, but tests are running with off, use defaults here + warn-about-java-serializer-usage = on + serialization-bindings { "akka.serialization.ConfigurationDummy" = test } @@ -70,6 +73,8 @@ object SerializationSetupSpec { akka { actor { allow-java-serialization = off + # this is by default on, but tests are running with off, use defaults here + warn-about-java-serializer-usage = on } } """.stripMargin)) @@ -107,9 +112,11 @@ class SerializationSetupSpec extends AkkaSpec( } val addedJavaSerializationProgramaticallyButDisabledSettings = BootstrapSetup(None, Some(ConfigFactory.parseString(""" akka { - loglevel = debug + loglevel = debug actor { allow-java-serialization = off + # this is by default on, but tests are running with off, use defaults here + warn-about-java-serializer-usage = on } } """)), None) @@ -147,7 +154,7 @@ class SerializationSetupSpec extends AkkaSpec( "disable java serialization also for incoming messages if serializer id usually would have found the serializer" in { val ser1 = SerializationExtension(system) - val msg = SerializableDummy() + val msg = new SerializableDummy val bytes = ser1.serialize(msg).get val serId = ser1.findSerializerFor(msg).identifier ser1.findSerializerFor(msg).includeManifest should ===(false) diff --git a/akka-actor/src/main/scala/akka/actor/dungeon/Dispatch.scala b/akka-actor/src/main/scala/akka/actor/dungeon/Dispatch.scala index bf3cfcae97..0587576e69 100644 --- a/akka-actor/src/main/scala/akka/actor/dungeon/Dispatch.scala +++ b/akka-actor/src/main/scala/akka/actor/dungeon/Dispatch.scala @@ -18,6 +18,7 @@ import akka.dispatch.MailboxType import akka.dispatch.ProducesMessageQueue import akka.serialization.SerializerWithStringManifest import akka.dispatch.UnboundedMailbox +import akka.serialization.DisabledJavaSerializer private[akka] trait Dispatch { this: ActorCell ⇒ @@ -157,13 +158,17 @@ private[akka] trait Dispatch { this: ActorCell ⇒ private def serializeAndDeserializePayload(obj: AnyRef): AnyRef = { val s = SerializationExtension(system) val serializer = s.findSerializerFor(obj) - val bytes = serializer.toBinary(obj) - serializer match { - case ser2: SerializerWithStringManifest ⇒ - val manifest = ser2.manifest(obj) - s.deserialize(bytes, serializer.identifier, manifest).get - case _ ⇒ - s.deserialize(bytes, obj.getClass).get + if (serializer.isInstanceOf[DisabledJavaSerializer] && !s.shouldWarnAboutJavaSerializer(obj.getClass, serializer)) + obj // skip check for known "local" messages + else { + val bytes = serializer.toBinary(obj) + serializer match { + case ser2: SerializerWithStringManifest ⇒ + val manifest = ser2.manifest(obj) + s.deserialize(bytes, serializer.identifier, manifest).get + case _ ⇒ + s.deserialize(bytes, obj.getClass).get + } } } diff --git a/akka-actor/src/main/scala/akka/serialization/Serialization.scala b/akka-actor/src/main/scala/akka/serialization/Serialization.scala index 9de3da6315..12240e40af 100644 --- a/akka-actor/src/main/scala/akka/serialization/Serialization.scala +++ b/akka-actor/src/main/scala/akka/serialization/Serialization.scala @@ -21,6 +21,7 @@ import java.util.concurrent.atomic.AtomicReference import scala.annotation.tailrec import java.util.NoSuchElementException +import akka.annotation.InternalApi object Serialization { @@ -389,7 +390,10 @@ class Serialization(val system: ExtendedActorSystem) extends Extension { serializer.isInstanceOf[JavaSerializer] && !system.settings.AllowJavaSerialization } - private def shouldWarnAboutJavaSerializer(serializedClass: Class[_], serializer: Serializer) = { + /** + * INTERNAL API + */ + @InternalApi private[akka] def shouldWarnAboutJavaSerializer(serializedClass: Class[_], serializer: Serializer) = { def suppressWarningOnNonSerializationVerification(serializedClass: Class[_]) = { //suppressed, only when warn-on-no-serialization-verification = off, and extending NoSerializationVerificationNeeded diff --git a/akka-remote/src/test/scala/akka/remote/serialization/AllowJavaSerializationOffSpec.scala b/akka-remote/src/test/scala/akka/remote/serialization/AllowJavaSerializationOffSpec.scala index 0befb79600..f600c2b95b 100644 --- a/akka-remote/src/test/scala/akka/remote/serialization/AllowJavaSerializationOffSpec.scala +++ b/akka-remote/src/test/scala/akka/remote/serialization/AllowJavaSerializationOffSpec.scala @@ -10,11 +10,11 @@ import akka.testkit.{ AkkaSpec, TestKit, TestProbe } import com.typesafe.config.ConfigFactory import scala.concurrent.duration._ import akka.actor.actorRef2Scala +import java.util.{ BitSet ⇒ ProgrammaticJavaDummy } +import java.util.{ Date ⇒ SerializableDummy } class ConfigurationDummy class ProgrammaticDummy -case class ProgrammaticJavaDummy() -case class SerializableDummy() // since case classes are serializable object AllowJavaSerializationOffSpec { @@ -28,6 +28,8 @@ object AllowJavaSerializationOffSpec { akka { actor { serialize-messages = off + # this is by default on, but tests are running with off, use defaults here + warn-about-java-serializer-usage = on serialization-bindings { "akka.serialization.ConfigurationDummy" = test @@ -42,6 +44,8 @@ object AllowJavaSerializationOffSpec { akka { actor { allow-java-serialization = off + # this is by default on, but tests are running with off, use defaults here + warn-about-java-serializer-usage = on } } """.stripMargin)) @@ -67,8 +71,10 @@ class AllowJavaSerializationOffSpec extends AkkaSpec( akka { loglevel = debug actor { - enable-additional-serialization-bindings = off # this should be overriden by the setting below, which should force it to be on + enable-additional-serialization-bindings = off # this should be overriden by the setting below, which should force it to be on allow-java-serialization = off + # this is by default on, but tests are running with off, use defaults here + warn-about-java-serializer-usage = on } } """)), None) @@ -111,7 +117,7 @@ class AllowJavaSerializationOffSpec extends AkkaSpec( "disable java serialization also for incoming messages if serializer id usually would have found the serializer" in { val ser1 = SerializationExtension(system) - val msg = SerializableDummy() + val msg = new SerializableDummy val bytes = ser1.serialize(msg).get val serId = ser1.findSerializerFor(msg).identifier ser1.findSerializerFor(msg).includeManifest should ===(false)