fix: don't resolve HOCON includes in config that arrived in a message (#3505) - #3518
Open
pjfanning wants to merge 1 commit into
Open
fix: don't resolve HOCON includes in config that arrived in a message (#3505)#3518pjfanning wants to merge 1 commit into
pjfanning wants to merge 1 commit into
Conversation
…apache#3505) * 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Backport of #3505 to 1.7.x: deserializing a message no longer resolves HOCON
includedirectives, which gave a peer a blind SSRF primitive and forced reads of local files and
classpath resources — reachable from
InitJoinbefore the sender is a cluster member.Modification
Cherry-pick of f7b0115, adapted for 1.7.x:
WireConfigSpecimportspekko.util.ccompat.JavaConvertersinstead ofscala.jdk.CollectionConverters, which 1.7.x provides only for Scala 2.13+.\"rewritten with triple quotes — that escape onlybecame valid inside interpolations in Scala 2.13.6, so it does not parse on 2.12.
MiscMessageSerializerand the two touched specs resolvedagainst the 1.7.x import style.
Result
Same as #3505: includes in wire config resolve to nothing; concise-rendered config from
well-behaved senders is unaffected.
Tests
sbt "++ 2.12.21 actor-tests/Test/compile" "++ 2.12.21 cluster/Test/compile" "++ 2.12.21 remote/Test/compile"— clean, validating the Scala 2.12 adaptationsthe ones reviewed and merged in fix: don't resolve HOCON includes in config that arrived in a message #3505
References
Backport of #3505.