Skip to content

Commit 1515a5b

Browse files
Prevent exposure of exception details to clients.
Closes GH-2571.
1 parent d5b4de4 commit 1515a5b

7 files changed

Lines changed: 137 additions & 20 deletions

File tree

spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractWebIntegrationTests.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
2121
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
2222

23-
import io.micrometer.observation.ObservationRegistry;
2423
import jakarta.servlet.Filter;
2524
import jakarta.servlet.FilterChain;
2625
import jakarta.servlet.ServletException;
@@ -129,13 +128,13 @@ protected MockHttpServletResponse putAndGet(Link link, Object payload, MediaType
129128
return StringUtils.hasText(response.getContentAsString()) ? response : client.request(link);
130129
}
131130

132-
protected MockHttpServletResponse putOnlyExpect5XXStatus(Link link, Object payload, MediaType mediaType)
131+
protected MockHttpServletResponse putOnlyExpect4XXStatus(Link link, Object payload, MediaType mediaType)
133132
throws Exception {
134133

135134
String href = link.isTemplated() ? link.expand().getHref() : link.getHref();
136135

137136
MockHttpServletResponse response = mvc.perform(put(href).content(payload.toString()).contentType(mediaType))//
138-
.andExpect(status().is5xxServerError())//
137+
.andExpect(status().is4xxClientError())//
139138
.andReturn().getResponse();
140139

141140
return StringUtils.hasText(response.getContentAsString()) ? response : client.request(link);

spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ void associateTwoCreatorsToOrderWithSinglePut() throws Exception {
373373
Link secondCreatorLink = preparePersonResource(new Person("Pippin", "Baggins"));
374374
Link orderLinkToItsCreator = prepareOrderResource(new Order());
375375

376-
MockHttpServletResponse response = putOnlyExpect5XXStatus(orderLinkToItsCreator,
376+
MockHttpServletResponse response = putOnlyExpect4XXStatus(orderLinkToItsCreator,
377377
toUriList(firstCreatorLink, secondCreatorLink), TEXT_URI_LIST);
378378
assertThat(response.getContentAsString()).contains("send only 1 link");
379379
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright 2026-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.rest.webmvc;
17+
18+
import org.springframework.util.Assert;
19+
20+
class InvalidStateTransitionRequest extends RuntimeException {
21+
22+
private static final long serialVersionUID = -8496438973254611610L;
23+
24+
private final String message;
25+
26+
InvalidStateTransitionRequest(String message) {
27+
28+
Assert.hasText(message, "Message must not be null or empty!");
29+
30+
this.message = message;
31+
}
32+
33+
@Override
34+
public String getMessage() {
35+
return message;
36+
}
37+
}

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ public ResponseEntity<? extends RepresentationModel<?>> createPropertyReference(
283283
}
284284

285285
if (!source.getLinks().hasSingleLink()) {
286-
throw new IllegalArgumentException(
286+
throw new InvalidStateTransitionRequest(
287287
"Must send only 1 link to update a property reference that isn't a List or a Map.");
288288
}
289289

@@ -425,8 +425,8 @@ public ResponseEntity<Void> handle(HttpRequestMethodNotSupportedException except
425425
}
426426

427427
/**
428-
* Custom {@link RepresentationModel} to be used with maps as {@link EntityModel} doesn't properly unwrap
429-
* {@link Map}s due to some limitation in Jackson.
428+
* Custom {@link RepresentationModel} to be used with maps as {@link EntityModel} doesn't properly unwrap {@link Map}s
429+
* due to some limitation in Jackson.
430430
*
431431
* @author Oliver Drotbohm
432432
* @see https://github.com/FasterXML/jackson-databind/issues/171

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestExceptionHandler.java

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,17 @@ ResponseEntity<ExceptionMessage> handleNotReadable(HttpMessageNotReadableExcepti
8585
return badRequest(new HttpHeaders(), o_O);
8686
}
8787

88+
@ExceptionHandler
89+
ResponseEntity<ExceptionMessage> handle(InvalidStateTransitionRequest o_O) {
90+
return badRequest(new HttpHeaders(), o_O);
91+
}
92+
8893
/**
8994
* Handle failures commonly thrown from code tries to read incoming data and convert or cast it to the right type by
90-
* returning {@code 500 Internal Server Error} and the thrown exception marshalled into JSON.
95+
* returning {@code 500 Internal Server Error}.
96+
* <p>
97+
* The raw exception message is not forwarded to the client to prevent leaking internal type names and input values.
98+
* The full exception is logged at WARN level for server-side diagnostics.
9199
*
92100
* @param o_O the exception to handle.
93101
* @return
@@ -96,7 +104,10 @@ ResponseEntity<ExceptionMessage> handleNotReadable(HttpMessageNotReadableExcepti
96104
ConversionFailedException.class, NullPointerException.class })
97105
ResponseEntity<ExceptionMessage> handleMiscFailures(Exception o_O) {
98106

99-
return errorResponse(HttpStatus.INTERNAL_SERVER_ERROR, new HttpHeaders(), o_O);
107+
LOG.warn("Unexpected failure during request processing!", o_O);
108+
109+
return response(HttpStatus.INTERNAL_SERVER_ERROR, new HttpHeaders(),
110+
new ExceptionMessage("Unexpected failure during request processing!"));
100111
}
101112

102113
/**
@@ -120,7 +131,10 @@ ResponseEntity<RepositoryConstraintViolationExceptionMessage> handleRepositoryCo
120131
*/
121132
@ExceptionHandler({ OptimisticLockingFailureException.class, DataIntegrityViolationException.class })
122133
ResponseEntity<ExceptionMessage> handleConflict(Exception o_O) {
123-
return errorResponse(HttpStatus.CONFLICT, new HttpHeaders(), o_O);
134+
135+
LOG.warn("Could not commit changes!", o_O);
136+
137+
return response(HttpStatus.CONFLICT, new HttpHeaders(), new ExceptionMessage("Could not commit changes!"));
124138
}
125139

126140
/**
Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,60 @@
11
package org.springframework.data.rest.webmvc.support;
22

3+
import org.springframework.lang.Nullable;
4+
import org.springframework.util.Assert;
5+
6+
import com.fasterxml.jackson.annotation.JsonIgnore;
37
import com.fasterxml.jackson.annotation.JsonProperty;
48

59
/**
610
* A helper that renders an {@link Exception} JSON-friendly.
7-
*
11+
* <p>
12+
* Only the outermost exception message is serialized into the HTTP response. The cause chain is accessible via
13+
* {@link #getCause()} for server-side use but is excluded from JSON serialization to prevent leaking internals to
14+
* clients.
15+
*
816
* @author Jon Brisbin
17+
* @author Oliver Drotbohm
918
*/
1019
public class ExceptionMessage {
1120

1221
private final Throwable throwable;
22+
private final String message;
1323

1424
public ExceptionMessage(Throwable throwable) {
25+
26+
Assert.notNull(throwable, "Throwable must not be null!");
27+
1528
this.throwable = throwable;
29+
this.message = throwable.getMessage();
30+
}
31+
32+
public ExceptionMessage(String message) {
33+
34+
Assert.notNull(message, "Message must not be null!");
35+
36+
this.message = message;
37+
this.throwable = null;
1638
}
1739

1840
@JsonProperty("message")
1941
public String getMessage() {
20-
return throwable.getMessage();
42+
return message;
2143
}
2244

23-
@JsonProperty("cause")
24-
public ExceptionMessage getCause() {
25-
return throwable.getCause() != null ? new ExceptionMessage(throwable.getCause()) : null;
45+
@JsonIgnore
46+
public @Nullable ExceptionMessage getCause() {
47+
48+
if (throwable == null) {
49+
return null;
50+
}
51+
52+
Throwable cause = throwable.getCause();
53+
54+
if (cause == null) {
55+
return null;
56+
}
57+
58+
return new ExceptionMessage(cause);
2659
}
2760
}

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestExceptionHandlerUnitTests.java

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,19 @@
2020
import ch.qos.logback.classic.Level;
2121
import ch.qos.logback.classic.Logger;
2222

23+
import java.util.stream.Stream;
24+
2325
import org.junit.jupiter.api.AfterAll;
2426
import org.junit.jupiter.api.BeforeAll;
2527
import org.junit.jupiter.api.Test;
28+
import org.junit.jupiter.params.ParameterizedTest;
29+
import org.junit.jupiter.params.provider.MethodSource;
2630
import org.slf4j.LoggerFactory;
2731
import org.springframework.context.support.StaticMessageSource;
32+
import org.springframework.core.convert.ConversionFailedException;
33+
import org.springframework.core.convert.TypeDescriptor;
2834
import org.springframework.dao.DataIntegrityViolationException;
35+
import org.springframework.dao.OptimisticLockingFailureException;
2936
import org.springframework.data.rest.webmvc.support.ExceptionMessage;
3037
import org.springframework.http.HttpStatus;
3138
import org.springframework.http.ResponseEntity;
@@ -74,14 +81,41 @@ void handlesConflictCorrectly() {
7481
assertThat(result.getStatusCode()).isEqualTo(HttpStatus.CONFLICT);
7582
}
7683

77-
@Test // DATAREST-706
78-
void forwardsExceptionForMiscellaneousFailure() {
84+
static java.util.stream.Stream<Exception> conflictExceptions() {
85+
return java.util.stream.Stream.of(
86+
new DataIntegrityViolationException("could not execute statement",
87+
new RuntimeException("Some message.")),
88+
new OptimisticLockingFailureException("Some message."));
89+
}
90+
91+
@ParameterizedTest // GH-2571
92+
@MethodSource("conflictExceptions")
93+
void conflictResponseHasExpectedStatusCode(Exception exception) {
94+
95+
ResponseEntity<ExceptionMessage> result = HANDLER.handleConflict(exception);
96+
97+
assertThat(result.getStatusCode()).isEqualTo(HttpStatus.CONFLICT);
98+
assertThat(result.getBody()).isNotNull();
99+
assertThat(result.getBody().getMessage()).doesNotContain("Some message.");
100+
}
101+
102+
static Stream<Exception> miscExceptions() {
103+
return Stream.of(
104+
new IllegalArgumentException("Some message."),
105+
new ClassCastException("Some message."),
106+
new NullPointerException("Some message."),
107+
new ConversionFailedException(TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(Long.class),
108+
"some-value", new RuntimeException("Some message.")));
109+
}
79110

80-
String message = "My Message";
111+
@ParameterizedTest // GH-2571
112+
@MethodSource("miscExceptions")
113+
void miscFailureResponseHasExpectedStatusCode(Exception exception) {
81114

82-
ResponseEntity<ExceptionMessage> result = HANDLER.handleMiscFailures(new Exception(message));
115+
ResponseEntity<ExceptionMessage> result = HANDLER.handleMiscFailures(exception);
83116

117+
assertThat(result.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
84118
assertThat(result.getBody()).isNotNull();
85-
assertThat(result.getBody().getMessage()).isEqualTo(message);
119+
assertThat(result.getBody().getMessage()).doesNotContain("Some message.");
86120
}
87121
}

0 commit comments

Comments
 (0)