[squid-dev] [PATCH] squid-3.5: fix for errors when receiving a non-existent file via ftp

[Vitaly Lavrov]

On 12.10.2015 18:40, Amos Jeffries wrote:
> On 12/10/2015 9:21 p.m., Vitaly Lavrov wrote:
>> On 11.10.2015 18:56, Amos Jeffries wrote:
>>> Attached is a cleaned version of the patch. It now meets the source
>>> coding guidelines, and applies cleanly to trunk / Squid-4.
>>>
>>> I took the opportunity to remove two useless if() conditions - one was
>>> duplicate checking a value, the other for values in a pointer that was
>>> just new()'d.
>>>
>>>
>>> Logic looks mostly okay, but this part in the middle of
>>> Ftp::Gateway::loginFailed() is rather odd:
>>>
>>> -    // any other problems are general falures.
>>> -    if (!err) {
>>> -        ftpFail(this);
>>> +    if (!err)
>>>            return;
>>> -    }
>>>
>>> The original behaviour was to output an error and perform the ftpQuit()
>>> sequence. Now it appears to just halt without doing anything.
>>>
>>> Is that behaviour change intentional? I suspect the correct behaviour is
>>> to remove that if()-condition entirely, but I'm unsure.
>> It is possible that  ftpFail(this) was deleted accidentally during
>> multiple rework code.
>> The original version "if(!err) { ftpFail(this); return; }" looks more
>> logical.
>>
>
> Could we get that tested please before I apply the patch to Squid-4.
All right. In the case of non-standard error code ftpFail() returns an error to the client ERR_FTP_FAILED/502.
>
> Thanks
> Amos
>