Hi,

I'm new to the PHP internals mailing list (and also to PHP internals in general), so let's first say hello to everybody and thanks for your work!

I needed to dive into PHP's internals myself because I hit a problem (which I later found to be bug #48607) with the ftp_fopen_wrapper: fwrite basically doesn't work at all with FTP, because PHP's implementation is in violation of the FTP specs. No EOF is signalled at the end of a transmission -- PHP's behaviour after writing to an FTP server has to be treated as an aborted connection by a clean server. If an fwrite to an FTP server ever succeeded, it was only due to lucky timing coincidence or buggy servers.

The patch below fixes this issue for me. I run it successfully since more than a month on two high-load production webservers. I'd be happy if somebody more familiar with PHP's internals could review it, apply it to SVN and then hopefully close bug #48607.

Just one important note for the reviewer: I wasn't quite sure on how to correctly "hand up" errors to the higher levels of the API. That's where I just used php_error_docref(), which is probably not the intended way, but worked for me to at least generate a warning. I also now set the return value of the function on failure, but realized that it's again hardcoded to "success" on higher levels of the code. This would probably make for another bug report and more patches. Sorry for my hard words on this in the bug database, I just fell into "rant- mode" when I realized that an exit code for fclose() was hardcoded to success in many places throughout PHP's code -- while I keep telling my developers to always check return values... :)

Cheers,
Beat


--- php-5.3.1/ext/standard/ftp_fopen_wrapper.c 2008-12-31 11:15:49.000000000 +0000 +++ php-5.3.1-ftp_fopen_wrapper_patch/ext/standard/ftp_fopen_wrapper.c 2009-12-17 21:32:53.000000000 +0000
@@ -97,14 +97,34 @@
  */
static int php_stream_ftp_stream_close(php_stream_wrapper *wrapper, php_stream *stream TSRMLS_DC)
 {
+       int ret = 0, result = 0;
+       char tmp_line[512];
        php_stream *controlstream = (php_stream *)stream->wrapperdata;
-       
+
+       /* For write modes close data stream first to signal EOF to server */
+       if (strpbrk(stream->mode, "wa+")) {
+               if (stream && controlstream) {
+                       php_stream_close(stream);
+                       stream = NULL;
+
+                       result = GET_FTP_RESULT(controlstream);
+                       if (result != 226 && result != 250) {
+ php_error_docref(NULL TSRMLS_CC, E_WARNING, "FTP server reports %s", tmp_line);
+                               ret = EOF;
+                       }
+               } else {
+ php_error_docref(NULL TSRMLS_CC, E_WARNING, "Broken streams to FTP server");
+                       ret = EOF;
+               }
+       }
+
        if (controlstream) {
                php_stream_write_string(controlstream, "QUIT\r\n");
                php_stream_close(controlstream);
-               stream->wrapperdata = NULL;
+               if (stream)
+                       stream->wrapperdata = NULL;
        }
-       return 0;
+       return ret;
 }
 /* }}} */

@@ -563,8 +583,9 @@
                goto errexit;
        }

-       /* remember control stream */   
+       /* remember control stream and mode */  
        datastream->wrapperdata = (zval *)stream;
+       strcpy(datastream->mode, mode);

        php_url_free(resource);
        return datastream;



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to