diff --git a/.travis.yml b/.travis.yml index dfe1b0f22..5a5334eac 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,10 +20,10 @@ env: - TEST_PHP_SQL_PWD=Password123 before_install: - - docker pull microsoft/mssql-server-linux + - docker pull microsoft/mssql-server-linux:2017-latest install: - - docker run -e 'ACCEPT_EULA=Y' -e 'SA_PASSWORD=Password123' -p 1433:1433 --name=$TEST_PHP_SQL_SERVER -d microsoft/mssql-server-linux + - docker run -e 'ACCEPT_EULA=Y' -e 'SA_PASSWORD=Password123' -p 1433:1433 --name=$TEST_PHP_SQL_SERVER -d microsoft/mssql-server-linux:2017-latest - docker build --build-arg PHPSQLDIR=$PHPSQLDIR -t msphpsql-dev -f Dockerfile-msphpsql . before_script: diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 78828a02e..d77a96237 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1067,6 +1067,33 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" ); + // Return the correct error in case the user calls nextRowset() on a null result set. + // Null means that SQLNumResultCols() returns 0 and SQLRowCount does not return > 0. But first + // check that the statement has been executed and that we are not past the end of a non-null + // result set to make sure the user gets the correct error message. These checks are also + // done in core_sqlsrv_next_result(), but we cannot check for null results there because that + // function can be called without calling this one, and SQLSRV_ERROR_NO_FIELDS can then + // be triggered incorrectly. + CHECK_CUSTOM_ERROR( !driver_stmt->executed, driver_stmt, SQLSRV_ERROR_STATEMENT_NOT_EXECUTED ) { + throw core::CoreException(); + } + + CHECK_CUSTOM_ERROR( driver_stmt->past_next_result_end, driver_stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { + throw core::CoreException(); + } + + // Now make sure the result set is not null. + bool has_result = core_sqlsrv_has_any_result( driver_stmt ); + + // Note that if fetch_called is false but has_result is true (i.e. the user is calling + // nextRowset() on a non-null result set before calling fetch()), it is handled + // in core_sqlsrv_next_result() below. + if( !driver_stmt->fetch_called ) { + CHECK_CUSTOM_ERROR( !has_result, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } + } + core_sqlsrv_next_result( static_cast( stmt->driver_data ) TSRMLS_CC ); // clear the current meta data since the new result will generate new meta data diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index 3398ed59a..29255029b 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -1057,7 +1057,7 @@ void core_sqlsrv_next_result( _Inout_ sqlsrv_stmt* stmt TSRMLS_DC, _In_ bool fin CHECK_CUSTOM_ERROR( stmt->past_next_result_end, stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { throw core::CoreException(); } - + close_active_stream( stmt TSRMLS_CC ); //Clear column sql types and sql display sizes. diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index a548bfc6d..d9b980c25 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -561,6 +561,32 @@ PHP_FUNCTION( sqlsrv_next_result ) try { + // Return the correct error in case the user calls sqlsrv_next_result() on a null result set. + // Null means that SQLNumResultCols() returns 0 and SQLRowCount does not return > 0. But first + // check that the statement has been executed and that we are not past the end of a non-null + // result set to make sure the user gets the correct error message. These checks are also + // done in core_sqlsrv_next_result(), but we cannot check for null results there because that + // function can be called without calling this one, and SQLSRV_ERROR_NO_FIELDS can then + // be triggered incorrectly. + CHECK_CUSTOM_ERROR( !stmt->executed, stmt, SQLSRV_ERROR_STATEMENT_NOT_EXECUTED ) { + throw core::CoreException(); + } + + CHECK_CUSTOM_ERROR( stmt->past_next_result_end, stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { + throw core::CoreException(); + } + + bool has_result = core_sqlsrv_has_any_result( stmt ); + + // Note that if fetch_called is false but has_result is true (i.e. the user is calling + // sqlsrv_next_result() on a non-null result set before calling fetch()), it is handled + // in core_sqlsrv_next_result() below. + if( !stmt->fetch_called ) { + CHECK_CUSTOM_ERROR( !has_result, stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } + } + core_sqlsrv_next_result( stmt TSRMLS_CC, true ); if( stmt->past_next_result_end ) { diff --git a/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt b/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt new file mode 100644 index 000000000..02631f48b --- /dev/null +++ b/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt @@ -0,0 +1,100 @@ +--TEST-- +Error messages from null result sets +--DESCRIPTION-- +Test that calling nextRowset() on an empty result set produces the correct error message. Fix for Github 507. +--SKIPIF-- + +--FILE-- +setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION ); + +DropTable($conn, 'TestEmptySetTable'); +$stmt = $conn->query("CREATE TABLE TestEmptySetTable ([c1] nvarchar(10),[c2] nvarchar(10))"); +$stmt = $conn->query("INSERT INTO TestEmptySetTable (c1, c2) VALUES ('a', 'b')"); + +// Create a procedure that can return a result set or can return nothing +DropProc($conn, 'TestEmptySetProc'); +$stmt = $conn->query("CREATE PROCEDURE TestEmptySetProc @a nvarchar(10), @b nvarchar(10) + AS SET NOCOUNT ON + BEGIN + IF @b='b' + BEGIN + SELECT 'a' as testValue + END + ELSE + BEGIN + UPDATE TestEmptySetTable SET c2 = 'c' WHERE c1 = @a + END + END"); + +// errors out when reaching the second nextRowset() call +// returned error indicates there are no more results +echo "Return a nonempty result set:\n"; +try +{ + $stmt = $conn->query("TestEmptySetProc @a='a', @b='b'"); + $result = $stmt->fetchAll(); + print_r($result); + $stmt->nextRowset(); + $result = $stmt->fetchAll(); + print_r($result); + $stmt->nextRowset(); +} +catch(Exception $e) +{ + echo $e->getMessage()."\n"; +} + +// errors out indicating the result set contains no fields +echo "Return an empty result set, call nextRowset on it before fetching anything:\n"; +try +{ + $stmt = $conn->query("TestEmptySetProc @a='a', @b='c'"); + $stmt->nextRowset(); +} +catch(Exception $e) +{ + echo $e->getMessage()."\n"; +} + +// errors out indicating the result set contains no fields +echo "Return an empty result set, call fetch on it:\n"; +try +{ + $stmt = $conn->query("TestEmptySetProc @a='a', @b='c'"); + $result = $stmt->fetchAll(); + print_r($result); +} +catch(Exception $e) +{ + echo $e->getMessage()."\n"; +} + +$stmt = $conn->query("DROP TABLE TestEmptySetTable"); +$stmt = $conn->query("DROP PROCEDURE TestEmptySetProc"); + +$conn = null; +?> +--EXPECT-- +Return a nonempty result set: +Array +( + [0] => Array + ( + [testValue] => a + [0] => a + ) + +) +Array +( +) +SQLSTATE[IMSSP]: There are no more results returned by the query. +Return an empty result set, call nextRowset on it before fetching anything: +SQLSTATE[IMSSP]: The active result for the query contains no fields. +Return an empty result set, call fetch on it: +SQLSTATE[IMSSP]: The active result for the query contains no fields. diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt new file mode 100644 index 000000000..200ec1ac7 --- /dev/null +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -0,0 +1,124 @@ +--TEST-- +Error messages from null result sets +--DESCRIPTION-- +Test that calling sqlsrv_next_result() on a null result set produces the correct error message. Fix for Github 507. +--SKIPIF-- + +--FILE-- +$databaseName, "uid"=>$uid, "pwd"=>$pwd)); + +DropTable($conn, 'TestEmptySetTable'); +$stmt = sqlsrv_query($conn, "CREATE TABLE TestEmptySetTable ([c1] nvarchar(10),[c2] nvarchar(10))"); +$stmt = sqlsrv_query($conn, "INSERT INTO TestEmptySetTable (c1, c2) VALUES ('a', 'b')"); + +// Create a procedure that can return a result set or can return nothing +DropProc($conn, 'TestEmptySetProc'); +$stmt = sqlsrv_query($conn, "CREATE PROCEDURE TestEmptySetProc @a nvarchar(10), @b nvarchar(10) + AS SET NOCOUNT ON + BEGIN + IF @b='b' + BEGIN + SELECT 'a' as testValue + END + ELSE + BEGIN + UPDATE TestEmptySetTable SET c2 = 'c' WHERE c1 = @a + END + END"); + +// errors out when reaching the second nextRowset() call +// returned error indicates there are no more results +echo "Return a nonempty result set:\n"; + +$stmt = sqlsrv_query($conn,"TestEmptySetProc @a='a', @b='b'"); +$result = sqlsrv_fetch_array($stmt); +print_r($result); +sqlsrv_next_result($stmt); +$result = sqlsrv_fetch_array($stmt); +print_r($result); +sqlsrv_next_result($stmt); + +print_r(sqlsrv_errors()); + +// errors out indicating the result set contains no fields +echo "Return an empty result set, call nextRowset on it before fetching anything:\n"; + +$stmt = sqlsrv_query($conn, "TestEmptySetProc @a='a', @b='c'"); +sqlsrv_next_result($stmt); +print_r(sqlsrv_errors()); + +// errors out indicating the result set contains no fields +echo "Return an empty result set, call fetch on it:\n"; + +$stmt = sqlsrv_query($conn, "TestEmptySetProc @a='a', @b='c'"); +$result = sqlsrv_fetch_array($stmt); +print_r($result); +print_r(sqlsrv_errors()); + +$stmt = sqlsrv_query($conn, "DROP TABLE TestEmptySetTable"); +$stmt = sqlsrv_query($conn, "DROP PROCEDURE TestEmptySetProc"); +sqlsrv_free_stmt($stmt); +sqlsrv_close($conn); +?> +--EXPECTF-- +Return a nonempty result set: +Array +( + [0] => a + [testValue] => a +) +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -26 + [code] => -26 + [2] => There are no more results returned by the query. + [message] => There are no more results returned by the query. + ) + + [1] => Array + ( + [0] => HY010 + [SQLSTATE] => HY010 + [1] => 0 + [code] => 0 + [2] => [%rMicrosoft|unixODBC%r][%rODBC D|D%rriver Manager]%r[ ]{0,1}%rFunction sequence error + [message] => [%rMicrosoft|unixODBC%r][%rODBC D|D%rriver Manager]%r[ ]{0,1}%rFunction sequence error + ) + +) +Return an empty result set, call nextRowset on it before fetching anything: +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -28 + [code] => -28 + [2] => The active result for the query contains no fields. + [message] => The active result for the query contains no fields. + ) + +) +Return an empty result set, call fetch on it: +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -28 + [code] => -28 + [2] => The active result for the query contains no fields. + [message] => The active result for the query contains no fields. + ) + +) diff --git a/test/functional/sqlsrv/test_warning_errors2.phpt b/test/functional/sqlsrv/test_warning_errors2.phpt index 3372ca895..4145565ac 100644 --- a/test/functional/sqlsrv/test_warning_errors2.phpt +++ b/test/functional/sqlsrv/test_warning_errors2.phpt @@ -1,159 +1,159 @@ ---TEST-- -warnings as errors ---SKIPIF-- - ---FILE-- - ---EXPECT-- -Array -( - [0] => Array - ( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -11 - [code] => -11 - [2] => The statement must be executed before results can be retrieved. - [message] => The statement must be executed before results can be retrieved. - ) - -) -Array -( - [0] => Array - ( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -11 - [code] => -11 - [2] => The statement must be executed before results can be retrieved. - [message] => The statement must be executed before results can be retrieved. - ) - -) -Array -( - [0] => Array - ( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -11 - [code] => -11 - [2] => The statement must be executed before results can be retrieved. - [message] => The statement must be executed before results can be retrieved. - ) - -) -Array -( - [0] => Array - ( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -11 - [code] => -11 - [2] => The statement must be executed before results can be retrieved. - [message] => The statement must be executed before results can be retrieved. - ) - -) -Array -( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -11 - [code] => -11 - [2] => The statement must be executed before results can be retrieved. - [message] => The statement must be executed before results can be retrieved. -) -Test successful +--TEST-- +warnings as errors +--SKIPIF-- + +--FILE-- + +--EXPECT-- +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -11 + [code] => -11 + [2] => The statement must be executed before results can be retrieved. + [message] => The statement must be executed before results can be retrieved. + ) + +) +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -11 + [code] => -11 + [2] => The statement must be executed before results can be retrieved. + [message] => The statement must be executed before results can be retrieved. + ) + +) +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -11 + [code] => -11 + [2] => The statement must be executed before results can be retrieved. + [message] => The statement must be executed before results can be retrieved. + ) + +) +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -11 + [code] => -11 + [2] => The statement must be executed before results can be retrieved. + [message] => The statement must be executed before results can be retrieved. + ) + +) +Array +( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -11 + [code] => -11 + [2] => The statement must be executed before results can be retrieved. + [message] => The statement must be executed before results can be retrieved. +) +Test successful