diff --git a/spring-beans/src/main/java/org/springframework/beans/InvalidPropertyPathException.java b/spring-beans/src/main/java/org/springframework/beans/InvalidPropertyPathException.java new file mode 100644 index 00000000000..181c2af4feb --- /dev/null +++ b/spring-beans/src/main/java/org/springframework/beans/InvalidPropertyPathException.java @@ -0,0 +1,72 @@ +/* + * Copyright 2002-present the original author or authors. + * + * Licensed 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 + * + * https://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.springframework.beans; + +/** + * Exception thrown when a property path is not a well-formed property path + * according to the grammar implemented by {@link PropertyPath}. + * + *

This signals a syntactically invalid path, as opposed to a + * syntactically valid path that happens not to resolve against a particular + * target object. The latter cases are reported as {@link NotReadablePropertyException} + * or {@link NotWritablePropertyException} instead. + * + *

Extends {@link PropertyAccessException} so that a malformed path + * encountered while binding a single property value can be collected into a + * {@link PropertyBatchUpdateException} alongside other per-property failures, + * rather than aborting the whole binding operation. + * + * @author Brian Clozel + * @since 7.1 + * @see PropertyPath#parse(String) + */ +@SuppressWarnings("serial") +public class InvalidPropertyPathException extends PropertyAccessException { + + /** + * Error code that an {@code InvalidPropertyPathException} is registered with. + */ + public static final String ERROR_CODE = "invalidPropertyPath"; + + + private final String propertyPath; + + + /** + * Create a new {@code InvalidPropertyPathException}. + * @param propertyPath the offending property path + * @param reason a description of the grammar rule that was violated + */ + public InvalidPropertyPathException(String propertyPath, String reason) { + super("Invalid property path '" + propertyPath + "': " + reason, null); + this.propertyPath = propertyPath; + } + + + /** + * Return the offending property path. + */ + public String getPropertyPath() { + return this.propertyPath; + } + + @Override + public String getErrorCode() { + return ERROR_CODE; + } + +} diff --git a/spring-beans/src/main/java/org/springframework/beans/PropertyPath.java b/spring-beans/src/main/java/org/springframework/beans/PropertyPath.java new file mode 100644 index 00000000000..7bba8254198 --- /dev/null +++ b/spring-beans/src/main/java/org/springframework/beans/PropertyPath.java @@ -0,0 +1,539 @@ +/* + * Copyright 2002-present the original author or authors. + * + * Licensed 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 + * + * https://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.springframework.beans; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.jspecify.annotations.Nullable; + +import org.springframework.util.Assert; + +/** + * A parsed bean property path, such as {@code "person.addresses[1].city"}. + * + *

Parses bean property paths for property access: it decides whether a given + * string is a well-formed property path (see grammar) and where each path + * segment begins and ends. The {@linkplain #canonicalName() canonical form} + * is used for policy matching and error reporting. + * The {@linkplain #segments() structured segment list} is used for property + * navigation within beans. + * + *

The grammar is: + *

+ * PropertyPath := Segment ('.' Segment)*
+ * Segment      := Name Index*                  -- Name may be empty only if at least one Index follows
+ * Name         := char* excluding '.', '[', ']'
+ * Index        := '[' Key ']'
+ * Key          := QuotedKey | RawKey
+ * QuotedKey    := "'" [^']* "'"  |  '"' [^"]* '"'
+ * RawKey       := char* excluding quote characters, with balanced '[' / ']' nesting
+ * 
+ * + *

Invalid property paths are rejected with {@link InvalidPropertyPathException}. + * + * @author Brian Clozel + * @since 7.1 + */ +public final class PropertyPath { + + + private final String canonicalName; + + private final List segments; + + + private PropertyPath(String canonicalName, List segments) { + this.canonicalName = canonicalName; + this.segments = segments; + } + + + /** + * Parse the given property path. + * @param path the property path to parse; an empty string is a valid path + * with no segments + * @return the parsed property path + * @throws InvalidPropertyPathException if the given path is not a + * well-formed property path + */ + public static PropertyPath parse(String path) throws InvalidPropertyPathException { + return parse(path, Options.UNLIMITED); + } + + /** + * Parse the given property path, rejecting it if it exceeds the given {@code options}. + * @param path the property path to parse; an empty string is a valid path + * with no segments + * @param options the parsing options to apply + * @return the parsed property path + * @throws InvalidPropertyPathException if the given path is not a + * well-formed property path, or if it exceeds the given options + */ + public static PropertyPath parse(String path, Options options) throws InvalidPropertyPathException { + Assert.notNull(path, "Property path must not be null"); + Assert.notNull(options, "Options must not be null"); + if (path.isEmpty()) { + return new PropertyPath("", Collections.emptyList()); + } + PropertyPath parsed = new Parser(path).parse(); + int nestingDepth = parsed.segments.size() - 1; + if (nestingDepth > options.maxNestedPathDepth) { + throw new InvalidPropertyPathException(path, + "nesting depth exceeds the maximum of " + options.maxNestedPathDepth); + } + return parsed; + } + + /** + * {@code path}'s canonical form, or {@code path} itself if it is not a + * well-formed property path (including a {@code null} path, for which + * this returns an empty string). + *

A convenience for callers with nothing better to fall back to than + * the original string, such as canonicalizing a user-supplied field name + * for display, comparison, or configuration matching, as opposed to + * {@link #parse(String)} itself, whose non-throwing behavior would be the + * wrong default for a caller that is about to navigate an object graph. + * @param path the property path to canonicalize, possibly {@code null} + * @return the canonical form of {@code path}, or {@code path} unchanged + * (or an empty string, if {@code path} is {@code null}) if it is not a + * well-formed property path + * @since 7.1 + */ + public static String canonicalNameOrOriginal(@Nullable String path) { + if (path == null) { + return ""; + } + try { + return parse(path).canonicalName(); + } + catch (InvalidPropertyPathException ex) { + return path; + } + } + + /** + * Return the canonical string form of this path. + *

Unnecessary surrounding quotes are removed from keys: + * {@code map['key'].name} → {@code map[key].name}. + */ + public String canonicalName() { + return this.canonicalName; + } + + /** + * Return the segments of this path, in order, as an unmodifiable list. + */ + public List segments() { + return this.segments; + } + + /** + * Return the sub-path made up of this path's segments from the given + * index to the end, such as the {@code "country.name"} sub-path of + * {@code "address.country.name"} from index 1. + * @param fromIndex the index of the first segment to include (inclusive) + * @return the sub-path starting at {@code fromIndex} + * @throws IndexOutOfBoundsException if {@code fromIndex} is negative + * @throws IllegalArgumentException if {@code fromIndex} is greater than + * {@link #segments()}{@code .size()} + */ + public PropertyPath subPath(int fromIndex) { + if (fromIndex == 0) { + return this; + } + List subSegments = this.segments.subList(fromIndex, this.segments.size()); + StringBuilder subCanonicalName = new StringBuilder(); + for (int i = 0; i < subSegments.size(); i++) { + if (i > 0) { + subCanonicalName.append(PropertyAccessor.NESTED_PROPERTY_SEPARATOR_CHAR); + } + subCanonicalName.append(subSegments.get(i).toCanonicalName()); + } + return new PropertyPath(subCanonicalName.toString(), subSegments); + } + + @Override + public boolean equals(@Nullable Object other) { + return (this == other || (other instanceof PropertyPath that && + this.canonicalName.equals(that.canonicalName))); + } + + @Override + public int hashCode() { + return this.canonicalName.hashCode(); + } + + @Override + public String toString() { + return this.canonicalName; + } + + + /** + * A dot-separated segment of a property path. It consists of a + * property name and the keys of any indexes applied to it, if the + * target property is an indexed collection. + *

For example, the path {@code "map[key].name"} has two segments: + * {@code Segment["map", ["key"]]} and {@code Segment["name", []]}. + * @param name the property name, which is empty only for a root-level + * indexed access such as {@code "[user]"} + * @param keys the keys of the indexes applied to the property, with any + * surrounding quotes removed; never {@code null}, possibly empty + */ + public record Segment(String name, List keys) { + + public Segment(String name, List keys) { + Assert.notNull(name, "Segment name must not be null"); + Assert.notNull(keys, "Segment keys must not be null"); + this.name = name; + this.keys = List.copyOf(keys); + } + + /** + * The canonical format of this segment alone: its name, followed by each + * key wrapped in brackets, quoted only when necessary. + */ + public String toCanonicalName() { + StringBuilder canonicalNameBuilder = new StringBuilder(); + canonicalNameBuilder.append(this.name); + for (String key : this.keys) { + appendCanonicalKey(canonicalNameBuilder, key); + } + return canonicalNameBuilder.toString(); + } + + /** + * Return a new segment instance with the same name, but dropping the last key. + */ + public Segment withoutLastKey() { + if (this.keys.isEmpty()) { + return this; + } + return new Segment(this.name, this.keys.subList(0, this.keys.size() - 1)); + } + + /** + * Append the canonical form of the given key, which is the key + * unquoted, or the key re-quoted if it contains illegal raw chars, + * such as {@code map['a]b']}. + */ + private void appendCanonicalKey(StringBuilder canonicalNameBuilder, String key) { + char quoteChar = canonicalQuoteChar(key); + canonicalNameBuilder.append(PropertyAccessor.PROPERTY_KEY_PREFIX_CHAR); + if (quoteChar != 0) { + canonicalNameBuilder.append(quoteChar); + } + canonicalNameBuilder.append(key); + if (quoteChar != 0) { + canonicalNameBuilder.append(quoteChar); + } + canonicalNameBuilder.append(PropertyAccessor.PROPERTY_KEY_SUFFIX_CHAR); + } + + /** + * Determine the quote character to surround the given key with in the + * canonical name, or {@code 0} if the key needs no quoting. + */ + private static char canonicalQuoteChar(String key) { + int depth = 0; + boolean balanced = true; + boolean containsSingleQuote = false; + boolean containsDoubleQuote = false; + for (int i = 0; i < key.length(); i++) { + switch (key.charAt(i)) { + case PropertyAccessor.PROPERTY_KEY_PREFIX_CHAR -> depth++; + case PropertyAccessor.PROPERTY_KEY_SUFFIX_CHAR -> { + if (--depth < 0) { + balanced = false; + } + } + case '\'' -> containsSingleQuote = true; + case '"' -> containsDoubleQuote = true; + default -> { + // Ordinary key character. + } + } + } + if (balanced && depth == 0 && !containsSingleQuote && !containsDoubleQuote) { + return 0; + } + return (containsSingleQuote && !containsDoubleQuote ? '"' : '\''); + } + } + + + /** + * Options to customize the parsing for {@link PropertyPath}. + */ + public static final class Options { + + /** + * Options with no limit on nesting depth. + */ + public static final Options UNLIMITED = new Options(Integer.MAX_VALUE); + + private final int maxNestedPathDepth; + + private Options(int maxNestedPathDepth) { + this.maxNestedPathDepth = maxNestedPathDepth; + } + + /** + * Create an {@link Options} instance that rejects a path whose nesting + * depth exceeds the given maximum. + * @param maxNestedPathDepth the maximum nesting depth + */ + public static Options withMaxNestedPathDepth(int maxNestedPathDepth) { + Assert.isTrue(maxNestedPathDepth >= 0, "'maxNestedPathDepth' must not be negative"); + return new Options(maxNestedPathDepth); + } + } + + + /** + * Parser for a property path string. + *

Each stage of the enforced grammar is modeled by a separate {@link State}. + */ + private static final class Parser { + + private final String path; + + private final List segments = new ArrayList<>(2); + + private final StringBuilder canonicalName; + + // Offset of the character currently being processed. + private int pos; + + // Offset at which the current segment's name starts. + private int segmentStart; + + // Offset at which the current segment's name ends, or -1 if not yet known. + private int segmentNameEnd = -1; + + // Keys collected for the current segment + private @Nullable List keys; + + // Offset at which the current key starts. + private int keyStart; + + // Bracket nesting depth within the current raw key. + private int depth; + + // The quote character that opened the current quoted key. + private char quoteChar; + + Parser(String path) { + this.path = path; + this.canonicalName = new StringBuilder(path.length()); + } + + PropertyPath parse() { + State state = State.NAME; + for (; this.pos < this.path.length(); this.pos++) { + state = state.process(this.path.charAt(this.pos), this); + } + state.onEof(this); + return new PropertyPath(this.canonicalName.toString(), Collections.unmodifiableList(this.segments)); + } + + private void addKey(String key) { + List keys = this.keys; + if (keys == null) { + keys = new ArrayList<>(2); + this.keys = keys; + } + keys.add(key); + } + + private void endSegment() { + String name = this.path.substring(this.segmentStart, this.segmentNameEnd); + List keys = (this.keys != null ? this.keys : Collections.emptyList()); + if (name.isEmpty() && keys.isEmpty()) { + throw new InvalidPropertyPathException(this.path, + "empty path segment (at position " + this.segmentStart + ")"); + } + if (!this.segments.isEmpty()) { + this.canonicalName.append(PropertyAccessor.NESTED_PROPERTY_SEPARATOR_CHAR); + } + Segment segment = new Segment(name, keys); + this.segments.add(segment); + this.canonicalName.append(segment.toCanonicalName()); + this.keys = null; + this.segmentNameEnd = -1; + } + + private InvalidPropertyPathException error(String reason) { + return new InvalidPropertyPathException(this.path, reason + " (at position " + this.pos + ")"); + } + + private InvalidPropertyPathException unclosedIndex() { + return error("unclosed '" + PropertyAccessor.PROPERTY_KEY_PREFIX_CHAR + "'"); + } + } + + + private enum State { + + // Reading a segment name, before any index of that segment + NAME { + @Override + State process(char ch, Parser parser) { + if (ch == PropertyAccessor.NESTED_PROPERTY_SEPARATOR_CHAR) { + parser.segmentNameEnd = parser.pos; + parser.endSegment(); + parser.segmentStart = parser.pos + 1; + return this; + } + if (ch == PropertyAccessor.PROPERTY_KEY_PREFIX_CHAR) { + parser.segmentNameEnd = parser.pos; + return INDEX_OPEN; + } + if (ch == PropertyAccessor.PROPERTY_KEY_SUFFIX_CHAR) { + throw parser.error("unexpected '" + PropertyAccessor.PROPERTY_KEY_SUFFIX_CHAR + + "' without a matching '" + PropertyAccessor.PROPERTY_KEY_PREFIX_CHAR + "'"); + } + return this; + } + + @Override + void onEof(Parser parser) { + parser.segmentNameEnd = parser.path.length(); + parser.endSegment(); + } + }, + + // Immediately after the "[" that opens an index + INDEX_OPEN { + @Override + State process(char ch, Parser parser) { + if (ch == PropertyAccessor.PROPERTY_KEY_SUFFIX_CHAR) { + // An empty key, as in "map[]". + parser.addKey(""); + return AFTER_INDEX; + } + if (ch == '\'' || ch == '"') { + parser.quoteChar = ch; + parser.keyStart = parser.pos + 1; + return QUOTED_KEY; + } + parser.keyStart = parser.pos; + // A key that itself opens a bracket level, as in "map[[a]]" + parser.depth = (ch == PropertyAccessor.PROPERTY_KEY_PREFIX_CHAR ? 1 : 0); + return RAW_KEY; + } + + @Override + void onEof(Parser parser) { + throw parser.unclosedIndex(); + } + }, + + // Inside an unquoted key, tracking the depth of bracket nesting. + RAW_KEY { + @Override + State process(char ch, Parser parser) { + if (ch == PropertyAccessor.PROPERTY_KEY_PREFIX_CHAR) { + parser.depth++; + return this; + } + if (ch == PropertyAccessor.PROPERTY_KEY_SUFFIX_CHAR) { + if (parser.depth == 0) { + parser.addKey(parser.path.substring(parser.keyStart, parser.pos)); + return AFTER_INDEX; + } + parser.depth--; + return this; + } + if (ch == '\'' || ch == '"') { + throw parser.error("unexpected quote '" + ch + "' in an unquoted key; " + + "quote the whole key to use quote characters within it"); + } + return this; + } + + @Override + void onEof(Parser parser) { + throw parser.unclosedIndex(); + } + }, + + // Inside a quoted key, looking for the closing quote. + QUOTED_KEY { + @Override + State process(char ch, Parser parser) { + if (ch == parser.quoteChar) { + parser.addKey(parser.path.substring(parser.keyStart, parser.pos)); + return QUOTE_CLOSED; + } + return this; + } + + @Override + void onEof(Parser parser) { + throw parser.error("unterminated quote '" + parser.quoteChar + "'"); + } + }, + + // Immediately after a closing quote, only "]" is legal. + QUOTE_CLOSED { + @Override + State process(char ch, Parser parser) { + if (ch != PropertyAccessor.PROPERTY_KEY_SUFFIX_CHAR) { + throw parser.error("unexpected '" + ch + "' after a closing quote; expected '" + + PropertyAccessor.PROPERTY_KEY_SUFFIX_CHAR + "'"); + } + return AFTER_INDEX; + } + + @Override + void onEof(Parser parser) { + throw parser.unclosedIndex(); + } + }, + + // Immediately after an index's "]" , only "." or "[" are legal. + AFTER_INDEX { + @Override + State process(char ch, Parser parser) { + if (ch == PropertyAccessor.NESTED_PROPERTY_SEPARATOR_CHAR) { + parser.endSegment(); + parser.segmentStart = parser.pos + 1; + return NAME; + } + if (ch == PropertyAccessor.PROPERTY_KEY_PREFIX_CHAR) { + return INDEX_OPEN; + } + throw parser.error("unexpected '" + ch + "' after an index; expected '" + + PropertyAccessor.NESTED_PROPERTY_SEPARATOR_CHAR + "', '" + + PropertyAccessor.PROPERTY_KEY_PREFIX_CHAR + "', or the end of the path"); + } + + @Override + void onEof(Parser parser) { + parser.endSegment(); + } + }; + + abstract State process(char ch, Parser parser); + + abstract void onEof(Parser parser); + } + +} diff --git a/spring-beans/src/test/java/org/springframework/beans/PropertyPathBehaviorChangeTests.java b/spring-beans/src/test/java/org/springframework/beans/PropertyPathBehaviorChangeTests.java new file mode 100644 index 00000000000..c6329caf5db --- /dev/null +++ b/spring-beans/src/test/java/org/springframework/beans/PropertyPathBehaviorChangeTests.java @@ -0,0 +1,410 @@ +/* + * Copyright 2002-present the original author or authors. + * + * Licensed 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 + * + * https://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.springframework.beans; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatNoException; + +/** + * Tests that check the behavior changes for a code migration + * from {@link PropertyAccessorUtils} and {@link AbstractNestablePropertyAccessor} + * to {@link PropertyPath}. + *

This test class should be removed after migration. + * + * @author Brian Clozel + */ +class PropertyPathBehaviorChangeTests { + + /** + * The canonical name is the policy-facing form of a path: it is what + * {@code DataBinder.isAllowed} matches against {@code allowedFields} and + * {@code disallowedFields}, and what {@code AbstractPropertyBindingResult} + * reports as the field name. + */ + @Nested + class CanonicalPropertyName { + + @Test + void wellFormedPathsAreCanonicalized() { + assertCanonical("", ""); + assertCanonical("name", "name"); + assertCanonical("person.name", "person.name"); + assertCanonical("map[key1]", "map[key1]"); + assertCanonical("map['key1']", "map[key1]"); + assertCanonical("map[\"key1\"]", "map[key1]"); + assertCanonical("map[key1][key2]", "map[key1][key2]"); + assertCanonical("map['key1'].name", "map[key1].name"); + assertCanonical("map[key[0]]", "map[key[0]]"); + assertCanonical("map['key[0]']", "map[key[0]]"); + assertCanonical("map[]", "map[]"); + assertCanonical("map['']", "map[]"); + assertCanonical("[user]", "[user]"); + } + + @Test // Malformed paths are passed through unchanged, not rejected. + void malformedPathsArePassedThrough() { + assertCanonical("map[key1]other", "map[key1]other"); + assertCanonical("map[key1]other.name", "map[key1]other.name"); + assertCanonical("map[key1]IGNORED[key2]", "map[key1]IGNORED[key2]"); + assertCanonical(".name", ".name"); + assertCanonical("person.", "person."); + assertCanonical("person..name", "person..name"); + assertCanonical("map[key1", "map[key1"); + assertCanonical("map]", "map]"); + assertCanonical("address.].city", "address.].city"); + } + + @Test // An unterminated quote falls back to treating the quote as literal content. + void unterminatedQuotesAreTreatedAsLiteralContent() { + assertCanonical("map['key1]", "map['key1]"); + assertCanonical("map[\"key1]", "map[\"key1]"); + assertCanonical("map[']", "map[']"); + assertCanonical("map[\"]", "map[\"]"); + } + + @Test // Quotes are stripped whenever the key merely starts and ends with one. + void outerQuotesAreStrippedWithoutRegardToNesting() { + assertCanonical("map['a'b']", "map[a'b]"); + assertCanonical("map[a'b]", "map[a'b]"); + assertCanonical("map['a]b']", "map['a]b']"); + } + + private void assertCanonical(String path, String expected) { + assertThat(PropertyAccessorUtils.canonicalPropertyName(path)) + .as("canonicalPropertyName(\"%s\")", path) + .isEqualTo(expected); + } + } + + + /** + * {@code getPropertyName} only strips keys when the path happens to end + * with {@code ]}, which makes it inconsistent between otherwise similar + * malformed paths. + */ + @Nested + class GetPropertyName { + + @Test + void keysAreStrippedOnlyWhenThePathEndsWithAKey() { + assertThat(PropertyAccessorUtils.getPropertyName("map[key1]")).isEqualTo("map"); + assertThat(PropertyAccessorUtils.getPropertyName("map[key1][key2]")).isEqualTo("map"); + assertThat(PropertyAccessorUtils.getPropertyName("[user]")).isEmpty(); + + // Not stripped: the path does not end with ']'. + assertThat(PropertyAccessorUtils.getPropertyName("map[key1].name")).isEqualTo("map[key1].name"); + assertThat(PropertyAccessorUtils.getPropertyName("map[key1]other")).isEqualTo("map[key1]other"); + + // Stripped, even though the path is malformed, because it does end with ']'. + assertThat(PropertyAccessorUtils.getPropertyName("map[key1]IGNORED[key2]")).isEqualTo("map"); + } + } + + + /** + * The access-facing resolution: which property the accessor actually reads + * or writes for a given path. + */ + @Nested + class AccessorResolution { + + @Test + void wellFormedPathsResolveAsExpected() { + assertThat(bind("name")).containsEntry("name", "V"); + assertThat(bind("nested.name")).containsEntry("nested.name", "V"); + assertThat(bind("map[key1]")).containsEntry("map", "{key1=V}"); + assertThat(bind("map['key1']")).containsEntry("map", "{key1=V}"); + assertThat(bind("map[\"key1\"]")).containsEntry("map", "{key1=V}"); + assertThat(bind("map[key[0]]")).containsEntry("map", "{key[0]=V}"); + assertThat(bind("map['key[0]']")).containsEntry("map", "{key[0]=V}"); + assertThat(bind("map[]")).containsEntry("map", "{=V}"); + assertThat(bind("map['']")).containsEntry("map", "{=V}"); + assertThat(bind("list[0]")).containsEntry("list", "[V]"); + assertThat(bind("nestedMap[a][b]")).containsEntry("nestedMap", "{a={b=V}}"); + } + + @ParameterizedTest // gh-36999 + @ValueSource(strings = {"map[key1", "map]", "nested.].name", "nested.[.name", + "nested.[[.name", "nested.]].name", "nested.][.name"}) + void unbalancedBracketsAreRejectedOnWrite(String path) { + assertThatExceptionOfType(NotWritablePropertyException.class) + .isThrownBy(() -> bind(path)) + .withMessageContaining("Nested property in path '" + path + "' does not exist"); + } + + @ParameterizedTest // gh-36999 + @ValueSource(strings = {"map[key1", "map]", "nested.].name", "nested.[.name"}) + void unbalancedBracketsAreRejectedOnRead(String path) { + BeanWrapperImpl accessor = new BeanWrapperImpl(new Target()); + + assertThatExceptionOfType(NotReadablePropertyException.class) + .isThrownBy(() -> accessor.getPropertyValue(path)) + .withMessageEndingWith("contains unbalanced brackets"); + } + + @ParameterizedTest + @ValueSource(strings = {".name", "person.", "nested..name"}) + void emptySegmentsAreRejectedOnWrite(String path) { + assertThatExceptionOfType(NotWritablePropertyException.class) + .isThrownBy(() -> bind(path)); + } + + /** + * The behavior that matters most for {@code DataBinder}: because + * {@code ignoreUnknownFields} defaults to {@code true} and + * {@code AbstractPropertyAccessor.setPropertyValues} swallows + * {@code NotWritablePropertyException} in that mode, a malformed path + * is currently dropped without any error being recorded. + */ + @ParameterizedTest + @ValueSource(strings = {"map[key1", "map]", "nested.].name", ".name", "person."}) + void malformedPathsAreSilentlyDroppedWhenIgnoringUnknownFields(String path) { + Target target = new Target(); + BeanWrapperImpl accessor = new BeanWrapperImpl(target); + MutablePropertyValues pvs = new MutablePropertyValues(Map.of(path, "V")); + + assertThatNoException().isThrownBy(() -> accessor.setPropertyValues(pvs, true, true)); + + assertThat(target.getMap()).isEmpty(); + assertThat(target.getName()).isEmpty(); + } + } + + + /** + * The rows where {@link PropertyPath} knowingly differs from the current + * implementation. Each test asserts the old behavior and the new behavior + * together, so that the diff is explicit rather than discovered later. + */ + @Nested + class IntentionalDivergences { + + /** + * The primary target of the refactor: trailing text after an index is + * dropped by the accessor but kept by {@code canonicalPropertyName}. + */ + @Test + void trailingTextAfterAnIndexIsDroppedByTheAccessorButKeptInTheCanonicalName() { + assertThat(PropertyAccessorUtils.canonicalPropertyName("map[key1]other")) + .isEqualTo("map[key1]other"); + assertThat(bind("map[key1]other")).containsEntry("map", "{key1=V}"); + + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse("map[key1]other")); + } + + @Test + void interleavedTextBetweenIndexesIsDroppedByTheAccessor() { + assertThat(PropertyAccessorUtils.canonicalPropertyName("nestedMap[a]X[b]")) + .isEqualTo("nestedMap[a]X[b]"); + assertThat(bind("nestedMap[a]X[b]")).containsEntry("nestedMap", "{a={b=V}}"); + + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse("nestedMap[a]X[b]")); + } + + @Test + void trailingTextBeforeANestedSeparatorIsDroppedByTheAccessor() { + assertThatExceptionOfType(NotWritablePropertyException.class) + .isThrownBy(() -> bind("map[key1]other.name")) + // The accessor resolved 'map[key1].name', dropping 'other'. + .withMessageContaining("map[key1].name"); + + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse("map[key1]other.name")); + } + + /** + * A key consisting of nothing but a quote character is + * deliberately supported today: commit {@code d3152c11c7} + * ("Consistently expose map key quotes", gh-36765) added + * {@code map.put("'", …)} and {@code map.put("\"", …)} to the shared + * {@code IndexedTestBean} fixture and asserted + * {@code getPropertyValue("map['].name")} in + * {@link AbstractPropertyAccessorTests}. The lenient fallback it + * relies on goes back further, to SPR-14293 ({@code cf0a0cd5d8}), + * where treating an unterminated quote as literal content was the + * chosen remedy for a {@code StringIndexOutOfBoundsException}. + *

The strict quote grammar knowingly reverts that: an opened quote + * must be closed. This is the one intentional divergence that removes + * a documented capability rather than an accident, so it needs + * explicit sign-off from the Beans/Core owners before the migration + * of {@link PropertyAccessorUtils} lands. + *

Auditing every property path literal in the accessor test suites + * found exactly three affected by the strict grammar, all from this + * lineage: {@code map['].name}, {@code map["].name} and {@code [']}. + */ + @ParameterizedTest // gh-36765 + @ValueSource(strings = {"map[']", "map[\"]"}) + void deliberatelySupportedQuoteOnlyKeyNoLongerBinds(String path) { + assertThatNoException().isThrownBy(() -> bind(path)); + + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse(path)) + .withMessageContaining("unterminated quote"); + } + + /** + * These shapes also bind today, but unlike + * {@link #deliberatelySupportedQuoteOnlyKeyNoLongerBinds} they are + * asserted nowhere in the test suite: they are incidental consequences + * of the same SPR-14293 leniency rather than intended behavior. Under + * the strict grammar a raw key may not contain quote characters at all, + * and a closing quote must be followed immediately by {@code ]}. + */ + @ParameterizedTest + @ValueSource(strings = {"map[don't]", "map[a'b]", "map['key1]", "map[\"key1]", "map['a'b']"}) + void incidentallyAcceptedQuoteCharactersInKeysNoLongerBind(String path) { + assertThatNoException().isThrownBy(() -> bind(path)); + + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse(path)); + } + + /** + * Today's canonical name is not a fixed point for nested quotes, so + * canonicalizing twice yields a third, different key. Nothing exploits + * this because {@code DataBinder} canonicalizes patterns once and + * fields once, but it is the fragility that the strict grammar removes: + * these inputs no longer parse, and for everything that does parse + * {@code PropertyPath.canonicalName()} is idempotent. + * @see PropertyPathTests#canonicalNameIdempotent(String) () + */ + @Test + void canonicalNameIsNotIdempotentForNestedQuotes() { + assertThat(PropertyAccessorUtils.canonicalPropertyName("map[''a'']")).isEqualTo("map['a']"); + assertThat(PropertyAccessorUtils.canonicalPropertyName("map['a']")).isEqualTo("map[a]"); + + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse("map[''a'']")); + } + + /** + * Conversely, a quoted key is now opaque, so a key containing an + * unbalanced {@code ]} becomes legal where it is rejected today. + */ + @Test + void quotedKeyWithUnbalancedBracketBecomesLegal() { + assertThatExceptionOfType(NotWritablePropertyException.class) + .isThrownBy(() -> bind("map['a]b']")); + + assertThat(PropertyPath.parse("map['a]b']").segments()) + .containsExactly(new PropertyPath.Segment("map", List.of("a]b"))); + } + } + + + /** + * Bind {@code "V"} to the given path and return a description of the + * resulting target state, keyed by property name. + */ + private static Map bind(String path) { + Target target = new Target(); + BeanWrapperImpl accessor = new BeanWrapperImpl(target); + accessor.setAutoGrowNestedPaths(true); + accessor.setPropertyValue(path, "V"); + return target.describe(); + } + + + @SuppressWarnings("unused") + public static class Target { + + private String name = ""; + + private Target nested; + + private Map map = new LinkedHashMap<>(); + + private Map> nestedMap = new LinkedHashMap<>(); + + private List list = new ArrayList<>(); + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + + public Target getNested() { + return this.nested; + } + + public void setNested(Target nested) { + this.nested = nested; + } + + public Map getMap() { + return this.map; + } + + public void setMap(Map map) { + this.map = map; + } + + public Map> getNestedMap() { + return this.nestedMap; + } + + public void setNestedMap(Map> nestedMap) { + this.nestedMap = nestedMap; + } + + public List getList() { + return this.list; + } + + public void setList(List list) { + this.list = list; + } + + Map describe() { + Map description = new HashMap<>(); + if (!this.name.isEmpty()) { + description.put("name", this.name); + } + if (!this.map.isEmpty()) { + description.put("map", this.map.toString()); + } + if (!this.nestedMap.isEmpty()) { + description.put("nestedMap", this.nestedMap.toString()); + } + if (!this.list.isEmpty()) { + description.put("list", this.list.toString()); + } + if (this.nested != null) { + this.nested.describe().forEach((key, value) -> description.put("nested." + key, value)); + } + return description; + } + } + +} diff --git a/spring-beans/src/test/java/org/springframework/beans/PropertyPathTests.java b/spring-beans/src/test/java/org/springframework/beans/PropertyPathTests.java new file mode 100644 index 00000000000..b6b8677bfe3 --- /dev/null +++ b/spring-beans/src/test/java/org/springframework/beans/PropertyPathTests.java @@ -0,0 +1,302 @@ +/* + * Copyright 2002-present the original author or authors. + * + * Licensed 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 + * + * https://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.springframework.beans; + +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import org.springframework.beans.PropertyPath.Segment; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +/** + * Tests for {@link PropertyPath}. + */ +class PropertyPathTests { + + @Test + void canonicalNameOfWellFormedPath() { + assertThat(canonicalName("name")).isEqualTo("name"); + assertThat(canonicalName("person.name")).isEqualTo("person.name"); + assertThat(canonicalName("person.addresses[1].city")).isEqualTo("person.addresses[1].city"); + assertThat(canonicalName("map[key1]")).isEqualTo("map[key1]"); + assertThat(canonicalName("map['key1']")).isEqualTo("map[key1]"); + assertThat(canonicalName("map[\"key1\"]")).isEqualTo("map[key1]"); + assertThat(canonicalName("map['key1'].name")).isEqualTo("map[key1].name"); + assertThat(canonicalName("map[key1][key2]")).isEqualTo("map[key1][key2]"); + assertThat(canonicalName("map['key1'][\"key2\"]")).isEqualTo("map[key1][key2]"); + assertThat(canonicalName("map[key[0]]")).isEqualTo("map[key[0]]"); + assertThat(canonicalName("map['key[0]']")).isEqualTo("map[key[0]]"); + assertThat(canonicalName("map['key[0]'].name")).isEqualTo("map[key[0]].name"); + assertThat(canonicalName("users['admin[0]']")).isEqualTo("users[admin[0]]"); + assertThat(canonicalName("map[]")).isEqualTo("map[]"); + assertThat(canonicalName("map['']")).isEqualTo("map[]"); + assertThat(canonicalName("map[\"\"]")).isEqualTo("map[]"); + assertThat(canonicalName("map[my.key]")).isEqualTo("map[my.key]"); + assertThat(canonicalName("map[[a]]")).isEqualTo("map[[a]]"); + assertThat(canonicalName("[user]")).isEqualTo("[user]"); + assertThat(canonicalName("[user].name")).isEqualTo("[user].name"); + } + + @Test + void canonicalNameKeepsQuotesWhenRequired() { + assertThat(canonicalName("map['a]b']")).isEqualTo("map['a]b']"); + assertThat(canonicalName("map['a[b']")).isEqualTo("map['a[b']"); + assertThat(canonicalName("map[\"a'b\"]")).isEqualTo("map[\"a'b\"]"); + assertThat(canonicalName("map['a\"b']")).isEqualTo("map['a\"b']"); + } + + @Test + void parseEmptyPath() { + PropertyPath path = PropertyPath.parse(""); + + assertThat(path.canonicalName()).isEmpty(); + assertThat(path.segments()).isEmpty(); + } + + @Test + void parseSimplePath() { + assertThat(PropertyPath.parse("name").segments()) + .containsExactly(new Segment("name", List.of())); + } + + @Test + void parseNestedPath() { + assertThat(PropertyPath.parse("person.addresses[1].city").segments()) + .containsExactly( + new Segment("person", List.of()), + new Segment("addresses", List.of("1")), + new Segment("city", List.of())); + } + + @Test + void parseMultipleIndexesInOneSegment() { + assertThat(PropertyPath.parse("map[key1][key2]").segments()) + .containsExactly(new Segment("map", List.of("key1", "key2"))); + } + + @Test + void parseRootLevelIndexedAccess() { + assertThat(PropertyPath.parse("[user]").segments()) + .containsExactly(new Segment("", List.of("user"))); + } + + @Test + void parseStripsQuotesFromKeys() { + assertThat(PropertyPath.parse("map['key1'][\"key2\"]").segments()) + .containsExactly(new Segment("map", List.of("key1", "key2"))); + } + + @Test + void parseKeepsDotsInsideKeysOpaque() { + assertThat(PropertyPath.parse("map[my.key].name").segments()) + .containsExactly( + new Segment("map", List.of("my.key")), + new Segment("name", List.of())); + } + + @Test + void parseKeepsNestedBracketsInRawKey() { + assertThat(PropertyPath.parse("map[key[0]]").segments()) + .containsExactly(new Segment("map", List.of("key[0]"))); + } + + @Test + void parseKeepsUnbalancedBracketInQuotedKey() { + assertThat(PropertyPath.parse("map['a]b']").segments()) + .containsExactly(new Segment("map", List.of("a]b"))); + } + + @Test + void parseEmptyKey() { + assertThat(PropertyPath.parse("map[]").segments()) + .containsExactly(new Segment("map", List.of(""))); + } + + @ParameterizedTest + @ValueSource(strings = {"", "name", "person.name", "person.addresses[1].city", "map[key1]", + "map['key1']", "map[\"key1\"]", "map[key1][key2]", "map[key[0]]", "map['key[0]']", + "map[]", "map['']", "map[my.key]", "map[[a]]", "[user]", "[user].name", + "map['a]b']", "map['a[b']", "map[\"a'b\"]", "map['a\"b']", "map['']['a]b']"}) + void canonicalNameIdempotent(String path) { + PropertyPath parsed = PropertyPath.parse(path); + + PropertyPath reparsed = PropertyPath.parse(parsed.canonicalName()); + + assertThat(reparsed.segments()).isEqualTo(parsed.segments()); + assertThat(reparsed.canonicalName()).isEqualTo(parsed.canonicalName()); + assertThat(reparsed).isEqualTo(parsed); + } + + @ParameterizedTest + @ValueSource(strings = {"name", "person.name", "person.addresses[1].city", "person.map[key1]", + "person.map['key1']", "person.map[\"key1\"]", "map[key1].map[key2]", "map['key[0]']", + "person.map['a]b']", "person.map['a[b']", "person.map[\"a'b\"]", "person.map['a\"b']", "person.map['']['a]b']"}) + void canonicalPathAndSegmentsAreEquivalent(String path) { + PropertyPath parsed = PropertyPath.parse(path); + assertThat(parsed.segments().stream() + .map(Segment::toCanonicalName) + .reduce((a, b) -> a + "." + b)) + .hasValue(parsed.canonicalName()); + } + + @Test + void segmentsCannotBeModified() { + PropertyPath path = PropertyPath.parse("map[key1]"); + + assertThat(path.segments()).hasSize(1); + assertThatExceptionOfType(UnsupportedOperationException.class) + .isThrownBy(() -> path.segments().clear()); + assertThatExceptionOfType(UnsupportedOperationException.class) + .isThrownBy(() -> path.segments().get(0).keys().clear()); + } + + @Test + void equalsAndHashCodeUseCanonicalName() { + assertThat(PropertyPath.parse("map['key1'].name")) + .isEqualTo(PropertyPath.parse("map[key1].name")) + .hasSameHashCodeAs(PropertyPath.parse("map[key1].name")); + assertThat(PropertyPath.parse("map[key1]")).isNotEqualTo(PropertyPath.parse("map[key2]")); + } + + @Test + void toStringReturnsCanonicalName() { + assertThat(PropertyPath.parse("map['key1'].name")).hasToString("map[key1].name"); + } + + @Test + void parseRejectsNull() { + assertThatIllegalArgumentException().isThrownBy(() -> PropertyPath.parse(null)); + } + + @ParameterizedTest + @ValueSource(strings = {"map[key1]other", "map[key1]other.name", "map[key1]IGNORED[key2]", + "options[priority]X", "options[security]IGNORED[role]", "map[key1]]", "[user]x"}) + void parseRejectsTextAfterIndex(String path) { + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse(path)) + .withMessageContaining("after an index") + .satisfies(ex -> assertThat(ex.getPropertyPath()).isEqualTo(path)); + } + + @ParameterizedTest + @ValueSource(strings = {"map[key1", "map[", "map[[a]", "map[a][", "map['a'"}) + void parseRejectsUnclosedIndex(String path) { + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse(path)) + .withMessageContaining("unclosed '['"); + } + + @ParameterizedTest + @ValueSource(strings = {"map]", "map]name", "publication.].published", "address.].city", "]"}) + void parseRejectsUnmatchedIndexSuffix(String path) { + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse(path)) + .withMessageContaining("without a matching '['"); + } + + @ParameterizedTest + @ValueSource(strings = {".name", "person.", "person..name", ".", "..", "map[key1]."}) + void parseRejectsEmptySegment(String path) { + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse(path)) + .withMessageContaining("empty path segment"); + } + + @ParameterizedTest + @ValueSource(strings = {"map[']", "map[\"]", "map['key1]", "map[\"key1]", "map['a"}) + void parseRejectsUnterminatedQuote(String path) { + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse(path)) + .withMessageContaining("unterminated quote"); + } + + @ParameterizedTest + @ValueSource(strings = {"map[don't]", "map[a'b]", "map[a\"b]"}) + void parseRejectsQuoteInRawKey(String path) { + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse(path)) + .withMessageContaining("in an unquoted key"); + } + + @ParameterizedTest + @ValueSource(strings = {"map['a'b']", "map['a'b]", "map[\"a\"b\"]"}) + void parseRejectsTextAfterClosingQuote(String path) { + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse(path)) + .withMessageContaining("after a closing quote"); + } + + @Test + void invalidPropertyPathExceptionIsAPropertyAccessException() { + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> PropertyPath.parse("map[key1]other")) + .isInstanceOf(PropertyAccessException.class) + .withMessageContaining("Invalid property path 'map[key1]other'") + .satisfies(ex -> { + assertThat(ex.getErrorCode()).isEqualTo(InvalidPropertyPathException.ERROR_CODE); + assertThat(ex.getPropertyPath()).isEqualTo("map[key1]other"); + }); + } + + @Test + void failsWhenExceedsNestingDepth() { + assertThatExceptionOfType(InvalidPropertyPathException.class) + .isThrownBy(() -> + PropertyPath.parse("one.two.three[1].four", PropertyPath.Options.withMaxNestedPathDepth(2))) + .withMessageContaining("nesting depth exceeds the maximum of 2"); + } + + @Test + void subPathFromZeroReturnsSamePath() { + PropertyPath path = PropertyPath.parse("address.country.name"); + assertThat(path.subPath(0)).isSameAs(path); + } + + @Test + void subPathDropsLeadingSegments() { + PropertyPath subPath = PropertyPath.parse("address.country[0].name").subPath(1); + assertThat(subPath.canonicalName()).isEqualTo("country[0].name"); + assertThat(subPath.segments()).containsExactly( + new Segment("country", List.of("0")), + new Segment("name", List.of())); + } + + @Test + void subPathAtLastIndexIsEmpty() { + PropertyPath subPath = PropertyPath.parse("address.name").subPath(2); + assertThat(subPath.canonicalName()).isEmpty(); + assertThat(subPath.segments()).isEmpty(); + } + + @Test + void subPathRejectsOutOfBoundsIndex() { + PropertyPath path = PropertyPath.parse("address.name"); + assertThatIllegalArgumentException().isThrownBy(() -> path.subPath(3)); + } + + + private static String canonicalName(String path) { + return PropertyPath.parse(path).canonicalName(); + } + +}