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

Handling of ResponseStatusException to also include setting of response headers #23741

Closed
Zabelinski-Andrey opened this issue Oct 1, 2019 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Milestone

Comments

@Zabelinski-Andrey
Copy link

Affects: 5.1.10
Spring RestController offers a nice function, one could filter requests by methods. E.g.:

@RequestMapping(value = "/some-pattern", method = {RequestMethod.GET, RequestMethod.HEAD})
public Mono<Void> handler(...)

If a request matches the path pattern but does not match the method pattern spring-web automatically generates a response 405 Method Not Allowed which is perfectly fine.

But RFC 7231 Explicitly requires servers to include the Allow header (https://tools.ietf.org/html/rfc7231#page-59):
The origin server MUST generate an Allow header field in a 405 response containing a list of the target resource's currently supported methods.

It request method filters must automatically insert an Allow header with a list of allowed methods into a response.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 1, 2019
@sbrannen sbrannen added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Oct 1, 2019
@sbrannen sbrannen changed the title RequestMapping should fill in Allow header when a request does not match a list of methods @RequestMapping should fill in Allow header in response when a request does not match a list of methods Oct 1, 2019
@rstoyanchev
Copy link
Contributor

It should be doing that:

response.setHeader("Allow", StringUtils.arrayToDelimitedString(supportedMethods, ", "));

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Oct 1, 2019
@Zabelinski-Andrey
Copy link
Author

Zabelinski-Andrey commented Oct 2, 2019

Thank you for the analysis, rstoyanchev.
I'm sorry I didn't show the vital detail. I use spring-webflux.
To be precise:
Dependencies:

  • org.springframework:spring-webflux:5.1.10
  • io.projectreactor:reactor-core:3.2.12.RELEASE

Project files:

  • class WebFluxConfiguration annotated with:
    • @org.springframework.context.annotation.Configuration
    • @org.springframework.web.reactive.config.EnableWebFlux

I create reactor.netty.http.server.HttpServer manually:

        HttpHandler handler = WebHttpHandlerBuilder.applicationContext(applicationContext).build();
        DisposableServer server = reactor.netty.http.server.HttpServer.create()
                .host(host).port(port)
                .tcpConfiguration(tcpServer -> tcpServer.bootstrap(
                        serverBootstrap -> serverBootstrap.group(...).channel(NioServerSocketChannel.class)))
                .handle(new ReactorHttpHandlerAdapter(handler))
                .bind().block();

And I also have some REST controllers like:

@org.springframework.web.bind.annotation.RestController
class MyRestController {
    @RequestMapping(value = "/some-pattern", method = {RequestMethod.GET, RequestMethod.HEAD})
    public Mono<Void> handler(ServerWebExchange exchange) {
        return Mono.just(exchange).subscribeOn(scheduler).flatMap(exch-> handle(exch));
    }
}

And when I request DELETE https://addr:port/some-pattern I get a response without Allow header:

HTTP/1.1 405 Method Not Allowed
content-length: 0

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 2, 2019
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 5, 2019
@rstoyanchev rstoyanchev added this to the 5.1.11 milestone Oct 5, 2019
@antimpatel
Copy link

@rstoyanchev can I pick this up? If no one is working on it. seems good for a newbie.

@rstoyanchev
Copy link
Contributor

@antimpatel sure. Probably something in ResponseStatusExceptionHandler?

@rstoyanchev rstoyanchev modified the milestones: 5.1.11, 5.2.1 Oct 15, 2019
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x labels Oct 15, 2019
@rstoyanchev rstoyanchev self-assigned this Oct 29, 2019
@rstoyanchev rstoyanchev changed the title @RequestMapping should fill in Allow header in response when a request does not match a list of methods Handling of ResponseStatusException to also include setting of response headers Oct 30, 2019
rstoyanchev added a commit that referenced this issue Oct 30, 2019
A ResponseStatus exception now exposes extra method to return headers
for the response. This is used in ResponseStatusExceptionHandler to
apply the headers to the response.

Closes gh-23741
rstoyanchev added a commit that referenced this issue Oct 30, 2019
pull bot pushed a commit to scope-demo/spring-framework that referenced this issue Oct 30, 2019
A ResponseStatus exception now exposes extra method to return headers
for the response. This is used in ResponseStatusExceptionHandler to
apply the headers to the response.

Closes spring-projectsgh-23741
pull bot pushed a commit to scope-demo/spring-framework that referenced this issue Oct 30, 2019
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) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants