diff --git a/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/main/java/org/springframework/data/rest/tests/mongodb/Address.java b/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/main/java/org/springframework/data/rest/tests/mongodb/Address.java index 4ed3a300ad..b1428ac319 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/main/java/org/springframework/data/rest/tests/mongodb/Address.java +++ b/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/main/java/org/springframework/data/rest/tests/mongodb/Address.java @@ -24,4 +24,5 @@ public class Address { public String street; public @JsonProperty(required = true) String zipCode; + public @JsonProperty("renamed") String aliased; } diff --git a/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolverUnitTests.java b/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolverUnitTests.java index 7e6a3843eb..c2bb2fb541 100755 --- a/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolverUnitTests.java +++ b/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolverUnitTests.java @@ -64,6 +64,7 @@ * Unit tests for {@link QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver}. * * @author Oliver Gierke + * @author Lovro Vrlec */ @ExtendWith(MockitoExtension.class) class QuerydslAwareRootResourceInformationHandlerMethodArgumentResolverUnitTests { @@ -180,6 +181,105 @@ void translatesJacksonRenamedPropertyToPersistentPropertyName() { assertThat(forwarded.get("aliased")).containsExactly("value"); } + @Test // GH-2579 + void forwardsNestedPropertyPathsToQuerydsl() { + + Object repository = mock(QuerydslUserRepository.class); + when(repositories.getRepositoryFor(User.class)).thenReturn(Optional.of(repository)); + + Map parameters = Map.of("address.street", new String[] { "value" }); + + ArgumentCaptor> captor = ArgumentCaptor.captor(); + when(builder.getPredicate(any(), captor.capture(), any())).thenReturn(mock(Predicate.class)); + + resolver.postProcess(parameter, invoker, User.class, parameters); + + // Neither User.address nor Address.street is hidden from serialization, so the nested path has to reach Querydsl, + // which resolves dotted paths itself. Dropping it would silently turn a filtered lookup into an unfiltered one. + assertThat(captor.getValue().get("address.street")).containsExactly("value"); + } + + @Test // GH-2579 + void doesNotExposeJsonIgnoredPropertiesNestedInAssociations() { + + Object repository = mock(QuerydslUserRepository.class); + when(repositories.getRepositoryFor(User.class)).thenReturn(Optional.of(repository)); + + Map parameters = Map.of("manager.ignored", new String[] { "candidate-value" }); + + ArgumentCaptor> captor = ArgumentCaptor.captor(); + when(builder.getPredicate(any(), captor.capture(), any())).thenReturn(mock(Predicate.class)); + + resolver.postProcess(parameter, invoker, User.class, parameters); + + // User.manager is visible, but User.manager.ignored is @JsonIgnore-annotated: the guarantee established for + // top-level properties has to hold for every segment of a nested path as well. + assertThat(captor.getValue()).doesNotContainKey("manager.ignored"); + } + + @Test // GH-2579 + void translatesJacksonRenamedPropertyNestedInAssociation() { + + Object repository = mock(QuerydslUserRepository.class); + when(repositories.getRepositoryFor(User.class)).thenReturn(Optional.of(repository)); + + // Address.aliased is exposed as "renamed" via @JsonProperty("renamed"). The translation applied to top-level + // properties has to be applied to every segment: the public alias reaches Querydsl under the domain property + // name, the bare Java field name is not accepted. + Map parameters = Map.of( + "address.renamed", new String[] { "value" }, + "address.aliased", new String[] { "ignored" }); + + ArgumentCaptor> captor = ArgumentCaptor.captor(); + when(builder.getPredicate(any(), captor.capture(), any())).thenReturn(mock(Predicate.class)); + + resolver.postProcess(parameter, invoker, User.class, parameters); + + MultiValueMap forwarded = captor.getValue(); + assertThat(forwarded).doesNotContainKey("address.renamed"); + assertThat(forwarded.get("address.aliased")).containsExactly("value"); + } + + @Test // GH-2579 + void rejectsNestedPathsBeneathTypesWithoutJacksonPropertyMapping() { + + Object repository = mock(QuerydslUserRepository.class); + when(repositories.getRepositoryFor(User.class)).thenReturn(Optional.of(repository)); + + Map parameters = Map.of("firstname.length", new String[] { "value" }); + + ArgumentCaptor> captor = ArgumentCaptor.captor(); + when(builder.getPredicate(any(), captor.capture(), any())).thenReturn(mock(Predicate.class)); + + resolver.postProcess(parameter, invoker, User.class, parameters); + + // User.firstname is a String, i.e. not a persistent entity, so there are no MappedJacksonProperties to establish + // the visibility of a segment beneath it. The whole parameter is rejected rather than forwarded unchecked. + assertThat(captor.getValue()).isEmpty(); + } + + @Test // GH-2579 + void dropsParameterNamesWithBlankSegments() { + + Object repository = mock(QuerydslUserRepository.class); + when(repositories.getRepositoryFor(User.class)).thenReturn(Optional.of(repository)); + + Map parameters = Map.of( + "", new String[] { "value" }, + ".street", new String[] { "value" }, + "address..street", new String[] { "value" }, + "address.", new String[] { "value" }); + + ArgumentCaptor> captor = ArgumentCaptor.captor(); + when(builder.getPredicate(any(), captor.capture(), any())).thenReturn(mock(Predicate.class)); + + resolver.postProcess(parameter, invoker, User.class, parameters); + + // A blank segment cannot refer to a property. Being client-controlled input, such parameters are dropped like + // any other unknown key instead of being rejected with an exception. + assertThat(captor.getValue()).isEmpty(); + } + interface QuerydslUserRepository extends QuerydslPredicateExecutor {} interface QuerydslProfileRepository extends QuerydslPredicateExecutor {} diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver.java index c342408cf8..b7f184b7c8 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver.java @@ -21,6 +21,7 @@ import java.util.Map.Entry; import java.util.Optional; import java.util.function.Function; +import java.util.regex.Pattern; import org.jspecify.annotations.Nullable; @@ -39,6 +40,7 @@ import org.springframework.data.rest.webmvc.RootResourceInformation; import org.springframework.data.rest.webmvc.json.MappedJacksonProperties; import org.springframework.data.util.Pair; +import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.method.support.HandlerMethodArgumentResolver; @@ -51,11 +53,14 @@ * * @author Oliver Gierke * @author Mark Paluch + * @author Lovro Vrlec * @since 2.4 */ class QuerydslAwareRootResourceInformationHandlerMethodArgumentResolver extends RootResourceInformationHandlerMethodArgumentResolver { + private static final Pattern NESTED_PATH_SEPARATOR = Pattern.compile("\\."); + private final Repositories repositories; private final QuerydslPredicateBuilder predicateBuilder; private final QuerydslBindingsFactory factory; @@ -133,16 +138,66 @@ private Map filterByJacksonVisibility(Class domainType, Map for (Entry entry : parameters.entrySet()) { - PersistentProperty property = properties.getPersistentProperty(entry.getKey()); + String path = resolvePath(properties, entry.getKey()); - if (property != null) { - filtered.put(property.getName(), entry.getValue()); + if (path != null) { + filtered.put(path, entry.getValue()); } } return filtered; } + /** + * Resolves the given request parameter name into the property path Querydsl operates on, applying the visibility + * rules of {@link #filterByJacksonVisibility(Class, Map)} to every segment of a nested path, as Querydsl supports + * filtering on nested properties (e.g. {@code address.street}). Returns {@literal null} if a segment is blank or + * does not refer to a property Jackson exposes, so that a hidden property cannot be reached through an association + * either, or if the type owning a segment is not backed by a {@link MappedJacksonProperties} instance, in which + * case visibility cannot be established. + * + * @param properties the Jackson property mapping of the type the leading segment is resolved against, must not be + * {@literal null}. + * @param parameterName must not be {@literal null}. + * @return the translated property path or {@literal null} if the parameter must not be forwarded to Querydsl. + */ + private @Nullable String resolvePath(MappedJacksonProperties properties, String parameterName) { + + Assert.notNull(properties, "MappedJacksonProperties must not be null"); + Assert.notNull(parameterName, "Parameter name must not be null"); + + StringBuilder path = new StringBuilder(parameterName.length()); + PersistentProperty property = null; + + for (String segment : NESTED_PATH_SEPARATOR.split(parameterName, -1)) { + + // The leading segment is resolved against the given properties, every further one against the type of the + // previously resolved segment, which is only backed by a MappedJacksonProperties instance if it is a + // persistent entity itself. + MappedJacksonProperties owner = property == null // + ? properties // + : jacksonPropertiesLookup.apply(property.getActualType()); + + if (owner == null || segment.isBlank()) { + return null; + } + + property = owner.getPersistentProperty(segment); + + if (property == null) { + return null; + } + + if (!path.isEmpty()) { + path.append('.'); + } + + path.append(property.getName()); + } + + return path.toString(); + } + @SuppressWarnings("unchecked") private static RepositoryInvoker getQuerydslAdapter(RepositoryInvoker invoker, QuerydslPredicateExecutor repository, Predicate predicate) {