Assorted ssh2 patches

Hi, I'm attaching patches for assorted bugs in the ssh2 extension. I'm interested in contributing more fixes if these are ok. I've been working with Scott while creating these patches; he recommended I attach them here. The files are named in an order, but I don't think there's any explicit dependency between them. (Hopefully they are attached in order, but they probably won't be...) Patches: --- * 01-ssh2-bug14397: fix for PHP 5.3 compilation (http://pecl.php.net/bugs/bug.php?id=14397) * 02-cvsignore: missing .cvsignore file (just ignores the build-related files) * 03-ssh2-bug12008: segfault when using a non-blocking ssh2 channel stream (http://pecl.php.net/bugs/bug.php?id=12008) * 04-segfault-invalid-path: segfault when you pass an invalid path into the ssh2 stream handler * 05-ssh2-bug13439: inverted return value for sftp streams from unlink/rename/mkdir/rmdir. Additionally, there are some memory leak fixes here that affect the context directly next to the return values. (http://pecl.php.net/bugs/bug.php?id=13439) * 06-memleak-sftp-urlstat: memory leak in the urlstat function for sftp streams * 07-memleak-sftp-rename: memory leak with sftp streams when renaming --- If you have any questions, let me know. Thanks! Mike Sullivan

Index: ssh2_fopen_wrappers.c =================================================================== --- ssh2_fopen_wrappers.c (revision 5) +++ ssh2_fopen_wrappers.c (revision 6) @@ -128,6 +128,9 @@ int username_len = 0, password_len = 0; resource = php_url_parse(path); + if (!resource) { + return NULL; + } if (strncmp(resource->scheme, "ssh2.", sizeof("ssh2.") - 1)) { /* Not an ssh wrapper */ Index: ssh2_sftp.c =================================================================== --- ssh2_sftp.c (revision 6) +++ ssh2_sftp.c (revision 7) @@ -417,7 +417,10 @@ resource = php_ssh2_fopen_wraper_parse_path(url, "sftp", context, &session, &resource_id, &sftp, &sftp_rsrcid TSRMLS_CC); if (!resource || !session || !sftp || !resource->path) { - return -1; + if (resource) { + php_url_free(resource); + } + return 0; } result = libssh2_sftp_unlink(sftp, resource->path); @@ -425,7 +428,8 @@ zend_list_delete(sftp_rsrcid); - return (result == 0) ? 0 : -1; + /* libssh2 uses 0 for success and the streams API uses 0 for failure, so invert */ + return (result == 0) ? -1 : 0; } /* }}} */ @@ -441,18 +445,24 @@ if (strncmp(url_from, "ssh2.sftp://", sizeof("ssh2.sftp://") - 1) || strncmp(url_to, "ssh2.sftp://", sizeof("ssh2.sftp://") - 1)) { - return -1; + return 0; } resource_to = php_url_parse(url_to); if (!resource_to || !resource_to->path) { - return -1; + if (resource_to) { + php_url_free(resource_to); + } + return 0; } resource = php_ssh2_fopen_wraper_parse_path(url_from, "sftp", context, &session, &resource_id, &sftp, &sftp_rsrcid TSRMLS_CC); if (!resource || !session || !sftp || !resource->path) { + if (resource) { + php_url_free(resource); + } php_url_free(resource_to); - return -1; + return 0; } result = libssh2_sftp_rename(sftp, resource->path, resource_to->path); @@ -460,7 +470,8 @@ zend_list_delete(sftp_rsrcid); - return (result == 0) ? 0 : -1; + /* libssh2 uses 0 for success and the streams API uses 0 for failure, so invert */ + return (result == 0) ? -1 : 0; } /* }}} */ @@ -476,7 +487,10 @@ resource = php_ssh2_fopen_wraper_parse_path(url, "sftp", context, &session, &resource_id, &sftp, &sftp_rsrcid TSRMLS_CC); if (!resource || !session || !sftp || !resource->path) { - return -1; + if (resource) { + php_url_free(resource); + } + return 0; } if (options & PHP_STREAM_MKDIR_RECURSIVE) { @@ -492,7 +506,8 @@ zend_list_delete(sftp_rsrcid); - return (result == 0) ? 0 : -1; + /* libssh2 uses 0 for success and the streams API uses 0 for failure, so invert */ + return (result == 0) ? -1 : 0; } /* }}} */ @@ -508,7 +523,10 @@ resource = php_ssh2_fopen_wraper_parse_path(url, "sftp", context, &session, &resource_id, &sftp, &sftp_rsrcid TSRMLS_CC); if (!resource || !session || !sftp || !resource->path) { - return -1; + if (resource) { + php_url_free(resource); + } + return 0; } result = libssh2_sftp_rmdir(sftp, resource->path); @@ -516,7 +534,8 @@ zend_list_delete(sftp_rsrcid); - return (result == 0) ? 0 : -1; + /* libssh2 uses 0 for success and the streams API uses 0 for failure, so invert */ + return (result == 0) ? -1 : 0; } /* }}} */ #endif Index: ssh2_sftp.c =================================================================== --- ssh2_sftp.c (revision 7) +++ ssh2_sftp.c (revision 8) @@ -397,6 +397,8 @@ zend_list_delete(sftp_rsrcid); return -1; } + + php_url_free(resource); /* parse_path addrefs the resource, but we're not holding on to it so we have to delref it before we leave */ zend_list_delete(sftp_rsrcid); Index: ssh2_sftp.c =================================================================== --- ssh2_sftp.c (revision 8) +++ ssh2_sftp.c (revision 9) @@ -469,6 +469,7 @@ result = libssh2_sftp_rename(sftp, resource->path, resource_to->path); php_url_free(resource); + php_url_free(resource_to); zend_list_delete(sftp_rsrcid); Index: ssh2.c =================================================================== --- ssh2.c (revision 1) +++ ssh2.c (revision 2) @@ -889,7 +889,7 @@ for(i = 0; i < numfds; i++) { zval *subarray = *pollmap[i]; - if (!subarray->is_ref && subarray->refcount > 1) { + if (!Z_ISREF_P(subarray) && Z_REFCOUNT_P(subarray) > 1) { /* Make a new copy of the subarray zval* */ MAKE_STD_ZVAL(subarray); *subarray = **pollmap[i]; @@ -899,8 +899,8 @@ zval_copy_ctor(subarray); /* Fixup its refcount */ - subarray->is_ref = 0; - subarray->refcount = 1; + Z_UNSET_ISREF_P(subarray); + Z_SET_REFCOUNT_P(subarray, 1); } zend_hash_del(Z_ARRVAL_P(subarray), "revents", sizeof("revents")); add_assoc_long(subarray, "revents", pollfds[i].revents); @@ -1009,8 +1009,8 @@ } zval_copy_ctor(&copyval); - copyval.is_ref = 0; - copyval.refcount = 1; + Z_UNSET_ISREF_P(&copyval); + Z_SET_REFCOUNT_P(&copyval, 1); convert_to_string(&copyval); if (*key == '*') { Index: php_ssh2.h =================================================================== --- php_ssh2.h (revision 1) +++ php_ssh2.h (revision 2) @@ -109,7 +109,33 @@ #define SSH2_TSRMLS_FETCH(datap) #endif +/* < 5.3 compatibility */ +#ifndef Z_REFCOUNT_P +#define Z_REFCOUNT_P(pz) (pz)->refcount +#define Z_REFCOUNT_PP(ppz) Z_REFCOUNT_P(*(ppz)) +#endif +#ifndef Z_SET_REFCOUNT_P +#define Z_SET_REFCOUNT_P(pz, rc) (pz)->refcount = rc +#define Z_SET_REFCOUNT_PP(ppz, rc) Z_SET_REFCOUNT_P(*(ppz), rc) +#endif + +#ifndef Z_ISREF_P +#define Z_ISREF_P(pz) (pz)->is_ref +#define Z_ISREF_PP(ppz) Z_ISREF_P(*(ppz)) +#endif + +#ifndef Z_SET_ISREF_P +#define Z_SET_ISREF_P(pz) (pz)->is_ref = 1 +#define Z_SET_ISREF_PP(ppz) Z_SET_ISREF_P(*(ppz)) +#endif + +#ifndef Z_UNSET_ISREF_P +#define Z_UNSET_ISREF_P(pz) (pz)->is_ref = 0 +#define Z_UNSET_ISREF_PP(ppz) Z_UNSET_ISREF_P(*(ppz)) +#endif + + typedef struct _php_ssh2_channel_data { LIBSSH2_CHANNEL *channel; Index: .cvsignore =================================================================== --- .cvsignore (revision 0) +++ .cvsignore (revision 4) @@ -0,0 +1,28 @@ +.deps +.libs +Makefile +Makefile.fragments +Makefile.global +Makefile.objects +acinclude.m4 +aclocal.m4 +autom4te.cache +build +config.guess +config.h +config.h.in +config.log +config.nice +config.status +config.sub +configure +configure.in +include +install-sh +libtool +ltmain.sh +missing +mkinstalldirs +modules +*.la +*.lo Index: ssh2_fopen_wrappers.c =================================================================== --- ssh2_fopen_wrappers.c (revision 4) +++ ssh2_fopen_wrappers.c (revision 5) @@ -40,10 +40,13 @@ static size_t php_ssh2_channel_stream_read(php_stream *stream, char *buf, size_t count TSRMLS_DC) { php_ssh2_channel_data *abstract = (php_ssh2_channel_data*)stream->abstract; + ssize_t readstate; stream->eof = libssh2_channel_eof(abstract->channel); libssh2_channel_set_blocking(abstract->channel, abstract->is_blocking); - return libssh2_channel_read_ex(abstract->channel, abstract->streamid, buf, count); + + readstate = libssh2_channel_read_ex(abstract->channel, abstract->streamid, buf, count); + return (readstate == LIBSSH2_ERROR_EAGAIN ? 0 : readstate); } static int php_ssh2_channel_stream_close(php_stream *stream, int close_handle TSRMLS_DC)