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

Test ftp_ssl_connect() function : error conditions #2584

Closed
wants to merge 1 commit into from
Closed

Test ftp_ssl_connect() function : error conditions #2584

wants to merge 1 commit into from

Conversation

chancegarcia
Copy link
Contributor

I don't know how to run coverage but I'm attempting to cover the return false on failure portion for ftp_ssl_connect and the timeout value warning from

http://gcov.php.net/PHP_HEAD/lcov_html/ext/ftp/php_ftp.c.gcov.php

screen shot 2017-06-18 at 8 23 57 pm

<?php
echo "*** Testing ftp_ssl_connect() function : error conditions ***\n";
echo "\n-- Testing ftp_ssl_connect() function on failure --\n";
var_dump(ftp_ssl_connect('127.0.0.1'));
Copy link
Contributor

Choose a reason for hiding this comment

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

No guarantee there isn't a server listening on localhost. Maybe try a hostname of does-not-exist.invalid

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

echo "\n-- Testing ftp_ssl_connect() function timeout warning for value 0 --\n";
ftp_ssl_connect('127.0.0.1', 21, 0);

?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Long as you're at it, add a test for invalid argument type? ftp_ssl_connect(array());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do. i'll cover invalid for each of the 3 arguments and also cover exceeds expected number of arguments

I don't possess enough understanding of the c code to figure out how to cover this portion:

if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|ll", &host, &host_len, &port, 
&timeout_sec) == FAILURE) {
return;
 }

Copy link
Member

@KalleZ KalleZ Jun 19, 2017

Choose a reason for hiding this comment

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

@chancegarcia s|ll means:

s - parameter #1 must be a string
| - following parameters are optional
l - parameter #2 must be a numeric (integer)
l - parameter #3 must be a numeric (integer)

To make this fail, you can pass something like
new stdClass
[]

To the first parameter, and the conditional should pass and the return statement is hit. If you look in the ext/standard/tests directory, there should be a lot of error condition tests you can use for reference

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, ok. so that portion of the code is parse/validating the arguments?

Thanks for explaining it! =D

Copy link
Member

Choose a reason for hiding this comment

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

@chancegarcia idd, the zend_parse_parameters() function translates the parameters sent to a function call into C variables. You take a look at the README.PARAMETER-PARSING-API or something similar in the root of php-src that has more in-depth about how this works and the meanings =)

- test timeout warning
- ensure connection fails with invalid hostname
- test invalid parameter types
- test exceeds expected number of parameters
@chancegarcia
Copy link
Contributor Author

made changes and rebased the commits into a single commit.

@php-pulls
Copy link

Comment on behalf of pollita at php.net:

510230d

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.

4 participants