Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash related to zend_hash_internal_pointer_reset #634

Merged
merged 2 commits into from
Dec 15, 2017

Conversation

yukiwongky
Copy link
Contributor

@yukiwongky yukiwongky commented Dec 15, 2017

Previous parse_param_array utilizes zend_hash_* functions for iterating through the parameters passed into sqlsrv_query or sqlsrv_prepare. These zend_hash_* functions alter the internal pointer in the array directly. This is incorrect because the user may be using the internal pointer for other purposes.

For example:

$params = array(array(1, SQLSRV_PARAM_IN, SQLSRV_PHPTYPE_INT, SQLSRV_SQLTYPE_INT), array('BLAH', SQLSRV_PARAM_OUT, SQLSRV_PHPTYPE_FLOAT));
$direction1 = next($params[0]);    // this changes the internal pointer to point to SQLSRV_PARAM_IN
$stmt = sqlsrv_query($conn, $tsql, $params);    // sqlsrv_query calls zend_hash_* functions to iterate through each array in $params; the internal pointers of the array inside $params now point SQLSRV_SQLTYPE_INT and SQLSRV_PHPTYPE_FLOAT for $params[0] and $params[1] respectively

$direction1 = current($params[0]);    // user still expects the internal pointer to point to SQLSRV_PARAM_IN, however since sqlsrv_query uses the internal pointer direct, now the internal pointer points to SQLSRV_SQLTYPE_INT

To avoid working directly with the array internal pointer, use zend_hash_*_ex functions instead. pos acts as a temporary internal pointer so we don't have alter the actual internal pointer.

This change is Reviewable

@coveralls
Copy link

coveralls commented Dec 15, 2017

Coverage Status

Coverage remained the same at 72.504% when pulling 0c0882b on v-kaywon:internalPointerReset into b3f2015 on Microsoft:dev.

@codecov-io
Copy link

codecov-io commented Dec 15, 2017

Codecov Report

Merging #634 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #634   +/-   ##
=======================================
  Coverage   77.13%   77.13%           
=======================================
  Files          25       25           
  Lines        7317     7317           
=======================================
  Hits         5644     5644           
  Misses       1673     1673
Impacted Files Coverage Δ
...rojects/php/x86/php-7.1.12-src/ext/sqlsrv/stmt.cpp 87.77% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3f2015...0c0882b. Read the comment docs.

@yukiwongky yukiwongky merged commit 35146b7 into microsoft:dev Dec 15, 2017
@yukiwongky yukiwongky deleted the internalPointerReset branch January 15, 2018 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants