Hi,
Please see if the attached patch better addresses your concerns.
Regarding the variable name, the PHP_FTP_OPT_USEPASVADDRESS is only
internal and is modeled after the other variables PHP_FTP_TIMEOUT_SEC
and PHP_FTP_OPT_AUTOSEEK. The variable actually passed to the
ftp_set_option() function by users is FTP_USEPASVADDRESS as defined in
php_ftp.c which is inline with the other variables FTP_AUTORESUME,
FTP_TIMEOUT_SEC, FTP_AUTOSEEK etc.
In terms of tests, what type of tests were you thinking of? We can't
ensure that ftp->pasvaddr is set properly in response to a PASV command
unless there's a way to expose those internal variables to the test -
I'm not familiar enough with the internal PHP code to know if that's
possible. If you're referring to tests that ensure that 1/0 true/false
values passed to ftp_set_option() work properly then I've attached a
test file for that.
I don't want to clutter the bugzilla ticket with many attachments so
once the patch is approved I will post the final version in the ticket
if that's okay.
All the best,
Avi Brender
Elite Hosts, Inc
www.elitehosts.com
------------------------------------------------
WARNING !!! This email message is for the sole use of the intended
recipient(s) and may contain confidential and privileged information.
Any unauthorized review; use, disclosure or distribution is prohibited,
and could result in criminal prosecution. If you are not the intended
recipient, please contact the sender by reply email and destroy all
copies of the original message. This message is private and is
considered a confidential exchange - public disclosure of this
electronic message or its contents are prohibited.
On 09/11/2011 04:24 PM, Pierre Joye wrote:
hi!
Please upload the patch in the bug tracker as well.
It would be also better to use a more verbose name.
FTP_OPT_USEPASVADDRESS is somehow cryptic.
Laruence's comment is still valid, the zval should be converted if it
is not int or bool.
Btw, could you test cases as well please?
Cheers,
On Sun, Sep 11, 2011 at 10:00 PM, Avi Brender<abren...@elitehosts.com> wrote:
Hi,
I've updated the patch - please see attached.
Avi Brender
Elite Hosts, Inc
www.elitehosts.com
------------------------------------------------
WARNING !!! This email message is for the sole use of the intended
recipient(s) and may contain confidential and privileged information. Any
unauthorized review; use, disclosure or distribution is prohibited, and
could result in criminal prosecution. If you are not the intended recipient,
please contact the sender by reply email and destroy all copies of the
original message. This message is private and is considered a confidential
exchange - public disclosure of this electronic message or its contents are
prohibited.
On 09/11/2011 10:53 AM, Pierre Joye wrote:
hi,
A simple test if it is IS_BOOL or IS_LONG should be enough, both types
use the the long value (convert_to_boolean_ex is slow and duplicate
the zval while it is not necessary). Then test> 0 instead of simply
assigning the value.
On Sun, Sep 11, 2011 at 5:59 AM, Laruence<larue...@php.net> wrote:
Hi:
after a quick look, I have one suggestion,
if the (Z_TYPE_P(z_value) != IS_BOOL), you should call
convert_to_boolean_ex to convert it to a boolean
otherwise, people can not use a interge 1 as a true flag.
thanks
2011/9/11 Avi Brender<abren...@elitehosts.com>:
Hi,
I've submitted bug #55651 along with a patch to implement a fix (also
attached) for the passive FTP mode issue. I was hoping that someone
could
review the bug report and consider the patch for inclusion in future PHP
releases.
Thanks so much!
Avi Brender
Elite Hosts, Inc
www.elitehosts.com
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
--
Laruence Xinchen Hui
http://www.laruence.com/
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
--TEST--
FTP FTP_USEPASVADDRESS
--SKIPIF--
<?php
require 'skipif.inc';
?>
--FILE--
<?php
require 'server.inc';
$ftp = ftp_connect('127.0.0.1', $port);
if (!$ftp) die("Couldn't connect to the server");
var_dump(ftp_login($ftp, 'user', 'pass'));
var_dump(ftp_set_option($ftp, FTP_USEPASVADDRESS, 1));
var_dump(ftp_get_option($ftp, FTP_USEPASVADDRESS));
var_dump(ftp_set_option($ftp, FTP_USEPASVADDRESS, true));
var_dump(ftp_get_option($ftp, FTP_USEPASVADDRESS));
var_dump(ftp_set_option($ftp, FTP_USEPASVADDRESS, 0));
var_dump(ftp_get_option($ftp, FTP_USEPASVADDRESS));
var_dump(ftp_set_option($ftp, FTP_USEPASVADDRESS, false));
var_dump(ftp_get_option($ftp, FTP_USEPASVADDRESS));
ftp_close($ftp);
?>
--EXPECT--
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(false)
bool(true)
bool(false)
diff -NaurbB ext/ftp.orig/ftp.c ext/ftp/ftp.c
--- ext/ftp.orig/ftp.c 2010-12-31 21:19:59.000000000 -0500
+++ ext/ftp/ftp.c 2011-09-09 03:51:08.000000000 -0400
@@ -712,10 +712,11 @@
memset(&ftp->pasvaddr, 0, n);
sa = (struct sockaddr *) &ftp->pasvaddr;
-#if HAVE_IPV6
if (getpeername(ftp->fd, sa, &n) < 0) {
return 0;
}
+
+#if HAVE_IPV6
if (sa->sa_family == AF_INET6) {
struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *) sa;
char *endptr, delimiter;
@@ -768,8 +769,9 @@
ipbox.c[n] = (unsigned char) b[n];
}
sin = (struct sockaddr_in *) sa;
- sin->sin_family = AF_INET;
+ if (ftp->usepasvaddress) {
sin->sin_addr = ipbox.ia[0];
+ }
sin->sin_port = ipbox.s[2];
ftp->pasv = 2;
diff -NaurbB ext/ftp.orig/ftp.h ext/ftp/ftp.h
--- ext/ftp.orig/ftp.h 2010-12-31 21:19:59.000000000 -0500
+++ ext/ftp/ftp.h 2011-09-09 03:52:38.000000000 -0400
@@ -31,6 +31,7 @@
#define FTP_DEFAULT_TIMEOUT 90
#define FTP_DEFAULT_AUTOSEEK 1
+#define FTP_DEFAULT_USEPASVADDRESS 1
#define PHP_FTP_FAILED 0
#define PHP_FTP_FINISHED 1
#define PHP_FTP_MOREDATA 2
@@ -71,6 +72,7 @@
php_sockaddr_storage pasvaddr; /* passive mode address */
long timeout_sec; /* User configureable timeout (seconds) */
int autoseek; /* User configureable autoseek flag */
+ int usepasvaddress; /* Use the address returned by the pasv command */
int nb; /* "nonblocking" transfer in progress */
databuf_t *data; /* Data connection for "nonblocking" transfers */
diff -NaurbB ext/ftp.orig/php_ftp.c ext/ftp/php_ftp.c
--- ext/ftp.orig/php_ftp.c 2010-12-31 21:19:59.000000000 -0500
+++ ext/ftp/php_ftp.c 2011-09-11 19:11:32.000000000 -0400
@@ -315,6 +315,7 @@
REGISTER_LONG_CONSTANT("FTP_AUTORESUME", PHP_FTP_AUTORESUME, CONST_PERSISTENT | CONST_CS);
REGISTER_LONG_CONSTANT("FTP_TIMEOUT_SEC", PHP_FTP_OPT_TIMEOUT_SEC, CONST_PERSISTENT | CONST_CS);
REGISTER_LONG_CONSTANT("FTP_AUTOSEEK", PHP_FTP_OPT_AUTOSEEK, CONST_PERSISTENT | CONST_CS);
+ REGISTER_LONG_CONSTANT("FTP_USEPASVADDRESS", PHP_FTP_OPT_USEPASVADDRESS, CONST_PERSISTENT | CONST_CS);
REGISTER_LONG_CONSTANT("FTP_FAILED", PHP_FTP_FAILED, CONST_PERSISTENT | CONST_CS);
REGISTER_LONG_CONSTANT("FTP_FINISHED", PHP_FTP_FINISHED, CONST_PERSISTENT | CONST_CS);
REGISTER_LONG_CONSTANT("FTP_MOREDATA", PHP_FTP_MOREDATA, CONST_PERSISTENT | CONST_CS);
@@ -363,6 +364,7 @@
/* autoseek for resuming */
ftp->autoseek = FTP_DEFAULT_AUTOSEEK;
+ ftp->usepasvaddress = FTP_DEFAULT_USEPASVADDRESS;
#if HAVE_OPENSSL_EXT
/* disable ssl */
ftp->use_ssl = 0;
@@ -399,6 +401,7 @@
/* autoseek for resuming */
ftp->autoseek = FTP_DEFAULT_AUTOSEEK;
+ ftp->usepasvaddress = FTP_DEFAULT_USEPASVADDRESS;
/* enable ssl */
ftp->use_ssl = 1;
@@ -1395,6 +1398,13 @@
ftp->autoseek = Z_LVAL_P(z_value);
RETURN_TRUE;
break;
+ case PHP_FTP_OPT_USEPASVADDRESS:
+ if (Z_TYPE_P(z_value) != IS_BOOL && Z_TYPE_P(z_value) != IS_LONG) {
+ convert_to_boolean_ex(&z_value);
+ }
+ ftp->usepasvaddress = Z_LVAL_P(z_value);
+ RETURN_TRUE;
+ break;
default:
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown option '%ld'", option);
RETURN_FALSE;
@@ -1424,6 +1434,9 @@
case PHP_FTP_OPT_AUTOSEEK:
RETURN_BOOL(ftp->autoseek);
break;
+ case PHP_FTP_OPT_USEPASVADDRESS:
+ RETURN_BOOL(ftp->usepasvaddress);
+ break;
default:
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown option '%ld'", option);
RETURN_FALSE;
diff -NaurbB ext/ftp.orig/php_ftp.h ext/ftp/php_ftp.h
--- ext/ftp.orig/php_ftp.h 2010-12-31 21:19:59.000000000 -0500
+++ ext/ftp/php_ftp.h 2011-09-09 02:59:44.000000000 -0400
@@ -29,6 +29,7 @@
#define PHP_FTP_OPT_TIMEOUT_SEC 0
#define PHP_FTP_OPT_AUTOSEEK 1
+#define PHP_FTP_OPT_USEPASVADDRESS 2
#define PHP_FTP_AUTORESUME -1
PHP_MINIT_FUNCTION(ftp);
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
