From 97ec441678d2d3ff672b8c3d5357ca19e48116ef Mon Sep 17 00:00:00 2001 From: Markus Klein Date: Mon, 25 Sep 2023 23:11:20 +0200 Subject: [PATCH] fix false positive truncation for varchar --- Cargo.lock | 4 +- Changelog.md | 4 + odbc-api/Cargo.toml | 2 +- odbc-api/src/buffers/text_column.rs | 5 +- odbc-api/src/cursor.rs | 14 ++- odbc-api/src/narrow.rs | 133 ++++++++++++++-------------- odbc-api/tests/integration.rs | 35 +++++--- odbcsv/Cargo.toml | 4 +- odbcsv/Changelog.md | 4 + 9 files changed, 117 insertions(+), 88 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f04000a2..628be58a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -870,7 +870,7 @@ dependencies = [ [[package]] name = "odbc-api" -version = "1.0.1" +version = "1.0.2" dependencies = [ "anyhow", "criterion", @@ -897,7 +897,7 @@ checksum = "592c5c4ce58f47dde428c52b39904252191d25436b410cc232fae0813ff58f08" [[package]] name = "odbcsv" -version = "1.0.0" +version = "1.0.1" dependencies = [ "anyhow", "assert_cmd", diff --git a/Changelog.md b/Changelog.md index 21f298db..ee16ce4d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,9 @@ # Changelog +## 1.0.2 + +* Fix: `Cursor::fetch_with_truncation_check` did erroneously report truncation errors for `NULL` fields, if the query also triggered any ODBC diagnostic. This has been fixed in the previous release for variadic binary column buffers but a similar failure had been missed for variadic text columns. + ## 1.0.1 * Fix: `Cursor::fetch_with_truncation_check` did erroneously report truncation errors for `NULL` fields, if the query also triggered any ODBC diagnostic. This remained undedected for a while, because in every situation that there is a truncation, there must also be a diagnostic according to the ODBC standard. However the reverse is not true. In this situation a `NULL` had been erronously treated the same as a `NO_TOTAL` indicator and a false positive has been reported to a user claiming that there is a truncation, there in fact is none. diff --git a/odbc-api/Cargo.toml b/odbc-api/Cargo.toml index bb9f2401..9820a5f3 100644 --- a/odbc-api/Cargo.toml +++ b/odbc-api/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "odbc-api" -version = "1.0.1" +version = "1.0.2" authors = ["Markus Klein"] edition = "2021" license = "MIT" diff --git a/odbc-api/src/buffers/text_column.rs b/odbc-api/src/buffers/text_column.rs index 145a1a8b..96876148 100644 --- a/odbc-api/src/buffers/text_column.rs +++ b/odbc-api/src/buffers/text_column.rs @@ -336,10 +336,7 @@ where .iter() .copied() .take(num_rows) - .any(|indicator| match Indicator::from_isize(indicator) { - Indicator::Null | Indicator::NoTotal => true, - Indicator::Length(length_in_bytes) => max_bin_length < length_in_bytes, - }) + .any(|indicator| Indicator::from_isize(indicator).is_truncated(max_bin_length)) } } diff --git a/odbc-api/src/cursor.rs b/odbc-api/src/cursor.rs index 6d9bdabf..48250a51 100644 --- a/odbc-api/src/cursor.rs +++ b/odbc-api/src/cursor.rs @@ -139,7 +139,11 @@ impl<'s> CursorRow<'s> { /// /// `true` indicates that the value has not been `NULL` and the value has been placed in `buf`. /// `false` indicates that the value is `NULL`. The buffer is cleared in that case. - pub fn get_wide_text(&mut self, col_or_param_num: u16, buf: &mut Vec) -> Result { + pub fn get_wide_text( + &mut self, + col_or_param_num: u16, + buf: &mut Vec, + ) -> Result { self.get_variadic::(col_or_param_num, buf) } @@ -198,8 +202,7 @@ impl<'s> CursorRow<'s> { let old_len = buf.len(); // Use an exponential strategy for increasing buffer size. buf.resize(old_len * 2, K::ZERO); - let buf_extend = - &mut buf[(old_len - K::TERMINATING_ZEROES)..]; + let buf_extend = &mut buf[(old_len - K::TERMINATING_ZEROES)..]; target = VarCell::<&mut [K::Element], K>::from_buffer( buf_extend, Indicator::NoTotal, @@ -224,7 +227,10 @@ impl<'s> CursorRow<'s> { let old_len = buf.len(); buf.resize(old_len + still_missing, K::ZERO); let buf_extend = &mut buf[(old_len - K::TERMINATING_ZEROES)..]; - target = VarCell::<&mut [K::Element], K>::from_buffer(buf_extend, Indicator::NoTotal); + target = VarCell::<&mut [K::Element], K>::from_buffer( + buf_extend, + Indicator::NoTotal, + ); } } // Fetch binary data into buffer. diff --git a/odbc-api/src/narrow.rs b/odbc-api/src/narrow.rs index 39033a6b..084cedaf 100644 --- a/odbc-api/src/narrow.rs +++ b/odbc-api/src/narrow.rs @@ -1,65 +1,68 @@ -use crate::{IntoParameter, parameter::{VarCharSlice, VarCharBox}}; - -/// Newtype wrapper intend to be used around `String`s or `str` slices to bind them always as narrow -/// text independent of wether the `narrow` feature is set or not. -pub struct Narrow(pub T); - -impl<'a> IntoParameter for Narrow<&'a str> { - type Parameter = VarCharSlice<'a>; - - fn into_parameter(self) -> Self::Parameter { - VarCharSlice::new(self.0.as_bytes()) - } -} - -impl<'a> IntoParameter for Narrow> { - type Parameter = VarCharSlice<'a>; - - fn into_parameter(self) -> Self::Parameter { - match self.0 { - Some(str) => Narrow(str).into_parameter(), - None => VarCharSlice::NULL, - } - } -} - -impl<'a> IntoParameter for Option> { - type Parameter = VarCharSlice<'a>; - - fn into_parameter(self) -> Self::Parameter { - match self { - Some(str) => Narrow(str.0).into_parameter(), - None => VarCharSlice::NULL, - } - } -} - -impl IntoParameter for Narrow { - type Parameter = VarCharBox; - - fn into_parameter(self) -> Self::Parameter { - VarCharBox::from_string(self.0) - } -} - -impl IntoParameter for Narrow> { - type Parameter = VarCharBox; - - fn into_parameter(self) -> Self::Parameter { - match self.0 { - Some(str) => Narrow(str).into_parameter(), - None => VarCharBox::null(), - } - } -} - -impl IntoParameter for Option> { - type Parameter = VarCharBox; - - fn into_parameter(self) -> Self::Parameter { - match self { - Some(str) => Narrow(str.0).into_parameter(), - None => VarCharBox::null(), - } - } -} \ No newline at end of file +use crate::{ + parameter::{VarCharBox, VarCharSlice}, + IntoParameter, +}; + +/// Newtype wrapper intend to be used around `String`s or `str` slices to bind them always as narrow +/// text independent of wether the `narrow` feature is set or not. +pub struct Narrow(pub T); + +impl<'a> IntoParameter for Narrow<&'a str> { + type Parameter = VarCharSlice<'a>; + + fn into_parameter(self) -> Self::Parameter { + VarCharSlice::new(self.0.as_bytes()) + } +} + +impl<'a> IntoParameter for Narrow> { + type Parameter = VarCharSlice<'a>; + + fn into_parameter(self) -> Self::Parameter { + match self.0 { + Some(str) => Narrow(str).into_parameter(), + None => VarCharSlice::NULL, + } + } +} + +impl<'a> IntoParameter for Option> { + type Parameter = VarCharSlice<'a>; + + fn into_parameter(self) -> Self::Parameter { + match self { + Some(str) => Narrow(str.0).into_parameter(), + None => VarCharSlice::NULL, + } + } +} + +impl IntoParameter for Narrow { + type Parameter = VarCharBox; + + fn into_parameter(self) -> Self::Parameter { + VarCharBox::from_string(self.0) + } +} + +impl IntoParameter for Narrow> { + type Parameter = VarCharBox; + + fn into_parameter(self) -> Self::Parameter { + match self.0 { + Some(str) => Narrow(str).into_parameter(), + None => VarCharBox::null(), + } + } +} + +impl IntoParameter for Option> { + type Parameter = VarCharBox; + + fn into_parameter(self) -> Self::Parameter { + match self { + Some(str) => Narrow(str.0).into_parameter(), + None => VarCharBox::null(), + } + } +} diff --git a/odbc-api/tests/integration.rs b/odbc-api/tests/integration.rs index 339a2a5d..c03afc96 100644 --- a/odbc-api/tests/integration.rs +++ b/odbc-api/tests/integration.rs @@ -18,7 +18,7 @@ use odbc_api::{ Blob, BlobRead, BlobSlice, VarBinaryArray, VarCharArray, VarCharSlice, WithDataType, }, sys, Bit, ColumnDescription, Connection, ConnectionOptions, Cursor, DataType, Error, InOut, - IntoParameter, Nullability, Nullable, Out, ResultSetMetadata, U16Str, U16String, Narrow, + IntoParameter, Narrow, Nullability, Nullable, Out, ResultSetMetadata, U16Str, U16String, }; use std::{ ffi::CString, @@ -1462,23 +1462,32 @@ fn bind_narrow_parameter_to_varchar(profile: &Profile) { .unwrap(); conn.execute(&insert_sql, &Some(Narrow("Hello")).into_parameter()) .unwrap(); - conn.execute(&insert_sql, &None::>.into_parameter()) + conn.execute(&insert_sql, &None::>.into_parameter()) .unwrap(); // String conn.execute(&insert_sql, &Narrow("Hello".to_string()).into_parameter()) .unwrap(); // Option String - conn.execute(&insert_sql, &Narrow(Some("Hello".to_string())).into_parameter()) - .unwrap(); + conn.execute( + &insert_sql, + &Narrow(Some("Hello".to_string())).into_parameter(), + ) + .unwrap(); conn.execute(&insert_sql, &Narrow(None::).into_parameter()) .unwrap(); - conn.execute(&insert_sql, &Some(Narrow("Hello".to_string())).into_parameter()) - .unwrap(); - conn.execute(&insert_sql, &None::>.into_parameter()) + conn.execute( + &insert_sql, + &Some(Narrow("Hello".to_string())).into_parameter(), + ) + .unwrap(); + conn.execute(&insert_sql, &None::>.into_parameter()) .unwrap(); let actual = table.content_as_string(&conn); - assert_eq!("Hello\nHello\nNULL\nHello\nNULL\nHello\nHello\nNULL\nHello\nNULL", actual); + assert_eq!( + "Hello\nHello\nNULL\nHello\nNULL\nHello\nHello\nNULL\nHello\nNULL", + actual + ); } #[test_case(MSSQL; "Microsoft SQL Server")] @@ -3483,7 +3492,10 @@ fn detect_truncated_output_in_bulk_fetch(profile: &Profile) { let query = format!("SELECT a FROM {table_name}"); let cursor = conn.execute(&query, ()).unwrap().unwrap(); let mut cursor = cursor.bind_buffer(buffer).unwrap(); - assert!(matches!(cursor.fetch_with_truncation_check(true), Err(Error::TooLargeValueForBuffer))) + assert!(matches!( + cursor.fetch_with_truncation_check(true), + Err(Error::TooLargeValueForBuffer) + )) } #[test_case(MSSQL; "Microsoft SQL Server")] @@ -3855,7 +3867,10 @@ fn cursor_get_text_from_text(profile: &Profile) { conn.execute(&insert_sql, &text.into_parameter()).unwrap(); // When - let mut cursor = conn.execute(&table.sql_all_ordered_by_id(), ()).unwrap().unwrap(); + let mut cursor = conn + .execute(&table.sql_all_ordered_by_id(), ()) + .unwrap() + .unwrap(); let mut row = cursor.next_row().unwrap().unwrap(); let mut buffer = Vec::new(); row.get_text(1, &mut buffer).unwrap(); diff --git a/odbcsv/Cargo.toml b/odbcsv/Cargo.toml index dbef1e5d..f56cda2a 100644 --- a/odbcsv/Cargo.toml +++ b/odbcsv/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "odbcsv" -version = "1.0.0" +version = "1.0.1" authors = ["Markus Klein"] edition = "2021" license = "MIT" @@ -29,7 +29,7 @@ readme = "Readme.md" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -odbc-api = { version = "1.0.1", path = "../odbc-api" } +odbc-api = { version = "1.0.2", path = "../odbc-api" } csv = "1.2.2" anyhow = "1.0.75" stderrlog = "0.5.4" diff --git a/odbcsv/Changelog.md b/odbcsv/Changelog.md index 43f3237b..dd978bff 100644 --- a/odbcsv/Changelog.md +++ b/odbcsv/Changelog.md @@ -1,5 +1,9 @@ # Changelog +## 1.0.1 + +* Fixed an issue which could cause errouneous report of truncation in presence of other warnings. This has been fixed in the previous release for variadic binary column buffers but a similar failure had been missed for variadic text columns. + ## 1.0.0 * Updated dependencies