FirebirdSQL / NETProvider

Firebird ADO.NET Data Provider
https://www.firebirdsql.org/en/net-provider/
Other
152 stars 62 forks source link

DbValue.GetDateTime throws a FormatException when converting datetime string to a DateTime value. #1173

Open deAtog opened 3 weeks ago

deAtog commented 3 weeks ago

Under certain circumstances, the call to Convert.ToDateTime in this method will throw a FormatException when attempting to retrieve a DateTime value inserted into a text column: https://github.com/FirebirdSQL/NETProvider/blob/73b0082bd253efb6e46cdb28b4f558632f431815/src/FirebirdSql.Data.FirebirdClient/Common/DbValue.cs#L246-L255

Steps to reproduce this issue are as follows:

  1. Create at table with a text column.
  2. Set the current locale to the invariant culture using: CultureInfo.CurrentCulture = CultureInfo.InvariantCulture;
  3. Insert a DateTime value with the date of "2024-02-29 00:00:00" into the column using a parameterized query with a DateTime parameter.
  4. Change the locale to ar-SA using: CultureInfo.CurrentCulture = CultureInfo.GetCulture("ar-SA");
  5. Select the text value from the table without casting the value in the query to date/time type.
  6. Use FbDataReader.GetDateTime() to retrieve the value and observe the exception.

A format exception is generated because the call to Convert.ToDateTime uses the current culture of the calling application and does not correctly parse the value that was stored in the database. This method should either attempt a direct cast to a DateTime and throw an InvalidCastException or perform an implict conversion from the string value to a DateTime value using the inverse conversion as performed by the database upon insert. The latter provides consistent insert and select functionality.

It may be sufficient to use the InvariantCulture in the call to Convert.ToDateTime here, but I personally do not know how Firebase converts DateTime values to text. Additional tests may be required for other locale combinations to ensure correct functionality.

cincuranet commented 3 weeks ago

The usage of CurrentCulture is certainly wrong. On the other hand this method is not expected to work with strings. If you store DateTime in database as a string, you're doing it wrong. Firebird has dedicated datatype(s) for date/time.

deAtog commented 3 weeks ago

Obviously it is not be advisable to store a DateTime in a text column. However there are likely cases where users have done so. When given a DateTime object as a parameter to insert, Firebase silently converts the value to text. The expectation here is that the driver should be able to reverse the conversion from text when asked to do so.