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 PDO::quote with string containing ASCII NUL character #550

Merged
merged 5 commits into from
Sep 28, 2017

Conversation

yukiwongky
Copy link
Contributor

@yukiwongky yukiwongky commented Sep 22, 2017

Fix for #538

@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.306% when pulling 57a41d7 on v-kaywon:fixPODQuoteNul into 54141c8 on Microsoft:dev.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.306% when pulling 57a41d7 on v-kaywon:fixPODQuoteNul into 54141c8 on Microsoft:dev.

@@ -1463,7 +1463,7 @@ int pdo_sqlsrv_dbh_quote( _Inout_ pdo_dbh_t* dbh, _In_reads_(unquoted_len) const
if ( encoding == SQLSRV_ENCODING_UTF8 ) {
quotes_needed = 3;
}
for ( size_t index = 0; index < unquoted_len && unquoted[ index ] != '\0'; ++index ) {
for ( size_t index = 0; index < unquoted_len; ++index ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the argument unquoted_len always correct? What is unquoted_len if the string is empty? Or what if the input string is composed of only one null character, something like '\0', or a series of null characters, something like '\0\0\0\0'?

Copy link
Contributor Author

@yukiwongky yukiwongky Sep 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I've tested, unquote_len seems to be always correct ( it was parsed out by the PDO extension using zend_parse_parameter). If the string is empty (""), unquoted_len is 0. If the input string is composed of only one null character, unquoted_len is 1, and for '\0\0\0\0' unquoted_len is 4.

{
$connection = connect();
//$connection->setAttribute( PDO::SQLSRV_ATTR_DIRECT_QUERY, PDO::SQLSRV_ENCODING_SYSTEM );

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you comment this line? Have you tested this with PDO::SQLSRV_ENCODING_SYSTEM in Windows?

Copy link
Contributor Author

@yukiwongky yukiwongky Sep 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested both ENCODING_UTF8 and ENCODING_SYSTEM and they are both OK. I'll delete this line from the test.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 73.272% when pulling 5ac1c73 on v-kaywon:fixPODQuoteNul into 54141c8 on Microsoft:dev.

@yukiwongky yukiwongky closed this Sep 26, 2017
@yukiwongky yukiwongky reopened this Sep 26, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 73.272% when pulling 5ac1c73 on v-kaywon:fixPODQuoteNul into 54141c8 on Microsoft:dev.

@yukiwongky yukiwongky closed this Sep 26, 2017
@yukiwongky yukiwongky reopened this Sep 26, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 73.272% when pulling 5ac1c73 on v-kaywon:fixPODQuoteNul into 54141c8 on Microsoft:dev.

@yukiwongky yukiwongky closed this Sep 26, 2017
@yukiwongky yukiwongky reopened this Sep 26, 2017
@coveralls
Copy link

coveralls commented Sep 26, 2017

Coverage Status

Coverage decreased (-0.03%) to 73.272% when pulling 5ac1c73 on v-kaywon:fixPODQuoteNul into 54141c8 on Microsoft:dev.


--EXPECT--
Original: XX{NUL}XX
Quoted: 'XX{NUL}XX'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this character at the EOF?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no newline

@yukiwongky yukiwongky merged commit 61f8811 into microsoft:dev Sep 28, 2017
@yukiwongky yukiwongky deleted the fixPODQuoteNul branch January 15, 2018 20:42
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.

6 participants