Commons Net
  1. Commons Net
  2. NET-148

Relaxed condition in __getReply causes other failures.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.4, Nightly Builds
    • Fix Version/s: 2.0
    • Component/s: None
    • Labels:
      None

      Description

      In FTP.java's __getReply() method, this do/while loop reads multi-line responses from the server:

      do

      { line = _controlInput.readLine(); ... }

      while (!(line.length() >= 4 && line.charAt(3) != '-' &&
      Character.isDigit(line.charAt(0))));
      // This is too strong a condition because of non-conforming ftp
      // servers like ftp.funet.fi which sent 226 as the last line of a
      // 426 multi-line reply in response to ls /. We relax the condition to
      // test that the line starts with a digit rather than starting with
      // the code.
      // line.startsWith(code)));
      }

      Note the comment and the commented-out termination condition. I think the relevant spec is http://www.ietf.org/rfc/rfc0959.txt and the section is "4.2. FTP REPLIES". This is causing problems with the return from the STAT command from Geocities' FTP servers. Here is an example reply.

      211- ftp.us.geocities.com FTP server status:
      Version wu-2.6.0(48) Tue Jan 2 16:30:15 PST 2007
      Connected to 144.212.217.85
      Logged in anonymously
      TYPE: ASCII, FORM: Nonprint; STRUcture: File; transfer MODE: Stream
      No data connection
      0 data bytes received in 0 files
      0 data bytes transmitted in 0 files
      0 data bytes total in 0 files
      57 traffic bytes received in 0 transfers
      733 traffic bytes transmitted in 0 transfers
      834 traffic bytes total in 0 transfers
      211 End of status

      Note that the line "0 data bytes total in 0 files" starts with a digit, but it isn't a reply code. This prematurely halts reading of lines from the server, and the remaining lines will look like a reply from the next command.

        Issue Links

          Activity

          Hide
          Rory Winston added a comment -

          This is now fixed. In order to get the broken test case to work, add the following line before connect():

          ftpClient.setStrictMultilineParsing(true);

          This will enable the client to parse the responses correctly.

          Show
          Rory Winston added a comment - This is now fixed. In order to get the broken test case to work, add the following line before connect(): ftpClient.setStrictMultilineParsing(true); This will enable the client to parse the responses correctly.
          Hide
          Rory Winston added a comment -

          Ok, I think the best way to do this may be to revert to the previous (more strict) behaviour for default operation, but add a parameter (settable on the FTPClient) which allows the user to enable strict or lenient multiline parsing.

          Show
          Rory Winston added a comment - Ok, I think the best way to do this may be to revert to the previous (more strict) behaviour for default operation, but add a parameter (settable on the FTPClient) which allows the user to enable strict or lenient multiline parsing.
          Hide
          Rory Winston added a comment -

          Thanks Matthew , that is very helpful. I can reproduce the issue now. I think we may need to modify that condition to fix this.

          Show
          Rory Winston added a comment - Thanks Matthew , that is very helpful. I can reproduce the issue now. I think we may need to modify that condition to fix this.
          Hide
          Matthew Simoneau added a comment -

          This is still broken in the nightly build. Here is a simple Java program that will reproduce this.

          import org.apache.commons.net.ftp.FTPClient;

          public class FTPTest {
          public static void main(String[] args) throws Exception

          { FTPClient ftpClient = new FTPClient(); ftpClient.connect("ftp.us.geocities.com",21); ftpClient.login("anonymous","anonymous@example.com"); ftpClient.getStatus(); System.out.println(ftpClient.listFiles()); ftpClient.disconnect(); }

          }

          getStatus() stops reading lines prematurely, which sets up listFiles() to fail.

          Show
          Matthew Simoneau added a comment - This is still broken in the nightly build. Here is a simple Java program that will reproduce this. import org.apache.commons.net.ftp.FTPClient; public class FTPTest { public static void main(String[] args) throws Exception { FTPClient ftpClient = new FTPClient(); ftpClient.connect("ftp.us.geocities.com",21); ftpClient.login("anonymous","anonymous@example.com"); ftpClient.getStatus(); System.out.println(ftpClient.listFiles()); ftpClient.disconnect(); } } getStatus() stops reading lines prematurely, which sets up listFiles() to fail.
          Hide
          Rory Winston added a comment -

          I have tried this with Geocities servers, but I cant reproduce the issue with 2.0 RC? Can you provide some more info?

          Show
          Rory Winston added a comment - I have tried this with Geocities servers, but I cant reproduce the issue with 2.0 RC? Can you provide some more info?

            People

            • Assignee:
              Rory Winston
              Reporter:
              Matthew Simoneau
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development