Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs should warn against split URL handling across handler mappings #24304

Closed
xiaoqiang0-0 opened this issue Jan 7, 2020 · 8 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Milestone

Comments

@xiaoqiang0-0
Copy link

Affects: <5.1.9.RELEASE>


This issue should also exist in older versions!

At the same time, through the custom controller and implement org.springframework.web.servlet.config.annotation.WebMvcConfigurer#addViewControllers to register the mapping of different methods with the url, a 405 error will occur.

    @Override
    public void addViewControllers(ViewControllerRegistry registry) {
        registry.addViewController("/foo").setViewName("foo");
    }
    @PostMapping("/foo")
    public ResponseEntity<?> foo() {
        return ResponseEntity.ok("Success!");
    }
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 7, 2020
@bclozel
Copy link
Member

bclozel commented Jan 7, 2020

When a request comes in, it is mapped to a HandlerMapping instance, checking each of those in the order they're defined. The ViewControllerRegistry creates a SimpleUrlHandlerMapping ordered at 1, and the RequestMappingHandlerMapping is ordered at 0 (so it is checked before the view controller one).

I've tried to reproduce your sample and couldn't:

$ curl -XPOST http://localhost:8080/foo
Success!

Since the annotated handlers are considered first, they first get a chance to handle that request. Maybe you've got something specific in your Spring MVC setup? Or maybe your Controller class has a @RequestMapping("/something") annotation on it that prefixes routes defined in it?

All routes defined with mapping annotations are considered within RequestMappingHandlerMapping and Spring MVC fails if duplicate mappings are defined. We can't do that globally for all HandlerMapping instances.

In your case, the SimpleUrlHandlerMapping that is created for view controllers is handling that request - and since only GET methods are supported there, you're getting the "method not allowed" HTTP error.

So, in summary:

  • I can't reproduce your setup, and the annotated handlers should handle that request
  • by design, we can't prevent your application to have "duplicate" URL registrations, because each HandlerMapping is free to define its own behavior to handle (or not) incoming requests.

For additional questions, please use StackOverflow.
Thanks!

@bclozel bclozel closed this as completed Jan 7, 2020
@bclozel bclozel added for: stackoverflow A question that's better suited to stackoverflow.com status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 7, 2020
@xiaoqiang0-0
Copy link
Author

Yes, I know that SimpleUrlHandlerMapping can be ranked before RequestMappingHandlerMapping by modifying the order value, and the GET methods will be supported。But If the order value of SimpleUrlHandlerMapping is set smaller than RequestMappingHandlerMapping, such as -1, it will cause POST or other methods to be unrecognized, and a 405 error will also occur.

@SpringBootApplication
@RestController
public class Application implements WebMvcConfigurer {
    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

    @PostMapping("/foo")
    public String foo(){
        return "foo";
    }

    @Override
    public void addViewControllers(ViewControllerRegistry registry) {
        registry.setOrder(-1);
        registry.addViewController("/foo").setViewName("foo");
    }
}

This is the result of my local run:

$ curl -s -XGET http://localhost:8080/foo
foo
$ curl -s -XPOST http://localhost:8080/foo
{"timestamp":"2020-01-07T11:35:52.752+0000","status":405,"error":"Method Not Allowed","message":"Request method 'POST' not supported","path":"/foo"}

If you do not modify the default order, the results will be reversed:

$ curl -s -XPOST http://localhost:8080/foo
foo
$ curl -s -XGET http://localhost:8080/foo
{"timestamp":"2020-01-07T11:38:37.792+0000","status":405,"error":"Method Not Allowed","message":"Request method 'GET' not supported","path":"/foo"}

Thanks!

@xiaoqiang0-0
Copy link
Author

I'm sorry. Although this may not be reasonable by design, I think it is a bug. After all, the two registration methods support different methods.

@bclozel
Copy link
Member

bclozel commented Jan 7, 2020

@xiaoqiang0-0 I see how this can be bothering in your case. Unfortunately, the HandlerMapping contract does not operate at that level. I don't think this is the proper place to solve this problem.

We can solve mapping conflicts withing a HandlerMapping implementation (as we do for the annotation variant). I'm not familiar with your use case, but maybe dynamically registering handlers or using the functional endpoints would be a better choice?

@xiaoqiang0-0
Copy link
Author

@bclozel This is demo of using the functional endpoints.

spring boot version: 2.2.2.RELEASE

@SpringBootApplication
@RestController
public class Application {

    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

    @Bean
    public RouterFunction<ServerResponse> routerFunctionFoo() {
        return route()
                .GET("/foo", accept(ALL), (req) -> ServerResponse.ok().contentType(TEXT_PLAIN).body("GET: Foo!"))
                .build();
    }

    @PostMapping("/foo")
    public String foo() {
        return "POST: foo!";
    }
}
$ curl -s -XPOST http://localhost:9999/foo
POST: foo!
$ curl -s -XGET http://localhost:9999/foo
{"timestamp":"2020-01-08T02:35:21.273+0000","status":405,"error":"Method Not Allowed","message":"Request method 'GET' not supported","trace":"org.springframework.web.HttpRequestMethodNotSupportedException: Request method 'GET' not supported\r\n\tat org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping.handleNoMatch(RequestMappingInfoHandlerMapping.java:201)\r\n\tat org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.lookupHandlerMethod(AbstractHandlerMethodMapping.java:421)\r\n\tat org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.getHandlerInternal(AbstractHandlerMethodMapping.java:367)\r\n\tat org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.getHandlerInternal(RequestMappingHandlerMapping.java:449)\r\n\tat org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.getHandlerInternal(RequestMappingHandlerMapping.java:67)\r\n\tat org.springframework.web.servlet.handler.AbstractHandlerMapping.getHandler(AbstractHandlerMapping.java:393)\r\n\tat org.springframework.web.servlet.DispatcherServlet.getHandler(DispatcherServlet.java:1234)\r\n\tat org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1016)\r\n\tat org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:943)\r\n\tat org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)\r\n\tat org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898)\r\n\tat javax.servlet.http.HttpServlet.service(HttpServlet.java:634)\r\n\tat org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)\r\n\tat javax.servlet.http.HttpServlet.service(HttpServlet.java:741)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)\r\n\tat org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)\r\n\tat org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)\r\n\tat org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)\r\n\tat org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)\r\n\tat org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)\r\n\tat org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)\r\n\tat org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:202)\r\n\tat org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:96)\r\n\tat org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:526)\r\n\tat org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:139)\r\n\tat org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)\r\n\tat org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)\r\n\tat org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)\r\n\tat org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:367)\r\n\tat org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)\r\n\tat org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:860)\r\n\tat org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1591)\r\n\tat org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)\r\n\tat java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)\r\n\tat java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)\r\n\tat org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)\r\n\tat java.lang.Thread.run(Thread.java:744)\r\n","path":"/foo"}

and result is similar.
If it is not allowed to use two or more methods to register handlers with the same URL and different methods at the same time, I think it should be explained at least in the documentation.

@rstoyanchev
Copy link
Contributor

A match by URL is considered a strong enough indication that the matched handler owns the handling of that endpoint URL. This is what enables returning a 405 error from RequestMappingHandlerMapping which in most cases is helpful as it points out what is actually supported.

Splitting the handling of a given URL across different handler mappings is always going to be a challenge for this reason. I would definitely encourage avoiding such a split and consolidating for example by creating your own view controller:

@Controller
public class MyViewController {

    @GetMapping("/foo")
    public String foo() {
        return "foo";
    }
}

I think we could maybe add something to the Javadoc of ViewControllerRegistry and/or the MVC config section to warn against splitting the handling of a URL across handler mappings.

@xiaoqiang0-0
Copy link
Author

@rstoyanchev Thank you very much for your answer. I also understand that it is not reasonable to write code like this, but unexpected operation found such a problem. However, this problem may actually occur, so I hope at least it can be explained or warned in the documentation.Of course, I hope that in the future, we can solve the problem of splitting the processing of a given URL across different handler mappings.
Thank you!

@rstoyanchev rstoyanchev changed the title Multiple registration URLs cause 405 errors Docs should warn against split URL handling across handler mappings Jan 9, 2020
@rstoyanchev rstoyanchev self-assigned this Jan 9, 2020
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task and removed for: stackoverflow A question that's better suited to stackoverflow.com status: invalid An issue that we don't feel is valid labels Jan 9, 2020
@rstoyanchev rstoyanchev added this to the 5.2.3 milestone Jan 9, 2020
@rstoyanchev
Copy link
Contributor

I don't see any easy answers to be honest. SimpleUrlHandlerMapping doesn't even match on anything other than the URL, and RequestMappingHandlerMapping sends 4xx errors based on partial matches. However we can improve the docs at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

4 participants