This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/pekko.git
The following commit(s) were added to refs/heads/1.7.x by this push:
new 274ec0d878 fix: don't resolve HOCON includes in config that arrived in
a message (#3505) (#3518)
274ec0d878 is described below
commit 274ec0d878028634393598c8405eb917c0b59298
Author: PJ Fanning <[email protected]>
AuthorDate: Fri Sep 4 11:26:01 2026 +0100
fix: don't resolve HOCON includes in config that arrived in a message
(#3505) (#3518)
* fix: don't resolve HOCON includes in config that arrived in a message
Motivation:
Three sites parse HOCON that came off the wire with the default parse
options: InternalClusterAction.InitJoin and InitJoinAck in
ClusterMessageSerializer, and the Config payload in MiscMessageSerializer.
HOCON include directives are resolved by the parser rather than by
resolve(), so include file(...) and include classpath(...) read from the
local filesystem and classpath and include url(...) performs an outbound
request, all while deserializing a peer's message. InitJoin is accepted
from a node that has not joined, in ClusterDaemon's uninitialized state.
Modification:
Add WireConfig (@InternalApi), which parses with a ConfigIncluder that
resolves every include to an empty object, and route the three sites
through it. The includer implements ConfigIncluderFile, ConfigIncluderURL
and ConfigIncluderClasspath as well as ConfigIncluder: the parser falls
back to its own handling, which does read the resource, for any of the
typed forms the configured includer does not implement.
Every serializer writes config with ConfigRenderOptions.concise, which
renders JSON and cannot produce an include, so a well-behaved sender is
unaffected.
Result:
Deserializing a message no longer reads local files or issues outbound
requests on behalf of the sender.
* Update WireConfigSpec.scala
---
.../pekko/serialization/WireConfigSpec.scala | 93 ++++++++++++++++++++++
.../apache/pekko/serialization/WireConfig.scala | 82 +++++++++++++++++++
.../protobuf/ClusterMessageSerializer.scala | 4 +-
.../protobuf/ClusterMessageSerializerSpec.scala | 35 ++++++++
.../serialization/MiscMessageSerializer.scala | 10 ++-
.../serialization/MiscMessageSerializerSpec.scala | 21 ++++-
6 files changed, 240 insertions(+), 5 deletions(-)
diff --git
a/actor-tests/src/test/scala/org/apache/pekko/serialization/WireConfigSpec.scala
b/actor-tests/src/test/scala/org/apache/pekko/serialization/WireConfigSpec.scala
new file mode 100644
index 0000000000..cf0ab9831c
--- /dev/null
+++
b/actor-tests/src/test/scala/org/apache/pekko/serialization/WireConfigSpec.scala
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.pekko.serialization
+
+import java.nio.charset.StandardCharsets
+import java.nio.file.{ Files, Path }
+
+import org.apache.pekko.util.ccompat.JavaConverters._
+
+import org.apache.pekko.testkit.PekkoSpec
+
+import com.typesafe.config.{ ConfigFactory, ConfigRenderOptions }
+
+class WireConfigSpec extends PekkoSpec {
+
+ // a file the parser must not read when a message asks it to
+ private val secretFile: Path = {
+ val f = Files.createTempFile("wire-config-spec", ".conf")
+ Files.write(f, "secret = leaked".getBytes(StandardCharsets.UTF_8))
+ f
+ }
+ private val filePath = secretFile.toAbsolutePath.toString
+ private val fileUrl = secretFile.toUri.toString
+
+ override def afterTermination(): Unit = Files.deleteIfExists(secretFile)
+
+ "WireConfig" must {
+
+ "parse ordinary HOCON" in {
+ val config = WireConfig.parseString("a = 1\nb { c = two }")
+ config.getInt("a") should ===(1)
+ config.getString("b.c") should ===("two")
+ }
+
+ "parse what a serializer writes" in {
+ // every serializer renders config with ConfigRenderOptions.concise
+ val rendered =
+ ConfigFactory.parseString("pekko.cluster.roles = [a,
b]").root.render(ConfigRenderOptions.concise())
+
WireConfig.parseString(rendered).getStringList("pekko.cluster.roles").asScala.toList
should ===(List("a", "b"))
+ }
+
+ "not read a file named by an include" in {
+ // s"...\"..." is not valid Scala 2.12, hence the triple quotes
+ val config = WireConfig.parseString(s"""include file("$filePath")
+ |a = 1""".stripMargin)
+ config.hasPath("secret") should ===(false)
+ config.getInt("a") should ===(1)
+ }
+
+ "not read a file named by a required include" in {
+ WireConfig
+ .parseString(s"""include required(file("$filePath"))
+ |a = 1""".stripMargin)
+ .hasPath("secret") should ===(false)
+ }
+
+ "not fetch a URL named by an include" in {
+ // a file: URL stands in for an outbound request, so the test needs no
network
+ val config = WireConfig.parseString(s"""include url("$fileUrl")
+ |a = 1""".stripMargin)
+ config.hasPath("secret") should ===(false)
+ config.getInt("a") should ===(1)
+ }
+
+ "not read a resource named by a classpath include" in {
+ // reference.conf is on the test classpath, so the default includer
would pull it in
+ WireConfig.parseString("include classpath(\"reference.conf\")\na =
1").hasPath("pekko.version") should ===(false)
+ }
+
+ "differ from the default parser, which does resolve all three" in {
+ // guards the premise of the tests above: these directives really do
resolve without the
+ // includer, so those tests are checking the change rather than an inert
directive
+ ConfigFactory.parseString(s"""include
file("$filePath")""").hasPath("secret") should ===(true)
+ ConfigFactory.parseString(s"""include
url("$fileUrl")""").hasPath("secret") should ===(true)
+ ConfigFactory.parseString("include
classpath(\"reference.conf\")").hasPath("pekko.version") should ===(true)
+ }
+ }
+}
diff --git
a/actor/src/main/scala/org/apache/pekko/serialization/WireConfig.scala
b/actor/src/main/scala/org/apache/pekko/serialization/WireConfig.scala
new file mode 100644
index 0000000000..673f56df34
--- /dev/null
+++ b/actor/src/main/scala/org/apache/pekko/serialization/WireConfig.scala
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.pekko.serialization
+
+import java.io.File
+import java.net.URL
+
+import org.apache.pekko.annotation.InternalApi
+
+import com.typesafe.config.{
+ Config,
+ ConfigFactory,
+ ConfigIncludeContext,
+ ConfigIncluder,
+ ConfigIncluderClasspath,
+ ConfigIncluderFile,
+ ConfigIncluderURL,
+ ConfigObject,
+ ConfigParseOptions
+}
+
+/**
+ * INTERNAL API
+ *
+ * Parsing of HOCON that arrived in a message.
+ *
+ * HOCON `include` directives are resolved by the parser, not by `resolve()`,
so parsing a
+ * string with the default includer reads whatever it names: `include
file(...)` and
+ * `include classpath(...)` read from the local filesystem and classpath, and
+ * `include url(...)` performs an outbound request. None of that belongs on a
path whose
+ * input came from a peer.
+ *
+ * Every serializer writes config with `ConfigRenderOptions.concise`, which
renders JSON and
+ * cannot produce an `include`, so dropping them costs a well-behaved sender
nothing.
+ */
+@InternalApi private[pekko] object WireConfig {
+
+ /**
+ * Resolves every form of `include` to an empty object.
+ *
+ * All four interfaces have to be implemented: the parser dispatches
`include file(...)`,
+ * `include url(...)` and `include classpath(...)` to the typed methods and
falls back to
+ * its own default handling — which does read the resource — when the
configured includer
+ * does not implement the matching interface. Only bare `include "..."` goes
to `include`.
+ */
+ private object NoIncludes
+ extends ConfigIncluder
+ with ConfigIncluderFile
+ with ConfigIncluderURL
+ with ConfigIncluderClasspath {
+
+ private def empty: ConfigObject = ConfigFactory.empty().root()
+
+ override def withFallback(fallback: ConfigIncluder): ConfigIncluder = this
+ override def include(context: ConfigIncludeContext, what: String):
ConfigObject = empty
+ override def includeFile(context: ConfigIncludeContext, what: File):
ConfigObject = empty
+ override def includeURL(context: ConfigIncludeContext, what: URL):
ConfigObject = empty
+ override def includeResources(context: ConfigIncludeContext, what:
String): ConfigObject = empty
+ }
+
+ private val parseOptions: ConfigParseOptions =
ConfigParseOptions.defaults().setIncluder(NoIncludes)
+
+ /**
+ * Like `ConfigFactory.parseString`, but with `include` directives resolved
to nothing.
+ */
+ def parseString(hocon: String): Config = ConfigFactory.parseString(hocon,
parseOptions)
+}
diff --git
a/cluster/src/main/scala/org/apache/pekko/cluster/protobuf/ClusterMessageSerializer.scala
b/cluster/src/main/scala/org/apache/pekko/cluster/protobuf/ClusterMessageSerializer.scala
index 48c0d4a83d..557f9a274b 100644
---
a/cluster/src/main/scala/org/apache/pekko/cluster/protobuf/ClusterMessageSerializer.scala
+++
b/cluster/src/main/scala/org/apache/pekko/cluster/protobuf/ClusterMessageSerializer.scala
@@ -314,7 +314,7 @@ final class ClusterMessageSerializer(val system:
ExtendedActorSystem)
private def deserializeInitJoin(bytes: Array[Byte]):
InternalClusterAction.InitJoin = {
val m = cm.InitJoin.parseFrom(bytes)
if (m.hasCurrentConfig)
-
InternalClusterAction.InitJoin(ConfigFactory.parseString(m.getCurrentConfig))
+
InternalClusterAction.InitJoin(WireConfig.parseString(m.getCurrentConfig))
else
InternalClusterAction.InitJoin(ConfigFactory.empty)
}
@@ -325,7 +325,7 @@ final class ClusterMessageSerializer(val system:
ExtendedActorSystem)
val configCheck =
i.getConfigCheck.getType match {
case cm.ConfigCheck.Type.CompatibleConfig =>
-
CompatibleConfig(ConfigFactory.parseString(i.getConfigCheck.getClusterConfig))
+
CompatibleConfig(WireConfig.parseString(i.getConfigCheck.getClusterConfig))
case cm.ConfigCheck.Type.IncompatibleConfig => IncompatibleConfig
case cm.ConfigCheck.Type.UncheckedConfig => UncheckedConfig
}
diff --git
a/cluster/src/test/scala/org/apache/pekko/cluster/protobuf/ClusterMessageSerializerSpec.scala
b/cluster/src/test/scala/org/apache/pekko/cluster/protobuf/ClusterMessageSerializerSpec.scala
index 7615de9585..6bb4b07707 100644
---
a/cluster/src/test/scala/org/apache/pekko/cluster/protobuf/ClusterMessageSerializerSpec.scala
+++
b/cluster/src/test/scala/org/apache/pekko/cluster/protobuf/ClusterMessageSerializerSpec.scala
@@ -13,6 +13,9 @@
package org.apache.pekko.cluster.protobuf
+import java.nio.charset.StandardCharsets
+import java.nio.file.Files
+
import collection.immutable.SortedSet
import scala.annotation.nowarn
@@ -22,6 +25,7 @@ import org.apache.pekko
import pekko.actor.{ Address, ExtendedActorSystem }
import pekko.cluster._
import pekko.cluster.InternalClusterAction.CompatibleConfig
+import pekko.cluster.protobuf.msg.{ ClusterMessages => cm }
import pekko.cluster.routing.{ ClusterRouterPool, ClusterRouterPoolSettings }
import pekko.routing.RoundRobinPool
import pekko.testkit.PekkoSpec
@@ -183,6 +187,37 @@ class ClusterMessageSerializerSpec extends
PekkoSpec("pekko.actor.provider = clu
env.gossip.members.tail.head.roles should be(Set("r1",
ClusterSettings.DcRolePrefix + "foo"))
}
+ "not resolve includes in the config of a join message" in {
+ // The joining node renders its config with ConfigRenderOptions.concise,
which is JSON and
+ // cannot carry an include, so nothing legitimate is lost by refusing to
resolve one. An
+ // include that did resolve would read a local file or issue an outbound
request while
+ // deserializing a message from a node that has not joined yet.
+ val secretFile = Files.createTempFile("cluster-message-serializer-spec",
".conf")
+ try {
+ Files.write(secretFile, """secret =
"leaked"""".getBytes(StandardCharsets.UTF_8))
+ val hocon = s"""include file("${secretFile.toAbsolutePath}")
+ pekko.cluster.roles = []"""
+
+ val initJoin = serializer
+
.fromBinary(cm.InitJoin.newBuilder().setCurrentConfig(hocon).build().toByteArray,
"IJ")
+ .asInstanceOf[InternalClusterAction.InitJoin]
+ initJoin.configOfJoiningNode.hasPath("secret") should ===(false)
+
+ val ackBytes = cm.InitJoinAck
+ .newBuilder()
+ .setAddress(serializer.addressToProto(Address("pekko", "system",
"some.host.org", 4711)))
+ .setConfigCheck(
+ cm.ConfigCheck
+ .newBuilder()
+ .setType(cm.ConfigCheck.Type.CompatibleConfig)
+ .setClusterConfig(hocon))
+ .build()
+ .toByteArray
+ val ack = serializer.fromBinary(ackBytes,
"IJA").asInstanceOf[InternalClusterAction.InitJoinAck]
+
ack.configCheck.asInstanceOf[CompatibleConfig].clusterConfig.hasPath("secret")
should ===(false)
+ } finally Files.deleteIfExists(secretFile)
+ }
+
"add a default data center role to internal join action if none is
present" in {
val join = roundtrip(InternalClusterAction.Join(a1.uniqueAddress, Set(),
Version.Zero))
join.roles should be(Set(ClusterSettings.DcRolePrefix + "default"))
diff --git
a/remote/src/main/scala/org/apache/pekko/remote/serialization/MiscMessageSerializer.scala
b/remote/src/main/scala/org/apache/pekko/remote/serialization/MiscMessageSerializer.scala
index 70c6c92591..ad86f34b34 100644
---
a/remote/src/main/scala/org/apache/pekko/remote/serialization/MiscMessageSerializer.scala
+++
b/remote/src/main/scala/org/apache/pekko/remote/serialization/MiscMessageSerializer.scala
@@ -29,7 +29,13 @@ import pekko.remote._
import pekko.remote.WireFormats.AddressData
import pekko.remote.routing.RemoteRouterConfig
import pekko.routing._
-import pekko.serialization.{ BaseSerializer, Serialization,
SerializationExtension, SerializerWithStringManifest }
+import pekko.serialization.{
+ BaseSerializer,
+ Serialization,
+ SerializationExtension,
+ SerializerWithStringManifest,
+ WireConfig
+}
import pekko.util.ccompat.JavaConverters._
class MiscMessageSerializer(val system: ExtendedActorSystem) extends
SerializerWithStringManifest with BaseSerializer {
@@ -544,7 +550,7 @@ class MiscMessageSerializer(val system:
ExtendedActorSystem) extends SerializerW
private def deserializeConfig(bytes: Array[Byte]): Config = {
if (bytes.isEmpty) EmptyConfig
- else ConfigFactory.parseString(new String(bytes, StandardCharsets.UTF_8))
+ else WireConfig.parseString(new String(bytes, StandardCharsets.UTF_8))
}
private def deserializeFromConfig(bytes: Array[Byte]): FromConfig =
diff --git
a/remote/src/test/scala/org/apache/pekko/remote/serialization/MiscMessageSerializerSpec.scala
b/remote/src/test/scala/org/apache/pekko/remote/serialization/MiscMessageSerializerSpec.scala
index 570e3f8d4c..78b808a5e4 100644
---
a/remote/src/test/scala/org/apache/pekko/remote/serialization/MiscMessageSerializerSpec.scala
+++
b/remote/src/test/scala/org/apache/pekko/remote/serialization/MiscMessageSerializerSpec.scala
@@ -14,6 +14,8 @@
package org.apache.pekko.remote.serialization
import java.io.NotSerializableException
+import java.nio.charset.StandardCharsets
+import java.nio.file.Files
import java.util.Optional
import java.util.concurrent.TimeoutException
@@ -21,7 +23,7 @@ import scala.annotation.nowarn
import scala.concurrent.duration._
import scala.util.control.NoStackTrace
-import com.typesafe.config.ConfigFactory
+import com.typesafe.config.{ Config, ConfigFactory }
import org.apache.pekko
import pekko.{ Done, NotUsed }
import pekko.actor._
@@ -157,6 +159,23 @@ class MiscMessageSerializerSpec extends
PekkoSpec(MiscMessageSerializerSpec.test
}
}
+ "not resolve includes in a serialized Config" in {
+ // Config is written with ConfigRenderOptions.concise, which is JSON and
cannot carry an
+ // include, so refusing to resolve one loses nothing. An include that
did resolve would
+ // read a local file or issue an outbound request while deserializing a
peer's message.
+ val secretFile = Files.createTempFile("misc-message-serializer-spec",
".conf")
+ try {
+ Files.write(secretFile, """secret =
"leaked"""".getBytes(StandardCharsets.UTF_8))
+ val serializer = new
MiscMessageSerializer(system.asInstanceOf[ExtendedActorSystem])
+ val hocon = s"""include file("${secretFile.toAbsolutePath}")
+ a = 1"""
+
+ val config =
serializer.fromBinary(hocon.getBytes(StandardCharsets.UTF_8),
"CF").asInstanceOf[Config]
+ config.hasPath("secret") should ===(false)
+ config.getInt("a") should ===(1)
+ } finally Files.deleteIfExists(secretFile)
+ }
+
"reject invalid manifest" in {
intercept[IllegalArgumentException] {
val serializer = new
MiscMessageSerializer(system.asInstanceOf[ExtendedActorSystem])
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
