Reject out-of-range sql date/time fields in DateTimeConverter.toDate - #436
Reject out-of-range sql date/time fields in DateTimeConverter.toDate#436rootvector2 wants to merge 2 commits into
Conversation
|
Hello @rootvector2 Thank you for the PR. The Javadoc and exception messages should be updated, for example
The behavioral contract has now become stricter: out-of-range fields are rejected instead of being rolled over. The error messages are unchanged. The Javadoc and messages are technically still true but it do not document the new |
garydgregory
changed the title
|
Makes sense. Updated all three exception messages to append "validation is strict, out-of-range fields are rejected" and added the same note to the |
in DateTimeConverter.toDate The problematic bug I fixed is three instances of double parsing. Instead of going round and round, I applied the diff locally, made the following changes, and pushed to master. Double parsing for the `java.sql.Date` case. Validation parses with the strict formatter and valueOf parses again. Functionally correct, but wasteful. For example: LocalDate.parse(value, SQL_DATE_FORMAT); return type.cast(java.sql.Date.valueOf(value)); The parsed LocalDate is discarded. A more efficient form would be Date.valueOf(LocalDate.parse(...)). Same for Time / Timestamp. Double parsing of for the `java.sql.Time` case. Same as above. Double parsing of for the `java.sql.Timestamp` case. Same as above. I added details to Javadocs to clarify intent. I added the follow unit tests: In SqlDateConverterTest testDefaultStringToTypeConvertValidLeapYear testDefaultStringToTypeConvertInvalidNonLeapYearFeb29 testDefaultStringToTypeConvertValidBoundaryDates testDefaultStringToTypeConvertStrictValidationMessage In SqlTimeConverterTest testDefaultStringToTypeConvertValidBoundaryTimes testDefaultStringToTypeConvertInvalidTimeHourMinuteSecond testDefaultStringToTypeConvertStrictValidationMessage In SqlTimestampConverterTest testDefaultStringToTypeConvertValidTimestampWithFraction testDefaultStringToTypeConvertInvalidTimestampOutOfRangeDate testDefaultStringToTypeConvertInvalidTimestampOutOfRangeTime testDefaultStringToTypeConvertStrictValidationMessage These tests cover valid leap-year and boundary values, invalid out-of-range values for all three SQL types, timestamp fraction handling and verification that the new ConversionException messages mention strict validation.
|
Hello @rootvector2 I pushed patched up version of this PR. I think this PR can be closed unless you see follow up work needed. The problematic bug I fixed is three instances of double parsing. Instead of going round and round, I applied the diff locally, made the following changes, and pushed to master. Double parsing for the Validation parses with the strict formatter and valueOf parses again. Functionally correct, but wasteful. For example: The parsed LocalDate is discarded. A more efficient form would be Double parsing of for the Double parsing of for the I added details to Javadocs to clarify intent. I added the follow unit tests: In SqlDateConverterTest testDefaultStringToTypeConvertValidLeapYear In SqlTimeConverterTest testDefaultStringToTypeConvertValidBoundaryTimes In SqlTimestampConverterTest testDefaultStringToTypeConvertValidTimestampWithFraction These tests cover valid leap-year and boundary values, invalid out-of-range values for all three SQL types, timestamp fraction handling and verification that the new ConversionException messages mention strict validation. You are credited in |
2 participants
DateTimeConverter.toDatebuildsjava.sql.Date/Time/TimestampwithvalueOf, which silently rolls over out-of-range fields (2006-02-31->2006-03-03,25:70:90->02:11:30,2006-10-23 25:70:90-> next day), so malformed input in the defaultuseLocaleFormat=falsepath is accepted as a different value instead of rejected; found while auditing the sql converters against the strictsetLenient(false)/ParsePositioncheck the same class already applies on its locale-format branch, and fixed by validating against strict JDBC-formatDateTimeFormatters (ResolverStyle.STRICT) beforevalueOf, leaving valid padded and single-digit input unchanged.mvn; that'smvnon the command line by itself.