diff --git a/spring-web/src/main/java/org/springframework/web/server/MethodNotAllowedException.java b/spring-web/src/main/java/org/springframework/web/server/MethodNotAllowedException.java index 4c10051fcbf6..14dcd2a62b8f 100644 --- a/spring-web/src/main/java/org/springframework/web/server/MethodNotAllowedException.java +++ b/spring-web/src/main/java/org/springframework/web/server/MethodNotAllowedException.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,12 +19,15 @@ import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; /** * Exception for errors that fit response status 405 (method not allowed). @@ -37,7 +40,7 @@ public class MethodNotAllowedException extends ResponseStatusException { private final String method; - private final Set supportedMethods; + private final Set httpMethods; public MethodNotAllowedException(HttpMethod method, Collection supportedMethods) { @@ -51,10 +54,21 @@ public MethodNotAllowedException(String method, @Nullable Collection supportedMethods = Collections.emptySet(); } this.method = method; - this.supportedMethods = Collections.unmodifiableSet(new HashSet<>(supportedMethods)); + this.httpMethods = Collections.unmodifiableSet(new HashSet<>(supportedMethods)); } + /** + * Return a Map with an "Allow" header. + * @since 5.1.11 + */ + @Override + public Map getHeaders() { + return !CollectionUtils.isEmpty(this.httpMethods) ? + Collections.singletonMap("Allow", StringUtils.collectionToDelimitedString(this.httpMethods, ", ")) : + Collections.emptyMap(); + } + /** * Return the HTTP method for the failed request. */ @@ -66,6 +80,7 @@ public String getHttpMethod() { * Return the list of supported HTTP methods. */ public Set getSupportedMethods() { - return this.supportedMethods; + return this.httpMethods; } + } diff --git a/spring-web/src/main/java/org/springframework/web/server/NotAcceptableStatusException.java b/spring-web/src/main/java/org/springframework/web/server/NotAcceptableStatusException.java index a93651f97220..aa8a01d46a68 100644 --- a/spring-web/src/main/java/org/springframework/web/server/NotAcceptableStatusException.java +++ b/spring-web/src/main/java/org/springframework/web/server/NotAcceptableStatusException.java @@ -18,9 +18,11 @@ import java.util.Collections; import java.util.List; +import java.util.Map; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; +import org.springframework.util.CollectionUtils; /** * Exception for errors that fit response status 406 (not acceptable). @@ -51,6 +53,17 @@ public NotAcceptableStatusException(List supportedMediaTypes) { } + /** + * Return a Map with an "Accept" header. + * @since 5.1.11 + */ + @Override + public Map getHeaders() { + return !CollectionUtils.isEmpty(this.supportedMediaTypes) ? + Collections.singletonMap("Accept", MediaType.toString(this.supportedMediaTypes)) : + Collections.emptyMap(); + } + /** * Return the list of supported content types in cases when the Accept * header is parsed but not supported, or an empty list otherwise. diff --git a/spring-web/src/main/java/org/springframework/web/server/ResponseStatusException.java b/spring-web/src/main/java/org/springframework/web/server/ResponseStatusException.java index af5e14c60b12..a0245686a081 100644 --- a/spring-web/src/main/java/org/springframework/web/server/ResponseStatusException.java +++ b/spring-web/src/main/java/org/springframework/web/server/ResponseStatusException.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,9 @@ package org.springframework.web.server; +import java.util.Collections; +import java.util.Map; + import org.springframework.core.NestedExceptionUtils; import org.springframework.core.NestedRuntimeException; import org.springframework.http.HttpStatus; @@ -72,12 +75,21 @@ public ResponseStatusException(HttpStatus status, @Nullable String reason, @Null /** - * The HTTP status that fits the exception (never {@code null}). + * Return the HTTP status associated with this exception. */ public HttpStatus getStatus() { return this.status; } + /** + * Return response headers associated with the exception, possibly required + * for the given status code (e.g. "Allow", "Accept"). + * @since 5.1.11 + */ + public Map getHeaders() { + return Collections.emptyMap(); + } + /** * The reason explaining the exception (potentially {@code null} or empty). */ @@ -86,6 +98,7 @@ public String getReason() { return this.reason; } + @Override public String getMessage() { String msg = this.status + (this.reason != null ? " \"" + this.reason + "\"" : ""); diff --git a/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java b/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java index f8388669cb9c..263e9cf1afe0 100644 --- a/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java +++ b/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.lang.Nullable; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; @@ -62,8 +63,7 @@ public void setWarnLogCategory(String loggerName) { @Override public Mono handle(ServerWebExchange exchange, Throwable ex) { - HttpStatus status = resolveStatus(ex); - if (status == null || !exchange.getResponse().setStatusCode(status)) { + if (!updateResponse(exchange.getResponse(), ex)) { return Mono.error(ex); } @@ -86,16 +86,25 @@ private String formatError(Throwable ex, ServerHttpRequest request) { return "Resolved [" + reason + "] for HTTP " + request.getMethod() + " " + path; } - @Nullable - private HttpStatus resolveStatus(Throwable ex) { + private boolean updateResponse(ServerHttpResponse response, Throwable ex) { + boolean result = false; HttpStatus status = determineStatus(ex); - if (status == null) { + if (status != null) { + if (response.setStatusCode(status)) { + if (ex instanceof ResponseStatusException) { + ((ResponseStatusException) ex).getHeaders() + .forEach((name, value) -> response.getHeaders().add(name, value)); + } + result = true; + } + } + else { Throwable cause = ex.getCause(); if (cause != null) { - status = resolveStatus(cause); + result = updateResponse(response, cause); } } - return status; + return result; } /** diff --git a/spring-web/src/test/java/org/springframework/web/server/handler/ResponseStatusExceptionHandlerTests.java b/spring-web/src/test/java/org/springframework/web/server/handler/ResponseStatusExceptionHandlerTests.java index e760e0cfd986..1048dbb226c9 100644 --- a/spring-web/src/test/java/org/springframework/web/server/handler/ResponseStatusExceptionHandlerTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/handler/ResponseStatusExceptionHandlerTests.java @@ -17,15 +17,21 @@ package org.springframework.web.server.handler; import java.time.Duration; +import java.util.Arrays; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; +import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; import org.springframework.mock.web.test.server.MockServerWebExchange; +import org.springframework.web.server.MethodNotAllowedException; +import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ResponseStatusException; import static org.assertj.core.api.Assertions.assertThat; @@ -67,6 +73,26 @@ public void handleNestedResponseStatusException() { assertThat(this.exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); } + @Test // gh-23741 + public void handleMethodNotAllowed() { + Throwable ex = new MethodNotAllowedException(HttpMethod.PATCH, Arrays.asList(HttpMethod.POST, HttpMethod.PUT)); + this.handler.handle(this.exchange, ex).block(Duration.ofSeconds(5)); + + MockServerHttpResponse response = this.exchange.getResponse(); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.METHOD_NOT_ALLOWED); + assertThat(response.getHeaders().getAllow()).containsOnly(HttpMethod.POST, HttpMethod.PUT); + } + + @Test // gh-23741 + public void handleResponseStatusExceptionWithHeaders() { + Throwable ex = new NotAcceptableStatusException(Arrays.asList(MediaType.TEXT_PLAIN, MediaType.TEXT_HTML)); + this.handler.handle(this.exchange, ex).block(Duration.ofSeconds(5)); + + MockServerHttpResponse response = this.exchange.getResponse(); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_ACCEPTABLE); + assertThat(response.getHeaders().getAccept()).containsOnly(MediaType.TEXT_PLAIN, MediaType.TEXT_HTML); + } + @Test public void unresolvedException() { Throwable expected = new IllegalStateException(); diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/messaging/WebSocketStompClient.java b/spring-websocket/src/main/java/org/springframework/web/socket/messaging/WebSocketStompClient.java index dfdcc1830ba4..59507459fd8e 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/messaging/WebSocketStompClient.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/messaging/WebSocketStompClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,6 +57,7 @@ import org.springframework.web.socket.WebSocketMessage; import org.springframework.web.socket.WebSocketSession; import org.springframework.web.socket.client.WebSocketClient; +import org.springframework.web.socket.handler.LoggingWebSocketHandlerDecorator; import org.springframework.web.socket.sockjs.transport.SockJsSession; import org.springframework.web.util.UriComponentsBuilder; @@ -265,7 +266,9 @@ public ListenableFuture connect(URI url, @Nullable WebSocketHttpHe Assert.notNull(url, "'url' must not be null"); ConnectionHandlingStompSession session = createSession(connectHeaders, sessionHandler); WebSocketTcpConnectionHandlerAdapter adapter = new WebSocketTcpConnectionHandlerAdapter(session); - getWebSocketClient().doHandshake(adapter, handshakeHeaders, url).addCallback(adapter); + getWebSocketClient() + .doHandshake(new LoggingWebSocketHandlerDecorator(adapter), handshakeHeaders, url) + .addCallback(adapter); return session.getSessionFuture(); } diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/messaging/WebSocketStompClientTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/messaging/WebSocketStompClientTests.java index f65ef7ed0c03..4d849bde949c 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/messaging/WebSocketStompClientTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/messaging/WebSocketStompClientTests.java @@ -46,6 +46,7 @@ import org.springframework.web.socket.WebSocketHandler; import org.springframework.web.socket.WebSocketSession; import org.springframework.web.socket.client.WebSocketClient; +import org.springframework.web.socket.handler.WebSocketHandlerDecorator; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; @@ -319,9 +320,12 @@ private WebSocketHandler connect() { @SuppressWarnings("unchecked") private TcpConnection getTcpConnection() throws Exception { - WebSocketHandler webSocketHandler = connect(); - webSocketHandler.afterConnectionEstablished(this.webSocketSession); - return (TcpConnection) webSocketHandler; + WebSocketHandler handler = connect(); + handler.afterConnectionEstablished(this.webSocketSession); + if (handler instanceof WebSocketHandlerDecorator) { + handler = ((WebSocketHandlerDecorator) handler).getLastHandler(); + } + return (TcpConnection) handler; } private void testInactivityTaskScheduling(Runnable runnable, long delay, long sleepTime)