Fix #2348 - ignore non-String environment properties (ClassCastException)#2446
Merged
Conversation
…ption) PropertyRegistry.saveProperties cast every java.util.Properties entry to Map<String, String>, so a non-String key or value (legal in Properties, e.g. after System.setProperties(...) with arbitrary objects) crashed startup with ClassCastException. The registry is String-keyed but stores Any values (PropertyRegistry._values is Map<String, Any>, getProperty uses `as? T`). So the correct fix keeps any String-keyed entry with its value as-is (Any) and only drops non-String keys (which can't be addressed as Koin property keys) — rather than forcing values to String. Removes the unchecked cast. Regression test (jvmTest): EnvironmentPropertiesTest — Properties with a non-String key, plus a String key holding an arbitrary object, no longer throws; the object value is preserved and retrievable, valid string props still load. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
arnaudgiuliani
force-pushed
the
fix/2348-env-properties-cast
branch
from
1486bbc to
26020e8
Compare
|
Thank you for fixing this, now I can finally update my koin app. Tho, is there any reason to drop all non-strings instead of allowing them as valid keys, like in #2358? |
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
2 participants
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.
Problem
PropertyRegistry.savePropertiescast everyjava.util.Propertiesentry toMap<String, String>:Propertiescan legally hold non-String keys/values (e.g. afterSystem.setProperties(...)with arbitrary objects — happens with some libraries/runtimes). The destructuring cast then throwsClassCastExceptiononstartKoin/loadEnvironmentProperties. Fixes #2348.Fix
Filter to
String/Stringentries and ignore the rest (debug-logged), per the issue's expected behavior ("start fine, ignoring invalid properties"). Removes the unchecked cast:properties.forEach { (k, v) -> if (k is String && v is String) saveProperty(k, v) else _koin.logger.debug("ignore non-string property '$k'") }Test
EnvironmentPropertiesTest(jvmTest) — aPropertiescontaining a non-String key, a non-String value, and a both-non-String entry no longer throws; the valid string property still loads, the invalid ones are ignored. RED (ClassCastException) → GREEN.Verification
:core:koin-core:jvmTestgreen;apiCheckpass.jvmMain), so no off-JVM targets involved.🤖 Generated with Claude Code