On 10/07/2015 12:16 PM, Vitaly Lavrov wrote: > Bug 4279: No response from proxy for FTP-download of non-existing file > > There is no code to handle errors ftp-protocol functions ftpFail(). > The patch forms a response to the client an error similar to loginFailed(). > To handle specific errors, you must add code in failedHttpStatus(). > > If you add to the template ERR_FTP_NOT_FOUND macro "%g", then the client > will be seen the original message server (550: Permission denied/File > not found) > > Patch tested on squid-3.5.9
Hello Vitaly, I am glad you were able to fix the problem, and I thank you for sharing your fix. Unfortunately, the patch itself needs significant refactoring because you are [incorrectly] duplicating an already duplicated code. As you probably know, most of the code you added already exists in Ftp::Gateway::loginFailed() and, more importantly, in Ftp::Client::failed(). We do not need a third imperfect copy of that logic! Somebody should examine your additions as well as the existing two methods and carefully merge them, keeping Ftp::Relay needs in mind. If you can help with that, I would recommend the following first steps: 0. Undo your changes (the code may be reused at step #3). 1. Add an optional ErrorState *err parameter to ftpFail() with a default NULL value. When the parameter is not nil, ftpFail() should use it instead of creating its own ErrorState. 2. Change Ftp::Gateway::loginFailed() to always call ftpFail() after trying to create an ErrorState. Move the bottom of Ftp::Gateway::loginFailed() (i.e., the code that handles non-nil err) to ftpFail(). Test the refactored code for regressions. This change will not fix the bug you are after. 3. Compare the resulting ftpFail() with the code from your patch. Add any missing actions. Test that the resulting code works fine, including handling the bug you are after. More work will be needed after the above steps to remove duplication with Ftp::Client::failed(), but these steps would move us a lot closer to that final goal. HTH, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev