Fixed bug #62379 failing ODBC long column functionality

Matt Ficken · weltling · commit 10251b20c3be · 2012-07-13T15:34:00.000+02:00
diff --git a/ext/pdo/tests/pdo_test.inc b/ext/pdo/tests/pdo_test.inc
@@ -66,13 +66,19 @@ class PDOTest {
 	}
 
 	static function test_factory($file) {
-		$data = file_get_contents($file);
-		$data = preg_replace('/^.*--REDIRECTTEST--/s', '', $data);
-		$config = eval($data);
+		$config = self::get_config($file);
 		foreach ($config['ENV'] as $k => $v) {
 			putenv("$k=$v");
 		}
 		return self::factory();
 	}
+
+	static function get_config($file) {
+		$data = file_get_contents($file);
+		$data = preg_replace('/^.*--REDIRECTTEST--/s', '', $data);
+		$config = eval($data);
+
+		return $config;
+	}
 }
 ?>
diff --git a/ext/pdo_odbc/odbc_stmt.c b/ext/pdo_odbc/odbc_stmt.c
@@ -633,58 +633,49 @@ static int odbc_stmt_get_col(pdo_stmt_t *stmt, int colno, char **ptr, unsigned l
 		}
 
 		if (rc == SQL_SUCCESS_WITH_INFO) {
-			/* promote up to a bigger buffer */
-
-			if (C->fetched_len != SQL_NO_TOTAL) {
-				/* use size suggested by the driver, if it knows it */
-				buf = emalloc(C->fetched_len + 1);
-				memcpy(buf, C->data, C->fetched_len);
-				buf[C->fetched_len] = 0;
-				used = C->fetched_len;
-			} else {
-				buf = estrndup(C->data, 256);
-				used = 255; /* not 256; the driver NUL terminated the buffer */
-			}
-
+			/* this is a 'long column'
+			
+			 read the column in 255 byte blocks until the end of the column is reached, reassembling those blocks
+			 in order into the output buffer
+			
+			 this loop has to work whether or not SQLGetData() provides the total column length.
+			 calling SQLDescribeCol() or other, specifically to get the column length, then doing a single read
+			 for that size would be slower except maybe for extremely long columns.*/
+			char *buf2;
+
+			buf2 = emalloc(256);
+			buf = estrndup(C->data, 256);
+			used = 255; /* not 256; the driver NUL terminated the buffer */
+			
 			do {
 				C->fetched_len = 0;
-				rc = SQLGetData(S->stmt, colno+1, SQL_C_CHAR,
-					buf + used, alloced - used,
-					&C->fetched_len);
-
-				if (rc == SQL_NO_DATA) {
-					/* we got the lot */
-					break;
-				} else if (rc != SQL_SUCCESS) {
-					pdo_odbc_stmt_error("SQLGetData");
-					if (rc != SQL_SUCCESS_WITH_INFO) {
-						break;
-					}
-				}
-
-				if (C->fetched_len == SQL_NO_TOTAL) {
-					used += alloced - used;
+				/* read block. 256 bytes => 255 bytes are actually read, the last 1 is NULL */
+				rc = SQLGetData(S->stmt, colno+1, SQL_C_CHAR, buf2, 256, &C->fetched_len);
+				
+				/* resize output buffer and reassemble block */
+				if (rc==SQL_SUCCESS_WITH_INFO) {
+					/* point 5, in section "Retrieving Data with SQLGetData" in http://msdn.microsoft.com/en-us/library/windows/desktop/ms715441(v=vs.85).aspx
+					 states that if SQL_SUCCESS_WITH_INFO, fetched_len will be > 255 (greater than buf2's size)
+					 (if a driver fails to follow that and wrote less than 255 bytes to buf2, this will AV or read garbage into buf) */
+					buf = erealloc(buf, used + 255+1);
+					memcpy(buf + used, buf2, 255);
+					used = used + 255;
+				} else if (rc==SQL_SUCCESS) {
+					buf = erealloc(buf, used + C->fetched_len+1);
+					memcpy(buf + used, buf2, C->fetched_len);
+					used = used + C->fetched_len;
 				} else {
-					used += C->fetched_len;
-				}
-
-				if (rc == SQL_SUCCESS) {
-					/* this was the final fetch */
+					/* includes SQL_NO_DATA */
 					break;
 				}
-
-				/* we need to fetch another chunk; resize the
-				 * buffer */
-				alloced *= 2;
-				buf = erealloc(buf, alloced);
+				
 			} while (1);
-
-			/* size down */
-			if (used < alloced - 1024) {
-				alloced = used+1;
-				buf = erealloc(buf, used+1);
-			}
+			
+			efree(buf2);
+			
+			/* NULL terminate the buffer once, when finished, for use with the rest of PHP */
 			buf[used] = '\0';
+
 			*ptr = buf;
 			*caller_frees = 1;
 			*len = used;
diff --git a/ext/pdo_odbc/tests/common.phpt b/ext/pdo_odbc/tests/common.phpt
@@ -2,28 +2,54 @@
 ODBC
 --SKIPIF--
 <?php # vim:ft=php
-if (!extension_loaded('pdo_odbc')) print 'skip'; ?>
+if (!extension_loaded('pdo_odbc')) print 'skip';
+if (substr(PHP_OS, 0, 3) == 'WIN' &&
+	false === getenv('PDOTEST_DSN') &&
+	false === getenv('PDO_ODBC_TEST_DSN') &&
+	!extension_loaded('com_dotnet')) {
+	die('skip - either PDOTEST_DSN or com_dotnet extension is needed to setup the connection');
+}
 --REDIRECTTEST--
 # magic auto-configuration
 
 $config = array(
-	'TESTS' => 'ext/pdo/tests'
+	'TESTS' => 'ext/pdo/tests',
+	'ENV' => array()
 );
 	
-
-if (false !== getenv('PDO_ODBC_TEST_DSN')) {
-	# user set them from their shell
+// try loading PDO driver using ENV vars and if none given, and on Windows, try using MS Access
+// and if not, skip the test
+//
+// try to use common PDO env vars, instead of PDO_ODBC specific
+if (false !== getenv('PDOTEST_DSN')) {
+	// user should have to set PDOTEST_DSN so that:
+	// 1. test is skipped if user doesn't want to test it, even if they have MS Access installed
+	// 2. it detects if ODBC driver is not installed - to avoid test bug
+	// 3. it detects if ODBC driver is installed - so test will be run
+	// 4. so a specific ODBC driver can be tested - if system has multiple ODBC drivers
+	
+	$config['ENV']['PDOTEST_DSN'] = getenv('PDOTEST_DSN');
+	$config['ENV']['PDOTEST_USER'] = getenv('PDOTEST_USER');
+	$config['ENV']['PDOTEST_PASS'] = getenv('PDOTEST_PASS');
+	if (false !== getenv('PDOTEST_ATTR')) {
+		$config['ENV']['PDOTEST_ATTR'] = getenv('PDOTEST_ATTR');
+	}
+} else if (false !== getenv('PDO_ODBC_TEST_DSN')) {
+	// user set these from their shell instead
 	$config['ENV']['PDOTEST_DSN'] = getenv('PDO_ODBC_TEST_DSN');
 	$config['ENV']['PDOTEST_USER'] = getenv('PDO_ODBC_TEST_USER');
 	$config['ENV']['PDOTEST_PASS'] = getenv('PDO_ODBC_TEST_PASS');
 	if (false !== getenv('PDO_ODBC_TEST_ATTR')) {
 		$config['ENV']['PDOTEST_ATTR'] = getenv('PDO_ODBC_TEST_ATTR');
 	}
 } elseif (preg_match('/^WIN/i', PHP_OS)) {
-	# on windows, try to create a temporary MS access database
+	// on Windows and user didn't set PDOTEST_DSN, try this as a fallback:
+	// check if MS Access DB is installed, and if yes, try using it. create a temporary MS access database.
+	//
 	$path = realpath(dirname(__FILE__)) . '\pdo_odbc.mdb';
 	if (!file_exists($path)) {
 		try {
+			// try to create database
 			$adox = new COM('ADOX.Catalog');
 			$adox->Create('Provider=Microsoft.Jet.OLEDB.4.0;Data Source=' . $path);
 			$adox = null;
@@ -32,9 +58,12 @@ if (false !== getenv('PDO_ODBC_TEST_DSN')) {
 		}
 	}
 	if (file_exists($path)) {
+		// database was created and written to file system
 		$config['ENV']['PDOTEST_DSN'] = "odbc:Driver={Microsoft Access Driver (*.mdb)};Dbq=$path;Uid=Admin";
-	}
-}
+	} // else: $config['ENV']['PDOTEST_DSN'] not set
+} // else: $config['ENV']['PDOTEST_DSN'] not set
+//         test will be skipped. see SKIPIF section of long_columns.phpt
+
 # other magic autodetection here, eg: for DB2 by inspecting env
 /*
 $USER = 'db2inst1';
diff --git a/ext/pdo_odbc/tests/long_columns.phpt b/ext/pdo_odbc/tests/long_columns.phpt
@@ -3,9 +3,44 @@ PDO ODBC "long" columns
 --SKIPIF--
 <?php # vim:ft=php
 if (!extension_loaded('pdo_odbc')) print 'skip not loaded';
+// make sure there is an ODBC driver and a DSN, or the test will fail
+include 'ext/pdo/tests/pdo_test.inc';
+$config = PDOTest::get_config('ext/pdo_odbc/tests/common.phpt');
+if (!isset($config['ENV']['PDOTEST_DSN']) || $config['ENV']['PDOTEST_DSN']===false) print 'skip';
 ?>
 --FILE--
 <?php
+// setup: set PDOTEST_DSN environment variable
+//        for MyODBC (MySQL) and MS SQL Server, you need to also set PDOTEST_USER and PDOTEST_PASS
+//
+// can use MS SQL Server on Linux - using unixODBC
+//   -RHEL6.2
+//   -download & instructions: http://www.microsoft.com/en-us/download/details.aspx?id=28160
+//      -Linux6\sqlncli-11.0.1790.0.tar.gz (it calls RHEL6.x 'Linux6' for some reason)
+//   -follow instructions on web page and install script
+//   -may have to specify connection info in connection string without using a DSN (DSN-less connection)
+//      -for example:
+//            set PDOTEST_DSN='odbc:Driver=SQL Server Native Client 11.0;Server=10.200.51.179;Database=testdb'
+//            set PDOTEST_USER=sa
+//            set PDOTEST_PASS=Password01
+//
+// on Windows, the easy way to do this:
+// 1. install MS Access (part of MS Office) and include ODBC (Development tools feature)
+//       install the x86 build of the Drivers. You might not be able to load the x64 drivers.
+// 2. in Control Panel, search for ODBC and open "Setup data sources (ODBC)"
+// 3. click on System DSN tab
+// 4. click Add and choose "Microsoft Access Driver (*.mdb, *.accdb)" driver
+// 5. enter a DSN, ex: accdb12
+// 6. click 'Create' and select a file to save the database as
+//       -otherwise, you'll have to open MS Access, create a database, then load that file in this Window to map it to a DSN
+// 7. set the environment variable PDOTEST_DSN="odbc:<system dsn from step 5>" ex: SET PDOTEST_DSN=odbc:accdb12
+//         -note: on Windows, " is included in environment variable
+// 
+// easy way to compile:
+// configure --disable-all --enable-cli --enable-zts --enable-pdo --with-pdo-odbc --enable-debug
+// configure --disable-all --eanble-cli --enable-pdo --with-pdo-odbc=unixODBC,/usr,/usr --with-unixODBC=/usr --enable-debug
+//
+
 require 'ext/pdo/tests/pdo_test.inc';
 $db = PDOTest::test_factory('ext/pdo_odbc/tests/common.phpt');
 $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT);
@@ -20,27 +55,86 @@ if (false === $db->exec('CREATE TABLE TEST (id INT NOT NULL PRIMARY KEY, data CL
 
 $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
 
-$sizes = array(32, 64, 128, 253, 254, 255, 256, 257, 258, 512, 1024, 2048, 3998, 3999, 4000);
+// the driver reads columns in blocks of 255 bytes and then reassembles those blocks into a single buffer.
+// test sizes around 255 to make sure that the reassembly works (and that the column is split into 255 byte blocks by the database)
+// also, test sizes below 255 to make sure that they work - and are not treated as a long column (should be read in a single read)
+$sizes = array(32, 53, 64, 79, 128, 253, 254, 255, 256, 257, 258, 1022, 1023, 1024, 1025, 1026, 510, 511, 512, 513, 514, 1278, 1279, 1280, 1281, 1282, 2046, 2047, 2048, 2049, 2050, 1534, 1535, 1536, 1537, 1538, 3070, 3071, 3072, 3073, 3074, 3998, 3999, 4000);
 
-$db->beginTransaction();
-$insert = $db->prepare('INSERT INTO TEST VALUES (?, ?)');
+function alpha_repeat($len) {
+	// use the alphabet instead of 'i' characters to make sure the blocks don't overlap when they are reassembled
+	$out = "";
+	while (strlen($out) < $len) {
+		$out .= "abcdefghijklmnopqrstuvwxyz";
+	}
+	return substr($out, 0, $len);
+}
+
+// don't use Prepared Statements. that fails on MS SQL server (works with Access, MyODBC), which is a separate failure, feature/code-path from what
+// this test does - nice to be able to test using MS SQL server
 foreach ($sizes as $num) {
-	$insert->execute(array($num, str_repeat('i', $num)));
+	$text = alpha_repeat($num);
+	$db->exec("INSERT INTO TEST VALUES($num, '$text')");
 }
-$insert = null;
-$db->commit();
 
+// verify data
 foreach ($db->query('SELECT id, data from TEST') as $row) {
-	$expect = str_repeat('i', $row[0]);
+	$expect = alpha_repeat($row[0]);
 	if (strcmp($expect, $row[1])) {
 		echo "Failed on size $row[id]:\n";
 		printf("Expected %d bytes, got %d\n", strlen($expect), strlen($row['data']));
-		echo bin2hex($expect) . "\n";
-		echo bin2hex($row['data']) . "\n";
+		echo ($expect) . "\n";
+		echo ($row['data']) . "\n";
+	} else {
+		echo "Passed on size $row[id]\n";
 	}
 }
 
 echo "Finished\n";
 
 --EXPECT--
+Passed on size 32
+Passed on size 53
+Passed on size 64
+Passed on size 79
+Passed on size 128
+Passed on size 253
+Passed on size 254
+Passed on size 255
+Passed on size 256
+Passed on size 257
+Passed on size 258
+Passed on size 1022
+Passed on size 1023
+Passed on size 1024
+Passed on size 1025
+Passed on size 1026
+Passed on size 510
+Passed on size 511
+Passed on size 512
+Passed on size 513
+Passed on size 514
+Passed on size 1278
+Passed on size 1279
+Passed on size 1280
+Passed on size 1281
+Passed on size 1282
+Passed on size 2046
+Passed on size 2047
+Passed on size 2048
+Passed on size 2049
+Passed on size 2050
+Passed on size 1534
+Passed on size 1535
+Passed on size 1536
+Passed on size 1537
+Passed on size 1538
+Passed on size 3070
+Passed on size 3071
+Passed on size 3072
+Passed on size 3073
+Passed on size 3074
+Passed on size 3998
+Passed on size 3999
+Passed on size 4000
 Finished
+

Original file line number	Diff line number	Diff line change
`@@ -66,13 +66,19 @@ class PDOTest {`
`66`	`66`	`}`
`67`	`67`
`68`	`68`	`static function test_factory($file) {`
`69`		`- $data = file_get_contents($file);`
`70`		`- $data = preg_replace('/^.*--REDIRECTTEST--/s', '', $data);`
`71`		`- $config = eval($data);`
	`69`	`+ $config = self::get_config($file);`
`72`	`70`	`foreach ($config['ENV'] as $k => $v) {`
`73`	`71`	`putenv("$k=$v");`
`74`	`72`	`}`
`75`	`73`	`return self::factory();`
`76`	`74`	`}`
	`75`	`+`
	`76`	`+ static function get_config($file) {`
	`77`	`+ $data = file_get_contents($file);`
	`78`	`+ $data = preg_replace('/^.*--REDIRECTTEST--/s', '', $data);`
	`79`	`+ $config = eval($data);`
	`80`	`+`
	`81`	`+ return $config;`
	`82`	`+ }`
`77`	`83`	`}`
`78`	`84`	`?>`