From 07114407caf39b80ac11dd3c65a1d081201a9df7 Mon Sep 17 00:00:00 2001 From: Sharang Gupta Date: Mon, 19 Oct 2026 21:02:01 +0100 Subject: [PATCH] Reject configuration property names with a repeated separator ConfigurationPropertyName.elementsOf already rejects a name with a leading or trailing dot, but a repeated dot anywhere in the name (e.g. "foo..bar") was accepted: ElementsParser.add silently drops the empty element produced between the two separators, so "foo..bar" and "foo...bar" both parse as identical to "foo.bar", and ConfigurationPropertyName.isValid("foo..bar") returns true. This matters beyond strict validation: the same ElementsParser is used by the lenient adapt(...) path that SpringIterableConfiguration PropertySource relies on to map raw property-source keys onto ConfigurationPropertyNames for relaxed binding. Two distinct raw keys that differ only by an accidental repeated separator (a typo in a properties file, or any code that builds a key by concatenation) collapse onto the same name there too, and the source's Cache keeps whichever one happens to land first in an unordered HashSet - so one of two real, distinct configuration values is silently and non-deterministically dropped, with no exception or log output. This change closes the gap in the strict of()/isValid() API by extending the existing leading/trailing-dot check to also reject an internal repeated separator, consistently with how the name is already documented and tested to behave at the start/end of the string. I want to be upfront that this does not, on its own, change the behavior of the lenient adapt(...) path, since that method is intentionally more permissive than of() (per its own Javadoc) and silently stripping or rejecting input is core to its contract; I did not want to unilaterally change that shared, more permissive code path without knowing which behavior the team would prefer for it (reject the key outright during relaxed-binding iteration, merge deterministically, or something else). Happy to follow up on that once the team has a view - I can reproduce the relaxed-binding collision with a small Binder-based test case if useful. Added ConfigurationPropertyNameTests.ofNameShouldNotContainDoubleDot and extended isValidWhenNotValidShouldReturnFalse to cover this. core:spring-boot test suite (965 tests, properties/binder packages) green, no regressions. Signed-off-by: Sharang Gupta --- .../source/ConfigurationPropertyName.java | 13 +++++++++++++ .../source/ConfigurationPropertyNameTests.java | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/core/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java b/core/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java index e106f395f20e..d9b22a1f5654 100644 --- a/core/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java +++ b/core/spring-boot/src/main/java/org/springframework/boot/context/properties/source/ConfigurationPropertyName.java @@ -672,6 +672,19 @@ private static Elements probablySingleElementOf(CharSequence name) { } throw new InvalidConfigurationPropertyNameException(name, Collections.singletonList('.')); } + for (int i = 0; i < name.length() - 1; i++) { + if (name.charAt(i) == '.' && name.charAt(i + 1) == '.') { + // A repeated separator produces an empty element between the two dots. + // ElementsParser.add silently drops empty elements, which would + // otherwise make this name indistinguishable from the same name with + // the repeated separator collapsed to one (e.g. "foo..bar" parsing as + // identical to "foo.bar"), silently merging two distinct names. + if (returnNullIfInvalid) { + return null; + } + throw new InvalidConfigurationPropertyNameException(name, Collections.singletonList('.')); + } + } Elements elements = new ElementsParser(name, '.', parserCapacity).parse(); for (int i = 0; i < elements.getSize(); i++) { if (elements.getType(i) == ElementType.NON_UNIFORM) { diff --git a/core/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameTests.java b/core/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameTests.java index 5367826b3858..519de64a113a 100644 --- a/core/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameTests.java +++ b/core/spring-boot/src/test/java/org/springframework/boot/context/properties/source/ConfigurationPropertyNameTests.java @@ -54,6 +54,22 @@ void ofNameShouldNotEndWithDot() { .withMessageContaining("is not valid"); } + @Test + void ofNameShouldNotContainDoubleDot() { + // A repeated separator produces an empty element, which ElementsParser.add + // silently drops. Without this check, "foo..bar" parses as identical to + // "foo.bar" (of("foo..bar").equals(of("foo.bar")) is true and + // isValid("foo..bar") is true), so two distinct property-source keys that + // differ only by a repeated separator silently collide on the same + // ConfigurationPropertyName. + assertThatExceptionOfType(InvalidConfigurationPropertyNameException.class) + .isThrownBy(() -> ConfigurationPropertyName.of("foo..bar")) + .withMessageContaining("is not valid"); + assertThatExceptionOfType(InvalidConfigurationPropertyNameException.class) + .isThrownBy(() -> ConfigurationPropertyName.of("foo...bar")) + .withMessageContaining("is not valid"); + } + @Test void ofNameShouldNotContainUppercase() { assertThatExceptionOfType(InvalidConfigurationPropertyNameException.class) @@ -731,6 +747,8 @@ void isValidWhenNotValidShouldReturnFalse() { assertThat(ConfigurationPropertyName.isValid("-foo")).isFalse(); assertThat(ConfigurationPropertyName.isValid("FooBar")).isFalse(); assertThat(ConfigurationPropertyName.isValid("foo!bar")).isFalse(); + assertThat(ConfigurationPropertyName.isValid("foo..bar")).isFalse(); + assertThat(ConfigurationPropertyName.isValid("foo...bar")).isFalse(); } @Test