NPE in RepositoryRestHandlerMapping.exposeEffectiveLookupPathKey in response to CORS preflight request [DATAREST-1305]

[spring-projects-issues]

Bartosz Kielczewski opened DATAREST-1305 and commented

With CORS configuration:

@Configuration
internal class RepositoryRestConfig : RepositoryRestConfigurer {

    override fun configureRepositoryRestConfiguration(config: RepositoryRestConfiguration) {
        with(config) {
            corsRegistry.addMapping(PATH_PATTERN)
                    .allowedMethods(*ALLOWED_METHODS)
                    .allowedOrigins(*ALLOWED_ORIGINS)
                    .exposedHeaders(*EXPOSED_HEADERS)
        }
    }

    @Bean
    fun corsConfigurer(): WebMvcConfigurer {
        return object : WebMvcConfigurer {
            override fun addCorsMappings(registry: CorsRegistry) {
                registry.addMapping(PATH_PATTERN)
                        .allowedMethods(*ALLOWED_METHODS)
                        .allowedOrigins(*ALLOWED_ORIGINS)
                        .exposedHeaders(*EXPOSED_HEADERS)
            }
        }
    }

    companion object {
        private const val PATH_PATTERN = "/**"
        private val EXPOSED_HEADERS = arrayOf("Location", "Authorization", "Content-Disposition")
        private val ALLOWED_METHODS = arrayOf("GET", "HEAD", "PATCH", "POST", "PUT", "DELETE", "OPTIONS")
        private val ALLOWED_ORIGINS = arrayOf("http://localhost:4000")
    }

}

And with some standard repositories, i.e.:

interface DistributorRepository : JpaRepository<Distributor, String> {

    @RestResource(path = "all", rel = "all")
    fun findByOrderById(): List<Distributor>

}

When CORS preflight request is done from the browser (or curl here):

curl -v -X OPTIONS https://domain.com/api/distributors -H "Origin: http://localhost:4000" -H "Access-Control-Request-Method: GET"

The response is 500:

< HTTP/1.1 500
< Access-Control-Allow-Credentials: true
< Access-Control-Allow-Methods: GET
< Access-Control-Allow-Origin: http://localhost:4000
< Access-Control-Expose-Headers: Location, Authorization, Content-Disposition
< Access-Control-Max-Age: 1800
< Allow: GET, HEAD, POST, PUT, DELETE, TRACE, OPTIONS, PATCH
< Cache-Control: no-cache, no-store, max-age=0, must-revalidate
< Date: Wed, 14 Nov 2018 14:05:33 GMT
< Expires: 0
< Pragma: no-cache
< Server: nginx/1.15.6
< Strict-Transport-Security: max-age=31536000 ; includeSubDomains
< Vary: Origin
< Vary: Access-Control-Request-Method
< Vary: Access-Control-Request-Headers
< X-Content-Type-Options: nosniff
< X-Frame-Options: DENY
< X-XSS-Protection: 1; mode=block
< Content-Length: 0
< Connection: keep-alive

And in the stack trace there's NPE in RepositoryRestHandlerMapping.exposeEffectiveLookupPathKey:

java.lang.NullPointerException: null
    at org.springframework.data.rest.webmvc.RepositoryRestHandlerMapping.exposeEffectiveLookupPathKey(RepositoryRestHandlerMapping.java:264)
    at org.springframework.data.rest.webmvc.RepositoryRestHandlerMapping.lookupHandlerMethod(RepositoryRestHandlerMapping.java:165)
    at org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.getHandlerInternal(AbstractHandlerMethodMapping.java:347)
    at org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.getHandlerInternal(AbstractHandlerMethodMapping.java:62)
    at org.springframework.web.servlet.handler.AbstractHandlerMapping.getHandler(AbstractHandlerMapping.java:401)
    at org.springframework.data.rest.webmvc.support.DelegatingHandlerMapping$HandlerSelectionResult.from(DelegatingHandlerMapping.java:108)
    at org.springframework.data.rest.webmvc.support.DelegatingHandlerMapping.getHandler(DelegatingHandlerMapping.java:74)
    at org.springframework.web.servlet.DispatcherServlet.getHandler(DispatcherServlet.java:1231)
    at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1014)
    at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:942)
    at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:998)
    at org.springframework.web.servlet.FrameworkServlet.doOptions(FrameworkServlet.java:937)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:669)
    at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:875)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:741)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:101)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.boot.actuate.web.trace.servlet.HttpTraceFilter.doFilterInternal(HttpTraceFilter.java:90)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:320)
    at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.invoke(FilterSecurityInterceptor.java:127)
    at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.doFilter(FilterSecurityInterceptor.java:91)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:119)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:137)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.authentication.AnonymousAuthenticationFilter.doFilter(AnonymousAuthenticationFilter.java:111)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter.doFilter(SecurityContextHolderAwareRequestFilter.java:170)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.savedrequest.RequestCacheAwareFilter.doFilter(RequestCacheAwareFilter.java:63)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:116)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at uk.ac.cam.psychometrics.tdp.service.security.JwtAuthenticationFilter.doFilterInternal(JwtAuthenticationFilter.kt:19)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:66)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:105)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter.doFilterInternal(WebAsyncManagerIntegrationFilter.java:56)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:215)
    at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:178)
    at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:357)
    at org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:270)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:99)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:92)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.HiddenHttpMethodFilter.doFilterInternal(HiddenHttpMethodFilter.java:93)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter.filterAndRecordMetrics(WebMvcMetricsFilter.java:119)
    at org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter.doFilterInternal(WebMvcMetricsFilter.java:107)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:200)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:199)
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:96)
    at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:490)
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:139)
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)
    at org.apache.catalina.valves.RemoteIpValve.invoke(RemoteIpValve.java:685)
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
    at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:408)
    at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:66)
    at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:770)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1415)
    at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
    at java.lang.Thread.run(Thread.java:748)

---

Affects: 3.1.2 (Lovelace SR2)

[spring-projects-issues]

Bartosz Kielczewski commented

The NPE happens in the code introduced by DATAREST-1294.

The NPE happens when preflight request has path = /{repository} and Access-Control-Request-Method: GET

This is because HandlerMethod of preflight request has no RequestMapping. It has none because org.springframework.web.servlet.handler.AbstractHandlerMethodMapping returns PREFLIGHT_AMBIGUOUS_MATCH = new HandlerMethod(new EmptyHandler(), ClassUtils.getMethod(EmptyHandler.class, "handle"));

Now the match is ambiguous, because there are two HandlerMethods matching the request: repositoryEntityController.getCollectionResource and repositoryEntityController.getCollectionResourceCompact and both are for GET /{repository} and differ only by produced mediaType.

Why GET /{repository} and not for OPTIONS /{repository}, repositoryEntityController.optionsForCollectionResource?

Ok, that's because RequestMethodsRequestCondition explicitly matches to would-be method based on Access-Control-Request-Method header, so GET is being matched. Moreover, both GET methods are matched, because for preflight requests only RequestMethodsRequestCondition is not empty so produces gets ignored. 

BTW, it looks like RepositoryRestController.optionsForCollectionResource and RepositoryRestController.optionsForItemResource won't be matched, like never-ever, should probably be removed. Looks like you can't have @RequestMapping for OPTIONS anymore. 

So what seems problematic is:

• the existence of two GET mappings for the same path, which turns out to be ambiguous for CORS preflight requests (any other cases like this, i.e. for different paths/methods?)
• RepositoryRestHandlerMapping.lookupHandlerMethod that only checks for null handlerMethod and then happily proceeds on assumption the handlerMethod needs to have @RequestMapping, since it got matched. This could actually check if it really has.

I would do a PR, but this seems potentially bigger

[spring-projects-issues]

Bartosz Kielczewski commented

Yeah, seems bigger, can reproduce this on other paths, i.e. {repository}/search/{search}, so CORS preflight requests are broken, because multiple "would-be" methods are being mapped instead of OPTIONS.

At least there was no NPM before DATAREST-1294 was merged, so short-term fix would be to address it, possibly in RepositoryRestHandlerMapping.lookupHandlerMethod. Still the matching doesn't seem to be working as originally intended, so it can have other implications if you've relied on it somewhere.

As for me downgrading to 3.1.1 is acceptable work-around and thankfully I'm not using CORS on production