Bug #48607 [Com]: fwrite() doesn't check reply from ftp server before exiting
Edit report at http://bugs.php.net/bug.php?id=48607edit=1 ID: 48607 Comment by: eric dot caron at gmail dot com Reported by:karachi at mail dot ru Summary:fwrite() doesn't check reply from ftp server before exiting Status: Closed Type: Bug Package:FTP related Operating System: FreeBSD PHP Version:5.2.10 Assigned To:iliaa Block user comment: N Private report: N New Comment: What are the steps involved in having this bug moved from SVN into the current 5.3.x branch? It is a bug fix, no new features are added nor does any functionality change, yet two 5.3.x releases have come out since this bug was marked CLOSED and they don't include this fix. Previous Comments: [2010-12-13 17:53:37] il...@php.net This bug has been fixed in SVN. Snapshots of the sources are packaged every three hours; this change will be in the next snapshot. You can grab the snapshot at http://snaps.php.net/. Thank you for the report, and for helping us make PHP better. [2010-12-13 17:53:28] il...@php.net Automatic comment from SVN on behalf of iliaa Revision: http://svn.php.net/viewvc/?view=revisionamp;revision=306342 Log: Fixed bug #48607 (fwrite() doesn't check reply from ftp server before exiting) [2010-10-14 19:41:54] eric dot caron at gmail dot com I can reproduce this on my CentOS 5 box running PHP 5.3.3. It occurs when sending a large file across a slow network. Wireshark reports getting the QUIT before the FTP server sends TRANSFER COMPLETE. Adding the sleep before the close fixes the issue. Reproduce code: -- ?php $fileName = SOME_LARGE_BINARY_FILE; $conn_id = ftp_connect(FTP_SERVER); $login_result = ftp_login($conn_id, USERNAME, PASSWORD); ftp_pasv($conn_id, true); ftp_put($conn_id, 'file_dump', $fileName, FTP_BINARY); //Putting a sleep here fixes the problem ftp_close($conn_id); [2010-08-31 19:03:59] savageman86 at yahoo dot fr I also ran into this bug. One possible solution is to use a custom ftp stream wrapper which encapsulates the ftp_* functions, because ftp_fput() works well and waits the file has finished uploading before returning. At the end, the only current solution is to use the ftp lib and not the default ftp stream wrapper, which is buggy. It's sad, because stream wrappers are really a killer feature ! :-) [2009-12-17 21:59:06] b dot vontobel at meteonews dot ch Sorry, just realized that I went a little bit too far when cleaning up my mess for the diff/patch. :) --- php-5.3.1/ext/standard/ftp_fopen_wrapper.c 2008-12-31 11:15:49.0 + +++ php-5.3.1-ftp_fopen_wrapper_patch/ext/standard/ftp_fopen_wrapper.c 2009-12-17 21:32:53.0 + @@ -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; } Also make sure that I or somebody else afterwards really does not call something on/in the streams after closing and probably freeing them (didn't really check out the internals of _php_stream_free() et al. -- and the control stream sort of being embedded within the data stream here, but me having to close them the other way round due to the FTP protocol, didn't really help in understanding what might go wrong somewhere deep in your API). But as I
Bug #48607 [Com]: fwrite() doesn't check reply from ftp server before exiting
Edit report at http://bugs.php.net/bug.php?id=48607edit=1 ID: 48607 Comment by: eric dot caron at gmail dot com Reported by:karachi at mail dot ru Summary:fwrite() doesn't check reply from ftp server before exiting Status: Verified Type: Bug Package:Streams related Operating System: FreeBSD PHP Version:5.2.10 Block user comment: N New Comment: I can reproduce this on my CentOS 5 box running PHP 5.3.3. It occurs when sending a large file across a slow network. Wireshark reports getting the QUIT before the FTP server sends TRANSFER COMPLETE. Adding the sleep before the close fixes the issue. Reproduce code: -- ?php $fileName = SOME_LARGE_BINARY_FILE; $conn_id = ftp_connect(FTP_SERVER); $login_result = ftp_login($conn_id, USERNAME, PASSWORD); ftp_pasv($conn_id, true); ftp_put($conn_id, 'file_dump', $fileName, FTP_BINARY); //Putting a sleep here fixes the problem ftp_close($conn_id); Previous Comments: [2010-08-31 19:03:59] savageman86 at yahoo dot fr I also ran into this bug. One possible solution is to use a custom ftp stream wrapper which encapsulates the ftp_* functions, because ftp_fput() works well and waits the file has finished uploading before returning. At the end, the only current solution is to use the ftp lib and not the default ftp stream wrapper, which is buggy. It's sad, because stream wrappers are really a killer feature ! :-) [2009-12-17 21:59:06] b dot vontobel at meteonews dot ch Sorry, just realized that I went a little bit too far when cleaning up my mess for the diff/patch. :) --- php-5.3.1/ext/standard/ftp_fopen_wrapper.c 2008-12-31 11:15:49.0 + +++ php-5.3.1-ftp_fopen_wrapper_patch/ext/standard/ftp_fopen_wrapper.c 2009-12-17 21:32:53.0 + @@ -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; } Also make sure that I or somebody else afterwards really does not call something on/in the streams after closing and probably freeing them (didn't really check out the internals of _php_stream_free() et al. -- and the control stream sort of being embedded within the data stream here, but me having to close them the other way round due to the FTP protocol, didn't really help in understanding what might go wrong somewhere deep in your API). But as I said, for me the patch works. [2009-12-17 20:17:41] b dot vontobel at meteonews dot ch Just stumbled across this (still in 5.3.1) a few days ago, trying to transmit data to three different FTP servers. One of the servers _never_ got a file, one got files, but in 9 out of 10 runs the last part of the files was cut off, only the last one got the files intact in about 8 of 10 runs (with the others also corrupted). I didn't find this bug report at first and so I opened up the PHP source for the first time in my life and was rather shocked: There's really no way that write operations using the ftp stream wrapper ever could've worked. If it works, it's out of pure luck. Was this never tested? The problem is, that FTP (see RFC959) uses the tear down of the _data_stream_ as its EOF marker. What this code does on the other hand, is just send a QUIT on the control stream and then tear down that one. So from the perspective of the FTP server it looks like an abort (transmission still in progress, but control
Bug #48607 [Com]: fwrite() doesn't check reply from ftp server before exiting
Edit report at http://bugs.php.net/bug.php?id=48607edit=1 ID: 48607 Comment by: savageman86 at yahoo dot fr Reported by:karachi at mail dot ru Summary:fwrite() doesn't check reply from ftp server before exiting Status: Verified Type: Bug Package:Streams related Operating System: FreeBSD PHP Version:5.2.10 Block user comment: N New Comment: I also ran into this bug. One possible solution is to use a custom ftp stream wrapper which encapsulates the ftp_* functions, because ftp_fput() works well and waits the file has finished uploading before returning. At the end, the only current solution is to use the ftp lib and not the default ftp stream wrapper, which is buggy. It's sad, because stream wrappers are really a killer feature ! :-) Previous Comments: [2009-12-17 21:59:06] b dot vontobel at meteonews dot ch Sorry, just realized that I went a little bit too far when cleaning up my mess for the diff/patch. :) --- php-5.3.1/ext/standard/ftp_fopen_wrapper.c 2008-12-31 11:15:49.0 + +++ php-5.3.1-ftp_fopen_wrapper_patch/ext/standard/ftp_fopen_wrapper.c 2009-12-17 21:32:53.0 + @@ -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; } Also make sure that I or somebody else afterwards really does not call something on/in the streams after closing and probably freeing them (didn't really check out the internals of _php_stream_free() et al. -- and the control stream sort of being embedded within the data stream here, but me having to close them the other way round due to the FTP protocol, didn't really help in understanding what might go wrong somewhere deep in your API). But as I said, for me the patch works. [2009-12-17 20:17:41] b dot vontobel at meteonews dot ch Just stumbled across this (still in 5.3.1) a few days ago, trying to transmit data to three different FTP servers. One of the servers _never_ got a file, one got files, but in 9 out of 10 runs the last part of the files was cut off, only the last one got the files intact in about 8 of 10 runs (with the others also corrupted). I didn't find this bug report at first and so I opened up the PHP source for the first time in my life and was rather shocked: There's really no way that write operations using the ftp stream wrapper ever could've worked. If it works, it's out of pure luck. Was this never tested? The problem is, that FTP (see RFC959) uses the tear down of the _data_stream_ as its EOF marker. What this code does on the other hand, is just send a QUIT on the control stream and then tear down that one. So from the perspective of the FTP server it looks like an abort (transmission still in progress, but control channel lost). Now it just depends on the implementation of the server and sometimes some random timing issues (which TCP close is handled first) what the outcome is: Some FTP servers just annihilate everything that was transmitted so far (realizing it was a client abort during transmission or a network glitch - and the file probably corrupted anyway), others keep what they got so far. Only sometimes (out of luck) they maybe get the close on the data stream first and are still able to send the okay on the control stream (which is not handled by the current code, but what sjoerd added in his first idea of a patch). Now, I'm not really familiar with the PHP stream wrapper API at