From a13f51b49ea1b2ec0dfde3898f0ab9ac0885bd1f Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 1 Mar 2017 21:30:37 +0100 Subject: [PATCH] DATAREST-1003 - Entity resources don't answer arbitrary JSON requests. Previously, when a request was sending an Accept header of some arbitrary *+json, the request was routed through the controllers and might have ended up producing a PersistentEntityResource that was then mapped using an uncustomized Jackson ObjectMapper. That has caused a huge JSON object to be unfolded which is highly undesirable. We now only answer JSON requests to repository resources that contain an Accept header with any of the explicit JSON media types we got registered. Tweaked the setup of the ExceptionHandlerExceptionResolver to not expose a bean in the first place but rather use the Spring MVC provided callbacks to register custom ones. We also now make sure MVC is bootstrapped property for integration tests through the inclusion of DelegatingWebMvcConfiguration. --- .../rest/tests/AbstractWebIntegrationTests.java | 3 ++- .../data/rest/tests/CommonWebTests.java | 13 +++++++++++++ .../rest/webmvc/RepositoryRestHandlerMapping.java | 9 ++------- .../config/RepositoryRestMvcConfiguration.java | 14 +++++++------- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractWebIntegrationTests.java b/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractWebIntegrationTests.java index cba3785b8..27ed9089d 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractWebIntegrationTests.java +++ b/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractWebIntegrationTests.java @@ -46,6 +46,7 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; import org.springframework.web.context.WebApplicationContext; +import org.springframework.web.servlet.config.annotation.DelegatingWebMvcConfiguration; import com.jayway.jsonpath.InvalidPathException; import com.jayway.jsonpath.JsonPath; @@ -61,7 +62,7 @@ import com.jayway.jsonpath.JsonPath; */ @RunWith(SpringJUnit4ClassRunner.class) @WebAppConfiguration -@ContextConfiguration(classes = RepositoryRestMvcConfiguration.class) +@ContextConfiguration(classes = { RepositoryRestMvcConfiguration.class, DelegatingWebMvcConfiguration.class }) public abstract class AbstractWebIntegrationTests { private static final String CONTENT_LINK_JSONPATH = "$._embedded.._links.%s.href"; diff --git a/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/CommonWebTests.java b/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/CommonWebTests.java index 406dc7d5c..7a0aaa84a 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/CommonWebTests.java +++ b/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/CommonWebTests.java @@ -239,4 +239,17 @@ public abstract class CommonWebTests extends AbstractWebIntegrationTests { // PATCH to non-existing resource mvc.perform(patch(URI.create(uri))).andExpect(status().isNotFound()); } + + @Test // DATAREST-1003 + public void rejectsUnsupportedAcceptTypeForResources() throws Exception { + + for (String string : expectedRootLinkRels()) { + + Link link = client.discoverUnique(string); + + mvc.perform(get(link.expand().getHref())// + .accept(MediaType.valueOf("application/schema+json")))// + .andExpect(status().isNotAcceptable()); + } + } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java index bee82255b..87d1fce7d 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java @@ -36,7 +36,6 @@ import org.springframework.data.rest.core.mapping.ResourceMetadata; import org.springframework.data.rest.webmvc.support.JpaHelper; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; -import org.springframework.http.converter.json.AbstractJackson2HttpMessageConverter; import org.springframework.orm.jpa.support.OpenEntityManagerInViewInterceptor; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; @@ -62,9 +61,6 @@ import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandl */ public class RepositoryRestHandlerMapping extends BasePathAwareHandlerMapping { - private static final MediaType EVERYTHING_JSON_MEDIA_TYPE = new MediaType("application", "*+json", - AbstractJackson2HttpMessageConverter.DEFAULT_CHARSET); - private final ResourceMappings mappings; private final RepositoryRestConfiguration configuration; private final Repositories repositories; @@ -102,8 +98,8 @@ public class RepositoryRestHandlerMapping extends BasePathAwareHandlerMapping { this.mappings = mappings; this.configuration = config; this.repositories = repositories; - this.corsConfigurationAccessor = new RepositoryCorsConfigurationAccessor(mappings, - NoOpStringValueResolver.INSTANCE, repositories); + this.corsConfigurationAccessor = new RepositoryCorsConfigurationAccessor(mappings, NoOpStringValueResolver.INSTANCE, + repositories); } /** @@ -195,7 +191,6 @@ public class RepositoryRestHandlerMapping extends BasePathAwareHandlerMapping { HashSet mediaTypes = new LinkedHashSet(); mediaTypes.add(configuration.getDefaultMediaType().toString()); mediaTypes.add(MediaType.APPLICATION_JSON_VALUE); - mediaTypes.add(EVERYTHING_JSON_MEDIA_TYPE.toString()); return new ProducesRequestCondition(mediaTypes.toArray(new String[mediaTypes.size()])); } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java index db67e9ca4..7a2e3698c 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java @@ -142,6 +142,7 @@ import org.springframework.util.ClassUtils; import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.method.support.HandlerMethodArgumentResolver; +import org.springframework.web.servlet.HandlerExceptionResolver; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.config.annotation.ResourceHandlerRegistry; import org.springframework.web.servlet.mvc.HttpRequestHandlerAdapter; @@ -659,13 +660,12 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon return new DefaultExcerptProjector(projectionFactory, resourceMappings()); } - /** - * Bean for looking up methods annotated with {@link org.springframework.web.bind.annotation.ExceptionHandler}. - * - * @return + /* + * (non-Javadoc) + * @see org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter#extendHandlerExceptionResolvers(java.util.List) */ - @Bean - public ExceptionHandlerExceptionResolver exceptionHandlerExceptionResolver() { + @Override + public void extendHandlerExceptionResolvers(List exceptionResolvers) { ExceptionHandlerExceptionResolver er = new ExceptionHandlerExceptionResolver(); er.setCustomArgumentResolvers(defaultMethodArgumentResolvers()); @@ -673,7 +673,7 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon configurerDelegate.configureExceptionHandlerExceptionResolver(er); - return er; + exceptionResolvers.add(0, er); } @Bean