Skip to content

Commit 6f911c2

Browse files
jahnvi480jahnvi480
committed
Fix #1599: Remove SQLCancel from core_sqlsrv_next_result for user-facing APIs
When SQLMoreResults returns SQL_ERROR for a mid-batch statement failure (e.g. divide-by-zero with XACT_ABORT OFF), the ODBC spec says it should return SQL_SUCCESS_WITH_INFO when the batch is not aborted and the failed statement is not the last one. The Microsoft ODBC driver incorrectly returns SQL_ERROR, but the statement handle remains valid. The old code called SQLCancel() in the catch block, which aborted the entire remaining batch, making subsequent result sets unreachable. This change adds a throw_on_errors parameter (default true) to core_sqlsrv_next_result: - throw_on_errors=true (default): used by internal flush loops (closeCursor, re-execute, param binding). Uses the throwing core::SQLMoreResults wrapper and calls SQLCancel in the catch block. Same behavior as before. - throw_on_errors=false: used only by sqlsrv_next_result() and PDO::nextRowset(). Calls SQLMoreResults directly, reports the error through the normal error handler, clears any pending PDO exception, and falls through to new_result_set(). The batch remains navigable. SQLCancel is NOT called in the catch block for this path. Tests cover both extensions (ERRMODE_WARNING and ERRMODE_EXCEPTION for PDO), re-execute after a batch error (flush loop), and error visibility via sqlsrv_errors() / errorInfo(). Fixes #1599
2 parents a2d3fcc + 1d3bc9f commit 6f911c2

15 files changed

Lines changed: 187 additions & 76 deletions

‎source/shared/core_stmt.cpp‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,11 +1035,15 @@ void core_sqlsrv_next_result( _Inout_ sqlsrv_stmt* stmt, _In_ bool finalize_outp
10351035
}
10361036
catch( core::CoreException& e ) {
10371037

1038-
// Do not call SQLCancel here. When SQLMoreResults returns SQL_ERROR
1039-
// for a mid-batch statement failure (e.g. divide-by-zero with
1040-
// XACT_ABORT OFF), the statement handle is still valid and the user
1041-
// should be able to call next_result again to reach subsequent result
1042-
// sets. Calling SQLCancel would abort the entire remaining batch.
1038+
// For internal callers (throw_on_errors=true — flush loops,
1039+
// closeCursor, param binding) we still call SQLCancel to clean up
1040+
// the ODBC handle on error. For user-facing callers
1041+
// (throw_on_errors=false — sqlsrv_next_result / PDO::nextRowset)
1042+
// we must NOT cancel because the handle is still valid and the
1043+
// batch remains navigable after a mid-batch statement failure.
1044+
if( throw_on_errors ) {
1045+
SQLCancel( stmt->handle() );
1046+
}
10431047
throw e;
10441048
}
10451049
}

‎test/functional/pdo_sqlsrv/PDO_ConnPool_Unix.phpt‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ if (extension_loaded('pdo_odbc')) {
1313
?>
1414
--FILE--
1515
<?php
16+
// Prevent 'sh: warning: setlocale: LC_ALL: cannot change locale' on systems
17+
// where en_US.UTF-8 is not installed (e.g., Red Hat containers)
18+
putenv('LC_ALL=C');
19+
1620
function findODBCDriver($content, $lines_to_add)
1721
{
1822
require_once('MsSetup.inc');
@@ -25,8 +29,29 @@ function findODBCDriver($content, $lines_to_add)
2529
$lines_to_add="CPTimeout=5\n[ODBC]\nPooling=Yes\n";
2630

2731
//get default odbcinst.ini location
28-
$lines = explode("\n", shell_exec("odbcinst -j"));
29-
$odbcinst_ini = explode(" ", $lines[1])[1];
32+
$output = shell_exec("odbcinst -j 2>/dev/null");
33+
$odbcinst_ini = '';
34+
if ($output) {
35+
foreach (explode("\n", $output) as $line) {
36+
if (stripos($line, 'DRIVERS') !== false) {
37+
// Handle both "DRIVERS: /path" and "DRIVERS............: /path" formats
38+
$parts = explode(":", $line, 2);
39+
$path = trim($parts[1] ?? '');
40+
if ($path && file_exists($path)) {
41+
$odbcinst_ini = $path;
42+
break;
43+
}
44+
}
45+
}
46+
}
47+
// Fallback to well-known default path
48+
if (empty($odbcinst_ini) || !file_exists($odbcinst_ini)) {
49+
if (file_exists('/etc/odbcinst.ini')) {
50+
$odbcinst_ini = '/etc/odbcinst.ini';
51+
} else {
52+
die("Could not determine odbcinst.ini location");
53+
}
54+
}
3055
$custom_odbcinst_ini = dirname(__FILE__)."/odbcinst.ini";
3156

3257
//copy the default odbcinst.ini into the current folder

‎test/functional/pdo_sqlsrv/pdo_connect_encrypt_attributes.phpt‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,18 @@ This test does not test if any connection is successful but mainly test if the E
55
different attributes.
66
--SKIPIF--
77
<?php require('skipif_mid-refactor.inc');
8-
$conn = connect();
9-
$stmt = $conn->query("SELECT SERVERPROPERTY('Edition') AS Edition");
10-
$row = $stmt->fetch(PDO::FETCH_ASSOC);
11-
if ($row && strpos($row['Edition'], 'Express') !== false) {
12-
die("skip LocalDB/Express Edition doesn't support Force Encryption");
8+
try {
9+
$conn = connect();
10+
if (!$conn) {
11+
die("skip Could not connect");
12+
}
13+
$stmt = $conn->query("SELECT SERVERPROPERTY('Edition') AS Edition");
14+
$row = $stmt->fetch(PDO::FETCH_ASSOC);
15+
if ($row && strpos($row['Edition'], 'Express') !== false) {
16+
die("skip LocalDB/Express Edition doesn't support Force Encryption");
17+
}
18+
} catch (Exception $e) {
19+
die("skip " . $e->getMessage());
1320
}
1421
?>
1522
--FILE--

‎test/functional/pdo_sqlsrv/skipif_azure_ad_acess_token.inc‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,12 @@ require_once("MsSetup.inc");
1212
require_once("MsCommon_mid-refactor.inc");
1313

1414
// MsCommon_mid-refactor.inc connect() signature: connect($keywords = '', $options=array(), $errmode, $disableCE)
15-
$conn = connect();
16-
if ($conn === false) {
15+
try {
16+
$conn = connect();
17+
} catch (Exception $e) {
18+
$conn = false;
19+
}
20+
if (!$conn) {
1721
die("skip Could not connect during SKIPIF.");
1822
}
1923

‎test/functional/sqlsrv/sqlsrv_ConnPool_Unix.phpt‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ This test assumes the default odbcinst.ini has not been modified.
77
<?php if (extension_loaded('pdo_odbc')) die('skip pdo_odbc extension forces ODBC pooling, interfering with this test'); ?>
88
--FILE--
99
<?php
10+
// Prevent 'sh: warning: setlocale: LC_ALL: cannot change locale' on systems
11+
// where en_US.UTF-8 is not installed (e.g., Red Hat containers)
12+
putenv('LC_ALL=C');
13+
1014
function findODBCDriver($content, $lines_to_add)
1115
{
1216
require_once('MsSetup.inc');
@@ -19,8 +23,29 @@ function findODBCDriver($content, $lines_to_add)
1923
$lines_to_add="CPTimeout=5\n[ODBC]\nPooling=Yes\n";
2024

2125
//get default odbcinst.ini location
22-
$lines = explode("\n", shell_exec("odbcinst -j"));
23-
$odbcinst_ini = explode(" ", $lines[1])[1];
26+
$output = shell_exec("odbcinst -j 2>/dev/null");
27+
$odbcinst_ini = '';
28+
if ($output) {
29+
foreach (explode("\n", $output) as $line) {
30+
if (stripos($line, 'DRIVERS') !== false) {
31+
// Handle both "DRIVERS: /path" and "DRIVERS............: /path" formats
32+
$parts = explode(":", $line, 2);
33+
$path = trim($parts[1] ?? '');
34+
if ($path && file_exists($path)) {
35+
$odbcinst_ini = $path;
36+
break;
37+
}
38+
}
39+
}
40+
}
41+
// Fallback to well-known default path
42+
if (empty($odbcinst_ini) || !file_exists($odbcinst_ini)) {
43+
if (file_exists('/etc/odbcinst.ini')) {
44+
$odbcinst_ini = '/etc/odbcinst.ini';
45+
} else {
46+
die("Could not determine odbcinst.ini location");
47+
}
48+
}
2449
$custom_odbcinst_ini = dirname(__FILE__)."/odbcinst.ini";
2550

2651
//copy the default odbcinst.ini into the current folder
@@ -34,15 +59,15 @@ file_put_contents($custom_odbcinst_ini, $new_content);
3459
//Creating a new php process, because for changes in odbcinst.ini file to affect pooling, drivers must be reloaded.
3560
//Also setting the odbcini path to the current folder for the same process.
3661
//This will let us modify odbcinst.ini without root permissions
37-
print_r(shell_exec("export ODBCSYSINI=".dirname(__FILE__)."&&".PHP_BINARY." ".dirname(__FILE__)."/isPooled.php"));
62+
print_r(shell_exec("export ODBCSYSINI=".dirname(__FILE__)." && ".PHP_BINARY." ".dirname(__FILE__)."/isPooled.php 2>/dev/null"));
3863

3964

4065
//disable pooling by modifying the odbcinst.ini file
4166
$current = file_get_contents($custom_odbcinst_ini);
4267
$current = str_replace($lines_to_add,'',$current);
4368
file_put_contents($custom_odbcinst_ini, $current);
4469

45-
print_r(shell_exec("export ODBCSYSINI=".dirname(__FILE__)."&&".PHP_BINARY." ".dirname(__FILE__)."/isPooled.php"));
70+
print_r(shell_exec("export ODBCSYSINI=".dirname(__FILE__)." && ".PHP_BINARY." ".dirname(__FILE__)."/isPooled.php 2>/dev/null"));
4671
?>
4772
--CLEAN--
4873
<?php

‎test/functional/sqlsrv/sqlsrv_ae_datetimes_as_strings.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ for ($i=0; $i<$SZ_DATE_all; $i++)
544544
}
545545
}
546546

547-
date_default_timezone_set('Canada/Pacific');
547+
date_default_timezone_set('America/Vancouver');
548548
sqlsrv_configure('WarningsReturnAsErrors', 1);
549549
sqlsrv_configure('LogSeverity', SQLSRV_LOG_SEVERITY_ALL);
550550
sqlsrv_configure('LogSubsystems', SQLSRV_LOG_SYSTEM_OFF);

‎test/functional/sqlsrv/sqlsrv_ae_insert_datetime.phpt‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Bind params using sqlsrv_prepare without any sql_type specified
99
require_once('MsCommon.inc');
1010
require_once('AEData.inc');
1111

12-
date_default_timezone_set("Canada/Pacific");
12+
date_default_timezone_set("America/Vancouver");
1313
$dataTypes = array( "date", "datetime", "datetime2", "smalldatetime", "time", "datetimeoffset" );
1414
$conn = AE\connect();
1515

@@ -53,23 +53,23 @@ sqlsrv_close($conn);
5353

5454
Testing date:
5555
****Encrypted default type is compatible with encrypted date****
56-
c_det: 0001-01-01 00:00:00.000000 Canada/Pacific
57-
c_rand: 9999-12-31 00:00:00.000000 Canada/Pacific
56+
c_det: 0001-01-01 00:00:00.000000 America/Vancouver
57+
c_rand: 9999-12-31 00:00:00.000000 America/Vancouver
5858

5959
Testing datetime:
6060
****Encrypted default type is compatible with encrypted datetime****
61-
c_det: 1753-01-01 00:00:00.000000 Canada/Pacific
62-
c_rand: 9999-12-31 23:59:59.997000 Canada/Pacific
61+
c_det: 1753-01-01 00:00:00.000000 America/Vancouver
62+
c_rand: 9999-12-31 23:59:59.997000 America/Vancouver
6363

6464
Testing datetime2:
6565
****Encrypted default type is compatible with encrypted datetime2****
66-
c_det: 0001-01-01 00:00:00.000000 Canada/Pacific
67-
c_rand: 9999-12-31 23:59:59.123456 Canada/Pacific
66+
c_det: 0001-01-01 00:00:00.000000 America/Vancouver
67+
c_rand: 9999-12-31 23:59:59.123456 America/Vancouver
6868

6969
Testing smalldatetime:
7070
****Encrypted default type is compatible with encrypted smalldatetime****
71-
c_det: 1900-01-01 00:00:00.000000 Canada/Pacific
72-
c_rand: 2079-06-05 23:59:00.000000 Canada/Pacific
71+
c_det: 1900-01-01 00:00:00.000000 America/Vancouver
72+
c_rand: 2079-06-05 23:59:00.000000 America/Vancouver
7373

7474
Testing time:
7575
****Encrypted default type is compatible with encrypted time****

‎test/functional/sqlsrv/sqlsrv_ae_insert_retrieve.phpt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Retrieving SQL query contains encrypted filter
88
<?php
99
require_once('MsCommon.inc');
1010

11-
date_default_timezone_set("Canada/Pacific");
11+
date_default_timezone_set("America/Vancouver");
1212
$conn = AE\connect();
1313

1414
// Create the table
@@ -73,7 +73,7 @@ Retrieving plaintext data:
7373
SSN: 795-73-9838
7474
FirstName: Catherine
7575
LastName: Abel
76-
BirthDate: 1996-10-19 00:00:00.000000 Canada/Pacific
76+
BirthDate: 1996-10-19 00:00:00.000000 America/Vancouver
7777

7878
Checking ciphertext data:
7979
Done

‎test/functional/sqlsrv/sqlsrv_ae_insert_retrieve_fixed_size.phpt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Test for inserting encrypted fixed size types data and retrieve both encrypted a
66
<?php
77
require_once('MsCommon.inc');
88

9-
date_default_timezone_set("Canada/Pacific");
9+
date_default_timezone_set("America/Vancouver");
1010
$conn = AE\connect();
1111
$testPass = true;
1212

@@ -88,6 +88,6 @@ IntData: 2147483647
8888
BigIntData: 92233720368547
8989
DecimalData: 79228162514264
9090
BitData: 1
91-
DateTimeData: 9999-12-31 23:59:59.997000 Canada/Pacific
92-
DateTime2Data: 9999-12-31 23:59:59.123456 Canada/Pacific
91+
DateTimeData: 9999-12-31 23:59:59.997000 America/Vancouver
92+
DateTime2Data: 9999-12-31 23:59:59.123456 America/Vancouver
9393
Done

‎test/functional/sqlsrv/sqlsrv_ae_insert_sqltype_datetime.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Bind params using sqlsrv_prepare with all sql_type
99
require_once('MsCommon.inc');
1010
require_once('AEData.inc');
1111

12-
date_default_timezone_set("Canada/Pacific");
12+
date_default_timezone_set("America/Vancouver");
1313
$dataTypes = array( "date", "datetime", "datetime2", "smalldatetime", "time", "datetimeoffset" );
1414

1515
// this is a list of implicit datatype conversion that SQL Server allows (https://docs.microsoft.com/en-us/sql/t-sql/data-types/data-type-conversion-database-engine)

0 commit comments

Comments
 (0)