Re: [PATCH]#55651

hmm, after deep looking, I found maybe this behavior(cann't connect to NAT server when use pasv mode) is "as expected", since this is not php ftp-ext issue, but a ftp server configure issue, that is, if we apply this patch, will make the php ftp-ext not a standard ftp protocl executor. thanks 2011/9/12 Laruence <laruence@php.net>: > 2011/9/12 Avi Brender <abrender@elitehosts.com>: >> 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 > actually I think it can,  plz refer to the existing test config script > "server.inc" under the ftp/tests/, > and maybe you can simulate a test environ? > > and thanks for your work on PHP >> 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<abrender@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<laruence@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<abrender@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 >>>> >>> >>> >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: http://www.php.net/unsub.php >> > > > > -- > Laruence  Xinchen Hui > http://www.laruence.com/ > -- Laruence  Xinchen Hui http://www.laruence.com/