Friendly app-level validation for a RANGE with end-before-start (#678) #706
@@ -68,19 +68,35 @@ public class GlobalExceptionHandler {
|
||||
/**
|
||||
* Backstop for any database integrity violation that slips past the explicit upstream
|
||||
* guards (e.g. a future constraint, or the import path emitting a bad range). Turns it into
|
||||
* a clean 400 instead of a 500 + Sentry alert. Deliberately generic: it does NOT inspect the
|
||||
* exception to identify the constraint — that inspection is the brittle thing we avoid, and the
|
||||
* known date-range cases are already caught upstream and never reach here.
|
||||
* a clean 400 instead of a 500 + Sentry alert. The known date-range cases are caught upstream
|
||||
* and never reach here; this only catches the unanticipated ones — so it logs the constraint
|
||||
* NAME at WARN to stay debuggable, without re-leaking SQL and without branching the response
|
||||
* on it (the response stays generic, which is the non-brittle part).
|
||||
*/
|
||||
@ExceptionHandler(DataIntegrityViolationException.class)
|
||||
public ResponseEntity<ErrorResponse> handleDataIntegrityViolation(DataIntegrityViolationException ex) {
|
||||
// Fixed message only: ex.getMessage() embeds the constraint name + SQL (CWE-209), and
|
||||
// passing ex to the logger would dump the same into Loki — so do neither, and no Sentry.
|
||||
log.warn("Rejected a request that violated a database integrity constraint");
|
||||
// Log the constraint NAME only — schema metadata, safe for Loki, and enough to tell which
|
||||
// constraint fired at 2am. Never pass `ex` / `ex.getMessage()`: those embed the SQL + the
|
||||
// offending values (CWE-209). No Sentry: an integrity violation is a 400, not a system fault.
|
||||
log.warn("Rejected a request that violated a database integrity constraint: {}", constraintNameOf(ex));
|
||||
return ResponseEntity.badRequest()
|
||||
.body(new ErrorResponse(ErrorCode.VALIDATION_ERROR, "The submitted data violated a database constraint"));
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the offending constraint's name from the cause chain, or {@code "unknown"}.
|
||||
* Reads only the name (a non-sensitive schema identifier) — never the SQL or the values.
|
||||
*/
|
||||
private static String constraintNameOf(Throwable ex) {
|
||||
for (Throwable t = ex; t != null && t != t.getCause(); t = t.getCause()) {
|
||||
if (t instanceof org.hibernate.exception.ConstraintViolationException cve
|
||||
&& cve.getConstraintName() != null) {
|
||||
return cve.getConstraintName();
|
||||
}
|
||||
}
|
||||
return "unknown";
|
||||
}
|
||||
|
||||
@ExceptionHandler(Exception.class)
|
||||
public ResponseEntity<ErrorResponse> handleGeneric(Exception ex) {
|
||||
Sentry.captureException(ex);
|
||||
|
||||
@@ -72,9 +72,48 @@ class GlobalExceptionHandlerTest {
|
||||
.as("logs a WARN line")
|
||||
.anySatisfy(e -> assertThat(e.getLevel()).isEqualTo(Level.WARN));
|
||||
assertThat(appender.list)
|
||||
.as("never logs the constraint name / SQL (would re-leak to Loki)")
|
||||
.as("never logs the SQL statement / values (would re-leak to Loki)")
|
||||
.noneSatisfy(e -> {
|
||||
assertThat(e.getFormattedMessage()).contains("chk_");
|
||||
assertThat(e.getFormattedMessage()).contains("could not execute statement");
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
void handleDataIntegrityViolation_logsConstraintName_butNotTheSql() {
|
||||
// Debuggability (DevOps): the WARN must name *which* constraint fired so an
|
||||
// unanticipated violation isn't a silent mystery — but it must carry the name only,
|
||||
// never the SQL statement or the offending values that the SQLException message holds.
|
||||
java.sql.SQLException sql = new java.sql.SQLException(
|
||||
"ERROR: violates check constraint; could not execute statement; values (1917-01-10)");
|
||||
org.hibernate.exception.ConstraintViolationException cve =
|
||||
new org.hibernate.exception.ConstraintViolationException(
|
||||
"constraint violation", sql, "chk_meta_date_end_after_start");
|
||||
DataIntegrityViolationException ex = new DataIntegrityViolationException("wrapper", cve);
|
||||
|
||||
Logger handlerLogger = (Logger) LoggerFactory.getLogger(GlobalExceptionHandler.class);
|
||||
ListAppender<ILoggingEvent> appender = new ListAppender<>();
|
||||
appender.start();
|
||||
handlerLogger.addAppender(appender);
|
||||
|
||||
try (MockedStatic<Sentry> sentryMock = mockStatic(Sentry.class)) {
|
||||
ResponseEntity<GlobalExceptionHandler.ErrorResponse> response =
|
||||
handler.handleDataIntegrityViolation(ex);
|
||||
|
||||
// Response stays generic and leak-free (CWE-209) regardless of what we log.
|
||||
assertThat(response.getStatusCode().value()).isEqualTo(400);
|
||||
assertThat(response.getBody().message())
|
||||
.doesNotContain("chk_")
|
||||
.doesNotContain("meta_date");
|
||||
sentryMock.verifyNoInteractions();
|
||||
} finally {
|
||||
handlerLogger.detachAppender(appender);
|
||||
}
|
||||
|
||||
assertThat(appender.list)
|
||||
.as("WARN names the constraint for debuggability")
|
||||
.anySatisfy(e -> assertThat(e.getFormattedMessage()).contains("chk_meta_date_end_after_start"));
|
||||
assertThat(appender.list)
|
||||
.as("but never the SQL statement or values")
|
||||
.noneSatisfy(e -> assertThat(e.getFormattedMessage()).contains("could not execute statement"));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user