From 6b6cd51edf7591b7ca9da369755f4981232033db Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Tue, 11 Oct 2022 15:32:27 +0200 Subject: [PATCH] HHH-15592 Fix NPE for uses of JdbcExceptionHelper.extractSqlState --- .../community/dialect/DerbyLegacyDialect.java | 8 +- .../community/dialect/MySQLLegacyDialect.java | 12 +- .../dialect/PostgreSQLLegacyDialect.java | 72 +++++++----- .../dialect/SybaseASELegacyDialect.java | 89 ++++++++------- .../org/hibernate/dialect/DerbyDialect.java | 8 +- .../org/hibernate/dialect/MySQLDialect.java | 12 +- .../hibernate/dialect/PostgreSQLDialect.java | 68 ++++++----- .../hibernate/dialect/SybaseASEDialect.java | 108 +++++++++--------- .../DialectSQLExceptionConversionTest.java | 34 ++++++ 9 files changed, 242 insertions(+), 169 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/dialect/DialectSQLExceptionConversionTest.java diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLegacyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLegacyDialect.java index 75dda6856a4b..c7cfabf0910d 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLegacyDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLegacyDialect.java @@ -647,8 +647,12 @@ public SQLExceptionConversionDelegate buildSQLExceptionConversionDelegate() { final String sqlState = JdbcExceptionHelper.extractSqlState( sqlException ); // final int errorCode = JdbcExceptionHelper.extractErrorCode( sqlException ); - if ( "40XL1".equals( sqlState ) || "40XL2".equals( sqlState ) ) { - throw new LockTimeoutException( message, sqlException, sql ); + if ( sqlState != null ) { + switch ( sqlState ) { + case "40XL1": + case "40XL2": + throw new LockTimeoutException( message, sqlException, sql ); + } } return null; }; diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/MySQLLegacyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/MySQLLegacyDialect.java index e4765112296c..e0f4751112a3 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/MySQLLegacyDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/MySQLLegacyDialect.java @@ -719,12 +719,14 @@ public ViolatedConstraintNameExtractor getViolatedConstraintNameExtractor() { private static final ViolatedConstraintNameExtractor EXTRACTOR = new TemplatedViolatedConstraintNameExtractor( sqle -> { - switch ( Integer.parseInt( JdbcExceptionHelper.extractSqlState( sqle ) ) ) { - case 23000: - return extractUsingTemplate( " for key '", "'", sqle.getMessage() ); - default: - return null; + final String sqlState = JdbcExceptionHelper.extractSqlState( sqle ); + if ( sqlState != null ) { + switch ( Integer.parseInt( sqlState ) ) { + case 23000: + return extractUsingTemplate( " for key '", "'", sqle.getMessage() ); + } } + return null; } ); @Override diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/PostgreSQLLegacyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/PostgreSQLLegacyDialect.java index 6d8c095ec384..ce3f61739578 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/PostgreSQLLegacyDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/PostgreSQLLegacyDialect.java @@ -872,44 +872,54 @@ public ViolatedConstraintNameExtractor getViolatedConstraintNameExtractor() { */ private static final ViolatedConstraintNameExtractor EXTRACTOR = new TemplatedViolatedConstraintNameExtractor( sqle -> { - switch ( Integer.parseInt( JdbcExceptionHelper.extractSqlState( sqle ) ) ) { - // CHECK VIOLATION - case 23514: - return extractUsingTemplate( "violates check constraint \"","\"", sqle.getMessage() ); - // UNIQUE VIOLATION - case 23505: - return extractUsingTemplate( "violates unique constraint \"","\"", sqle.getMessage() ); - // FOREIGN KEY VIOLATION - case 23503: - return extractUsingTemplate( "violates foreign key constraint \"","\"", sqle.getMessage() ); - // NOT NULL VIOLATION - case 23502: - return extractUsingTemplate( "null value in column \"","\" violates not-null constraint", sqle.getMessage() ); - // TODO: RESTRICT VIOLATION - case 23001: - return null; - // ALL OTHER - default: - return null; + final String sqlState = JdbcExceptionHelper.extractSqlState( sqle ); + if ( sqlState != null ) { + switch ( Integer.parseInt( sqlState ) ) { + // CHECK VIOLATION + case 23514: + return extractUsingTemplate( "violates check constraint \"", "\"", sqle.getMessage() ); + // UNIQUE VIOLATION + case 23505: + return extractUsingTemplate( "violates unique constraint \"", "\"", sqle.getMessage() ); + // FOREIGN KEY VIOLATION + case 23503: + return extractUsingTemplate( + "violates foreign key constraint \"", + "\"", + sqle.getMessage() + ); + // NOT NULL VIOLATION + case 23502: + return extractUsingTemplate( + "null value in column \"", + "\" violates not-null constraint", + sqle.getMessage() + ); + // TODO: RESTRICT VIOLATION + case 23001: + return null; + } } + return null; } ); @Override public SQLExceptionConversionDelegate buildSQLExceptionConversionDelegate() { return (sqlException, message, sql) -> { - switch ( JdbcExceptionHelper.extractSqlState( sqlException ) ) { - case "40P01": - // DEADLOCK DETECTED - return new LockAcquisitionException(message, sqlException, sql); - case "55P03": - // LOCK NOT AVAILABLE - return new PessimisticLockException(message, sqlException, sql); - case "57014": - return new QueryTimeoutException( message, sqlException, sql ); - default: - // returning null allows other delegates to operate - return null; + final String sqlState = JdbcExceptionHelper.extractSqlState( sqlException ); + if ( sqlState != null ) { + switch ( sqlState ) { + case "40P01": + // DEADLOCK DETECTED + return new LockAcquisitionException( message, sqlException, sql ); + case "55P03": + // LOCK NOT AVAILABLE + return new PessimisticLockException( message, sqlException, sql ); + case "57014": + return new QueryTimeoutException( message, sqlException, sql ); + } } + return null; }; } diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacyDialect.java index 244acc5643c8..97baf711051b 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacyDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacyDialect.java @@ -642,20 +642,22 @@ public ViolatedConstraintNameExtractor getViolatedConstraintNameExtractor() { */ private static final ViolatedConstraintNameExtractor EXTRACTOR = new TemplatedViolatedConstraintNameExtractor( sqle -> { + final String sqlState = JdbcExceptionHelper.extractSqlState( sqle ); final int errorCode = JdbcExceptionHelper.extractErrorCode( sqle ); - switch ( JdbcExceptionHelper.extractSqlState( sqle ) ) { - // UNIQUE VIOLATION - case "S1000": - if (2601 == errorCode) { - return extractUsingTemplate( "with unique index '", "'", sqle.getMessage() ); - } - break; - case "23000": - if (546 == errorCode) { - // Foreign key violation - return extractUsingTemplate( "constraint name = '", "'", sqle.getMessage() ); - } - break; + if ( sqlState != null ) { + switch ( sqlState ) { + // UNIQUE VIOLATION + case "S1000": + if ( 2601 == errorCode ) { + return extractUsingTemplate( "with unique index '", "'", sqle.getMessage() ); + } + break; + case "23000": + if ( 546 == errorCode ) { + // Foreign key violation + return extractUsingTemplate( "constraint name = '", "'", sqle.getMessage() ); + } + break; // // FOREIGN KEY VIOLATION // case 23503: // return extractUsingTemplate( "violates foreign key constraint \"","\"", sqle.getMessage() ); @@ -665,9 +667,7 @@ public ViolatedConstraintNameExtractor getViolatedConstraintNameExtractor() { // // TODO: RESTRICT VIOLATION // case 23001: // return null; - // ALL OTHER - default: - return null; + } } return null; } ); @@ -681,34 +681,39 @@ public SQLExceptionConversionDelegate buildSQLExceptionConversionDelegate() { return (sqlException, message, sql) -> { final String sqlState = JdbcExceptionHelper.extractSqlState( sqlException ); final int errorCode = JdbcExceptionHelper.extractErrorCode( sqlException ); - switch ( sqlState ) { - case "JZ0TO": - case "JZ006": - throw new LockTimeoutException( message, sqlException, sql ); - case "S1000": - switch ( errorCode ) { - case 515: + if ( sqlState != null ) { + switch ( sqlState ) { + case "JZ0TO": + case "JZ006": + throw new LockTimeoutException( message, sqlException, sql ); + case "S1000": + switch ( errorCode ) { + case 515: + // Attempt to insert NULL value into column; column does not allow nulls. + case 2601: + // Unique constraint violation + final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( + sqlException ); + return new ConstraintViolationException( message, sqlException, sql, constraintName ); + } + break; + case "ZZZZZ": + if ( 515 == errorCode ) { // Attempt to insert NULL value into column; column does not allow nulls. - case 2601: - // Unique constraint violation - final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( sqlException ); + final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( + sqlException ); return new ConstraintViolationException( message, sqlException, sql, constraintName ); - } - break; - case "ZZZZZ": - if (515 == errorCode) { - // Attempt to insert NULL value into column; column does not allow nulls. - final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( sqlException ); - return new ConstraintViolationException( message, sqlException, sql, constraintName ); - } - break; - case "23000": - if (546 == errorCode) { - // Foreign key violation - final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( sqlException ); - return new ConstraintViolationException( message, sqlException, sql, constraintName ); - } - break; + } + break; + case "23000": + if ( 546 == errorCode ) { + // Foreign key violation + final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( + sqlException ); + return new ConstraintViolationException( message, sqlException, sql, constraintName ); + } + break; + } } return null; }; diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/DerbyDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/DerbyDialect.java index 31a6a1692bfb..e70f4bb50efa 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/DerbyDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/DerbyDialect.java @@ -602,8 +602,12 @@ public SQLExceptionConversionDelegate buildSQLExceptionConversionDelegate() { final String sqlState = JdbcExceptionHelper.extractSqlState( sqlException ); // final int errorCode = JdbcExceptionHelper.extractErrorCode( sqlException ); - if ( "40XL1".equals( sqlState ) || "40XL2".equals( sqlState ) ) { - throw new LockTimeoutException( message, sqlException, sql ); + if ( sqlState != null ) { + switch ( sqlState ) { + case "40XL1": + case "40XL2": + throw new LockTimeoutException( message, sqlException, sql ); + } } return null; }; diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java index 88b5d4398365..cde1f711ebee 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java @@ -728,12 +728,14 @@ public ViolatedConstraintNameExtractor getViolatedConstraintNameExtractor() { private static final ViolatedConstraintNameExtractor EXTRACTOR = new TemplatedViolatedConstraintNameExtractor( sqle -> { - switch ( Integer.parseInt( JdbcExceptionHelper.extractSqlState( sqle ) ) ) { - case 23000: - return extractUsingTemplate( " for key '", "'", sqle.getMessage() ); - default: - return null; + final String sqlState = JdbcExceptionHelper.extractSqlState( sqle ); + if ( sqlState != null ) { + switch ( Integer.parseInt( sqlState ) ) { + case 23000: + return extractUsingTemplate( " for key '", "'", sqle.getMessage() ); + } } + return null; } ); @Override diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLDialect.java index 7163a888fdd7..fed7f64e82ff 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLDialect.java @@ -847,44 +847,50 @@ public ViolatedConstraintNameExtractor getViolatedConstraintNameExtractor() { */ private static final ViolatedConstraintNameExtractor EXTRACTOR = new TemplatedViolatedConstraintNameExtractor( sqle -> { - switch ( Integer.parseInt( JdbcExceptionHelper.extractSqlState( sqle ) ) ) { - // CHECK VIOLATION - case 23514: - return extractUsingTemplate( "violates check constraint \"","\"", sqle.getMessage() ); - // UNIQUE VIOLATION - case 23505: - return extractUsingTemplate( "violates unique constraint \"","\"", sqle.getMessage() ); - // FOREIGN KEY VIOLATION - case 23503: - return extractUsingTemplate( "violates foreign key constraint \"","\"", sqle.getMessage() ); - // NOT NULL VIOLATION - case 23502: - return extractUsingTemplate( "null value in column \"","\" violates not-null constraint", sqle.getMessage() ); - // TODO: RESTRICT VIOLATION - case 23001: - return null; - // ALL OTHER - default: - return null; + final String sqlState = JdbcExceptionHelper.extractSqlState( sqle ); + if ( sqlState != null ) { + switch ( Integer.parseInt( sqlState ) ) { + // CHECK VIOLATION + case 23514: + return extractUsingTemplate( "violates check constraint \"", "\"", sqle.getMessage() ); + // UNIQUE VIOLATION + case 23505: + return extractUsingTemplate( "violates unique constraint \"", "\"", sqle.getMessage() ); + // FOREIGN KEY VIOLATION + case 23503: + return extractUsingTemplate( "violates foreign key constraint \"", "\"", sqle.getMessage() ); + // NOT NULL VIOLATION + case 23502: + return extractUsingTemplate( + "null value in column \"", + "\" violates not-null constraint", + sqle.getMessage() + ); + // TODO: RESTRICT VIOLATION + case 23001: + return null; + } } + return null; } ); @Override public SQLExceptionConversionDelegate buildSQLExceptionConversionDelegate() { return (sqlException, message, sql) -> { - switch ( JdbcExceptionHelper.extractSqlState( sqlException ) ) { - case "40P01": - // DEADLOCK DETECTED - return new LockAcquisitionException(message, sqlException, sql); - case "55P03": - // LOCK NOT AVAILABLE - return new PessimisticLockException(message, sqlException, sql); - case "57014": - return new QueryTimeoutException( message, sqlException, sql ); - default: - // returning null allows other delegates to operate - return null; + final String sqlState = JdbcExceptionHelper.extractSqlState( sqlException ); + if ( sqlState != null ) { + switch ( sqlState ) { + case "40P01": + // DEADLOCK DETECTED + return new LockAcquisitionException( message, sqlException, sql ); + case "55P03": + // LOCK NOT AVAILABLE + return new PessimisticLockException( message, sqlException, sql ); + case "57014": + return new QueryTimeoutException( message, sqlException, sql ); + } } + return null; }; } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASEDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASEDialect.java index 46a808e06801..2cfd3eca7bd4 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASEDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASEDialect.java @@ -625,32 +625,33 @@ public ViolatedConstraintNameExtractor getViolatedConstraintNameExtractor() { */ private static final ViolatedConstraintNameExtractor EXTRACTOR = new TemplatedViolatedConstraintNameExtractor( sqle -> { + final String sqlState = JdbcExceptionHelper.extractSqlState( sqle ); final int errorCode = JdbcExceptionHelper.extractErrorCode( sqle ); - switch ( JdbcExceptionHelper.extractSqlState( sqle ) ) { - // UNIQUE VIOLATION - case "S1000": - if (2601 == errorCode) { - return extractUsingTemplate( "with unique index '", "'", sqle.getMessage() ); - } - break; - case "23000": - if (546 == errorCode) { - // Foreign key violation - return extractUsingTemplate( "constraint name = '", "'", sqle.getMessage() ); - } - break; -// // FOREIGN KEY VIOLATION -// case 23503: -// return extractUsingTemplate( "violates foreign key constraint \"","\"", sqle.getMessage() ); -// // NOT NULL VIOLATION -// case 23502: -// return extractUsingTemplate( "null value in column \"","\" violates not-null constraint", sqle.getMessage() ); -// // TODO: RESTRICT VIOLATION -// case 23001: -// return null; - // ALL OTHER - default: - return null; + if ( sqlState != null ) { + switch ( sqlState ) { + // UNIQUE VIOLATION + case "S1000": + if ( 2601 == errorCode ) { + return extractUsingTemplate( "with unique index '", "'", sqle.getMessage() ); + } + break; + case "23000": + if ( 546 == errorCode ) { + // Foreign key violation + return extractUsingTemplate( "constraint name = '", "'", sqle.getMessage() ); + } + break; + // // FOREIGN KEY VIOLATION + // case 23503: + // return extractUsingTemplate( "violates foreign key constraint \"","\"", sqle.getMessage() ); + // // NOT NULL VIOLATION + // case 23502: + // return extractUsingTemplate( "null value in column \"","\" violates not-null constraint", sqle.getMessage() ); + // // TODO: RESTRICT VIOLATION + // case 23001: + // return null; + // ALL OTHER + } } return null; } ); @@ -660,34 +661,39 @@ public SQLExceptionConversionDelegate buildSQLExceptionConversionDelegate() { return (sqlException, message, sql) -> { final String sqlState = JdbcExceptionHelper.extractSqlState( sqlException ); final int errorCode = JdbcExceptionHelper.extractErrorCode( sqlException ); - switch ( sqlState ) { - case "JZ0TO": - case "JZ006": - throw new LockTimeoutException( message, sqlException, sql ); - case "S1000": - switch ( errorCode ) { - case 515: + if ( sqlState != null ) { + switch ( sqlState ) { + case "JZ0TO": + case "JZ006": + throw new LockTimeoutException( message, sqlException, sql ); + case "S1000": + switch ( errorCode ) { + case 515: + // Attempt to insert NULL value into column; column does not allow nulls. + case 2601: + // Unique constraint violation + final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( + sqlException ); + return new ConstraintViolationException( message, sqlException, sql, constraintName ); + } + break; + case "ZZZZZ": + if ( 515 == errorCode ) { // Attempt to insert NULL value into column; column does not allow nulls. - case 2601: - // Unique constraint violation - final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( sqlException ); + final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( + sqlException ); return new ConstraintViolationException( message, sqlException, sql, constraintName ); - } - break; - case "ZZZZZ": - if (515 == errorCode) { - // Attempt to insert NULL value into column; column does not allow nulls. - final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( sqlException ); - return new ConstraintViolationException( message, sqlException, sql, constraintName ); - } - break; - case "23000": - if (546 == errorCode) { - // Foreign key violation - final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( sqlException ); - return new ConstraintViolationException( message, sqlException, sql, constraintName ); - } - break; + } + break; + case "23000": + if ( 546 == errorCode ) { + // Foreign key violation + final String constraintName = getViolatedConstraintNameExtractor().extractConstraintName( + sqlException ); + return new ConstraintViolationException( message, sqlException, sql, constraintName ); + } + break; + } } return null; }; diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/DialectSQLExceptionConversionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/DialectSQLExceptionConversionTest.java new file mode 100644 index 000000000000..1d86f7841351 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/DialectSQLExceptionConversionTest.java @@ -0,0 +1,34 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.orm.test.dialect; + +import java.sql.SQLException; + +import org.hibernate.dialect.Dialect; +import org.hibernate.exception.spi.SQLExceptionConversionDelegate; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.orm.junit.DialectContext; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; + +public class DialectSQLExceptionConversionTest { + + private final Dialect dialect = DialectContext.getDialect(); + + @Test + @TestForIssue(jiraKey = "HHH-15592") + public void testExceptionConversionDoesntNPE() { + final SQLExceptionConversionDelegate conversionDelegate = dialect.buildSQLExceptionConversionDelegate(); + Assumptions.assumeTrue( conversionDelegate != null ); + conversionDelegate.convert( + new SQLException(), + "test", + "test" + ); + } +}