FtpServer BUG: all exceptions in event handlers are reported with code "200"

Giganews Newsgroups
Subject: FtpServer BUG: all exceptions in event handlers are reported with code "200"
Posted by:  Carlo Sirna (carlo.sir…@digisoft.it)
Date: Wed, 27 Apr 2005

Hi I found this bug working with TidFTPServer, but the bug should be
present also in other protocol servers, since it really is in
idCommandHandlers.pas.

the problem is that if you raise an exception in any event handler of
TFTPserver, the error line you get is

200  <message of the exception you raised>

but 200 means "command successful"!

for example, take this "changedirectory" event:

procedure TMyFtpServer.ftpServerChangeDirectory(
    ASender:TIdFTPServerContext;
    var VDirectory: String);
begin
  if not DirectoryExists(VDirectory) then
    Raise Exception.Create('Path not found!');
end;

if you try to issue this command

    CWD foobar

you will get

    200 Path not found!

instead of

    550 Path not found

and your client will belive that the directory has been successfully
changed...

The problem is not in TFtpServer, as you can see from these lines from
TIdFTPServer.InitializeCommandHandlers:

  //CWD  <SP> <pathname> <CRLF>
  LCmd := CommandHandlers.Add;
  LCmd.Command := 'CWD';    {Do not Localize}
  LCmd.OnCommand := CommandCWD;
  LCmd.ExceptionReply.NumericCode := 550;
  LCmd.Description.Text := 'Syntax: CWD [ <sp> directory-name ]';

The problem resides in the class TIdCommandHandler that NEVER uses
ExceptionReply.NumericCode, since it

1) sets the reply code 200 BEFORE calling the command handler
2) in the exception handler overwrites the reply code ONLY if it is
    empty... but it is already 200!

Solution i found:

I changed this portion of TIdCommandHandler.DoCommand

------------------------------------------------------
    PerformReply := True;
    Reply.Assign(Self.NormalReply);  <<<---- HERE!!!!!!
    Reply.UpdateText;
    Response.Assign(Self.Response);
    try
      DoCommand;  <<---- THE COMMAND IS EXECUTED NOW
    except
      on E: Exception do begin
        ----- HERE STARTS THE EXCEPTION HANDLING, BUT THE ERROR CODE
        ----- IS NEVER CHANGED, BECAUSE IT IS ALREADY NOT EMPTY!
        if PerformReply then begin
          // Try from command handler first
          if Reply.Code = '' then begin
            Reply.Assign(Self.ExceptionReply);
          end;
---------------------------------------------------------

with this one:

---------------------------------------------------------

    PerformReply := True;

    //---  moved this piece of code after the DoCommand
    //  Reply.Assign(Self.NormalReply);
    //  Reply.UpdateText;
    //----
    Response.Assign(Self.Response);

    try
      DoCommand;
      // --Inserted block begins here ----------
      // now the error code is set only after having called succesfully
      // the command handler, and only if the command handlrer didn't
      // provide one
      //
      if Reply.Code = '' then
          Reply.Assign(Self.NormalReply); the

      Reply.UpdateText;

      // ---inserted block ends here -----------

    except
      on E: Exception do begin
        if PerformReply then begin
          // Try from command handler first
          if Reply.Code = '' then begin

----------------------------------------------------------------------

I don't think that this has side effects on servers other than
TidFTPServer...

Regards,
  Carlo Sirna

Replies