Skip to content

Commit 32ecea7

Browse files
jahnvi480David-Engel
jahnvi480
and
David-Engel
authored
FIX: Silent failure during connection failure (#1595)
* FIX: Silent failure during connection failure * Change fallback error to use common error arrays and functions --------- Co-authored-by: David Engel <davidengel@microsoft.com>
1 parent d1027a4 commit 32ecea7

5 files changed

Lines changed: 140 additions & 9 deletions

File tree

‎source/pdo_sqlsrv/pdo_util.cpp‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,10 @@ pdo_error PDO_ERRORS[] = {
489489
SQLSRV_ERROR_TVP_INPUT_PARAM_ONLY,
490490
{ IMSSP, (SQLCHAR*) "You cannot return data in a table-valued parameter. Table-valued parameters are input-only.", -106, false }
491491
},
492+
{
493+
SQLSRV_ERROR_ODBC_DIAGNOSTICS_UNAVAILABLE,
494+
{ IMSSP, (SQLCHAR*) "The ODBC operation failed. Diagnostic information is not available from the driver.", -107, false }
495+
},
492496

493497
{ UINT_MAX, {} }
494498
};
@@ -691,16 +695,9 @@ void format_or_get_all_errors(_Inout_ sqlsrv_context& ctx, _In_opt_ unsigned int
691695
}
692696
}
693697
else {
694-
// Failed to retrieve ODBC error - create a minimal error with HY000 (general error)
698+
// Failed to retrieve ODBC error - create a fallback error
695699
// This indicates that both SQLGetDiagRecW and SQLGetDiagFieldW failed to retrieve error information
696-
error = new ( sqlsrv_malloc( sizeof( sqlsrv_error ))) sqlsrv_error();
697-
error->sqlstate = reinterpret_cast<SQLCHAR*>(sqlsrv_malloc(SQL_SQLSTATE_BUFSIZE));
698-
error->native_message = reinterpret_cast<SQLCHAR*>(sqlsrv_malloc(SQL_MAX_ERROR_MESSAGE_LENGTH + 1));
699-
strcpy_s(reinterpret_cast<char*>(error->sqlstate), SQL_SQLSTATE_BUFSIZE, "HY000");
700-
strcpy_s(reinterpret_cast<char*>(error->native_message), SQL_MAX_ERROR_MESSAGE_LENGTH + 1,
701-
"The ODBC operation failed. Diagnostic information is not available from the driver.");
702-
error->native_code = -1;
703-
error->format = false;
700+
core_sqlsrv_format_driver_error(ctx, get_error_message(SQLSRV_ERROR_ODBC_DIAGNOSTICS_UNAVAILABLE), error, SEV_ERROR, NULL);
704701
}
705702

706703
// core_sqlsrv_get_odbc_error() returns the error_code of size SQL_SQLSTATE_BUFSIZE,

‎source/shared/core_sqlsrv.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2060,6 +2060,7 @@ enum SQLSRV_ERROR_CODES {
20602060
SQLSRV_ERROR_TVP_ROW_NOT_ARRAY,
20612061
SQLSRV_ERROR_TVP_ROWS_UNEXPECTED_SIZE,
20622062
SQLSRV_ERROR_TVP_INPUT_PARAM_ONLY,
2063+
SQLSRV_ERROR_ODBC_DIAGNOSTICS_UNAVAILABLE,
20632064

20642065
// Driver specific error codes starts from here.
20652066
SQLSRV_ERROR_DRIVER_SPECIFIC = 1000,

‎source/sqlsrv/util.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,10 @@ ss_error SS_ERRORS[] = {
473473
SQLSRV_ERROR_TVP_INPUT_PARAM_ONLY,
474474
{ IMSSP, (SQLCHAR*) "You cannot return data in a table-valued parameter. Table-valued parameters are input-only.", -130, false }
475475
},
476+
{
477+
SQLSRV_ERROR_ODBC_DIAGNOSTICS_UNAVAILABLE,
478+
{ IMSSP, (SQLCHAR*) "The ODBC operation failed. Diagnostic information is not available from the driver.", -131, false }
479+
},
476480

477481
// terminate the list of errors/warnings
478482
{ UINT_MAX, {} }
@@ -890,14 +894,24 @@ bool handle_errors_and_warnings( _Inout_ sqlsrv_context& ctx, _Inout_ zval* repo
890894
}
891895

892896
SQLSMALLINT record_number = 0;
897+
bool odbc_error_found = false;
893898
do {
894899

895900
result = core_sqlsrv_get_odbc_error( ctx, ++record_number, error, log_severity );
896901
if( result ) {
902+
odbc_error_found = true;
897903
copy_error_to_zval( &error_z, error, reported_chain, ignored_chain, warning );
898904
}
899905
} while( result );
900906

907+
// If the ODBC operation failed but no diagnostic record was available (e.g. the ODBC driver
908+
// returned SQL_ERROR without setting a diagnostic), report a generic error so that
909+
// sqlsrv_errors() does not return NULL for a failed call.
910+
if( sqlsrv_error_code == SQLSRV_ERROR_ODBC && !odbc_error_found && !warning ) {
911+
core_sqlsrv_format_driver_error( ctx, get_error_message( SQLSRV_ERROR_ODBC_DIAGNOSTICS_UNAVAILABLE ), error, SEV_ERROR, NULL );
912+
copy_error_to_zval( &error_z, error, reported_chain, ignored_chain, warning );
913+
}
914+
901915
// If it were a warning, we report that warnings where ignored except if warnings_return_as_errors
902916
// was true and we added some warnings to the reported_chain.
903917
if( warning ) {

‎test/functional/pdo_sqlsrv/pdo_connect_errors_not_null.phpt‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
--TEST--
2+
GitHub Issue #1514 - PDO connection failure must always populate errorInfo
3+
--DESCRIPTION--
4+
When a PDO connection fails, the PDOException must always contain meaningful
5+
errorInfo. This is the pdo_sqlsrv counterpart of the sqlsrv fix ensuring ODBC
6+
failures without diagnostic records still produce a usable error message.
7+
--SKIPIF--
8+
<?php require('skipif.inc'); ?>
9+
--FILE--
10+
<?php
11+
12+
// Test 1: Connection to a non-existent server
13+
try {
14+
$conn = new PDO("sqlsrv:Server=nonexistent_server_12345;LoginTimeout=1;Encrypt=no", "", "");
15+
echo "Test 1 FAILED: Expected connection to fail\n";
16+
} catch (PDOException $e) {
17+
$info = $e->errorInfo;
18+
if (!is_array($info) || count($info) < 3) {
19+
echo "Test 1 FAILED: errorInfo missing expected fields\n";
20+
} elseif (empty($info[0]) || empty($info[2])) {
21+
echo "Test 1 FAILED: errorInfo has empty SQLSTATE or message\n";
22+
} else {
23+
echo "Test 1 passed: PDOException has proper errorInfo\n";
24+
}
25+
}
26+
27+
// Test 2: Connection with invalid credentials
28+
require_once('MsSetup.inc');
29+
try {
30+
$conn = new PDO("sqlsrv:Server=$server;Database=$databaseName;Driver=$driver;Encrypt=$encrypt;LoginTimeout=5", "bogus_user_12345", "bogus_pwd");
31+
// If the connection succeeded (e.g., Windows auth fallback), skip
32+
echo "Test 2 passed: connection succeeded (Windows auth or trusted)\n";
33+
} catch (PDOException $e) {
34+
$info = $e->errorInfo;
35+
if (!is_array($info) || count($info) < 3) {
36+
echo "Test 2 FAILED: errorInfo missing expected fields\n";
37+
} elseif (empty($info[0]) || empty($info[2])) {
38+
echo "Test 2 FAILED: errorInfo has empty SQLSTATE or message\n";
39+
} else {
40+
echo "Test 2 passed: PDOException has proper errorInfo\n";
41+
}
42+
}
43+
44+
echo "Done\n";
45+
46+
?>
47+
--EXPECT--
48+
Test 1 passed: PDOException has proper errorInfo
49+
Test 2 passed: PDOException has proper errorInfo
50+
Done

‎test/functional/sqlsrv/sqlsrv_connect_errors_not_null.phpt‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
--TEST--
2+
GitHub Issue #1514 - sqlsrv_connect must always populate sqlsrv_errors() on failure
3+
--DESCRIPTION--
4+
When sqlsrv_connect returns false, sqlsrv_errors() should never return null.
5+
Previously, if the ODBC driver returned SQL_ERROR without setting a diagnostic
6+
record (e.g. connecting to Microsoft Fabric Data Warehouse with MARS enabled),
7+
sqlsrv_errors() would return null, making the failure impossible to diagnose.
8+
--SKIPIF--
9+
<?php require('skipif.inc'); ?>
10+
--FILE--
11+
<?php
12+
13+
sqlsrv_configure('WarningsReturnAsErrors', 0);
14+
15+
// Test 1: Connection to a non-existent server should produce errors
16+
$conn = sqlsrv_connect("nonexistent_server_12345", array("LoginTimeout" => 1, "Encrypt" => "no"));
17+
if ($conn !== false) {
18+
fatalError("Test 1: Expected connection to fail");
19+
}
20+
$errors = sqlsrv_errors();
21+
if ($errors === null) {
22+
echo "Test 1 FAILED: sqlsrv_errors() returned null after failed connection\n";
23+
} else {
24+
// Verify error structure
25+
if (!is_array($errors) || count($errors) === 0) {
26+
echo "Test 1 FAILED: sqlsrv_errors() returned empty array\n";
27+
} elseif (!isset($errors[0][0]) || !isset($errors[0][1]) || !isset($errors[0][2])) {
28+
echo "Test 1 FAILED: error entry missing expected fields\n";
29+
} else {
30+
echo "Test 1 passed: sqlsrv_errors() returned proper error info\n";
31+
}
32+
}
33+
34+
// Test 2: Connection with invalid driver should produce errors
35+
$conn = sqlsrv_connect("localhost", array("Driver" => "Nonexistent Driver 99", "LoginTimeout" => 1, "Encrypt" => "no"));
36+
if ($conn !== false) {
37+
fatalError("Test 2: Expected connection to fail");
38+
}
39+
$errors = sqlsrv_errors();
40+
if ($errors === null) {
41+
echo "Test 2 FAILED: sqlsrv_errors() returned null after failed connection\n";
42+
} else {
43+
echo "Test 2 passed: sqlsrv_errors() returned proper error info\n";
44+
}
45+
46+
// Test 3: Connection with invalid credentials should produce errors
47+
require_once('MsSetup.inc');
48+
$conn = sqlsrv_connect($server, array("UID" => "bogus_user_12345", "PWD" => "bogus_pwd", "LoginTimeout" => 5, "Driver" => $driver, "Encrypt" => $encrypt));
49+
if ($conn !== false) {
50+
// If the connection succeeded (e.g., Windows auth fallback), just skip this check
51+
echo "Test 3 passed: connection succeeded (Windows auth or trusted)\n";
52+
sqlsrv_close($conn);
53+
} else {
54+
$errors = sqlsrv_errors();
55+
if ($errors === null) {
56+
echo "Test 3 FAILED: sqlsrv_errors() returned null after failed connection\n";
57+
} else {
58+
echo "Test 3 passed: sqlsrv_errors() returned proper error info\n";
59+
}
60+
}
61+
62+
echo "Done\n";
63+
64+
?>
65+
--EXPECT--
66+
Test 1 passed: sqlsrv_errors() returned proper error info
67+
Test 2 passed: sqlsrv_errors() returned proper error info
68+
Test 3 passed: sqlsrv_errors() returned proper error info
69+
Done

0 commit comments

Comments
 (0)