Commons Net
  1. Commons Net
  2. NET-346

FTP should support reporting NATed external IP address

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 3.1
    • Component/s: FTP
    • Labels:
      None

      Description

      When trying to do an active FTP transfer as client from behind a firewall with NAT, setActiveExternalIPAddress is not sufficient, as you can only report the internal IP of the client (e.g. 192.168.1.2 vs 72.14.X.X)

      FTPClient should support an additional function to set the REPORTABLE external active IP Address

      I created and successfully tested a fix for this:

      ADD PRIVATE MEMBER:
      private InetAddress __reportActiveExternalHost;

      IN __initDefaults():
      __reportActiveExternalHost = null;

      ADD PUBLIC FUNCTIONS:
      private InetAddress getReportHostAddress()
      {
      if (__reportActiveExternalHost != null)

      { return __reportActiveExternalHost ; }

      else if (__activeExternalHost != null)

      { return __activeExternalHost; }

      else

      { // default local address return getLocalAddress(); }

      }

      public void setReportActiveExternalIPAddress(String ipAddress) throws UnknownHostException

      { this.__reportActiveExternalHost = InetAddress.getByName(ipAddress); }

      IN openDataConnection:

      if (isInet6Address)
      {
      if (!FTPReply.isPositiveCompletion(eprt(getReportHostAddress(), server.getLocalPort())))

      { server.close(); return null; }
      }
      else
      {
      if (!FTPReply.isPositiveCompletion(port(getReportHostAddress(), server.getLocalPort())))
      { server.close(); return null; }

      }

      will also attach changed file

      sorry I am not familiar with the correct way to submit patches, although I tried to model this on #NET-285 as much as possible

      1. ftpclient.patch
        3 kB
        Sebb
      2. FTPClient.java
        120 kB
        Kevin Samuel

        Activity

        Hide
        Sebb added a comment -

        Applied, with a minor change: getReportHostAddress now defers to getHostAddress if __reportActiveExternalHost is null.

        Show
        Sebb added a comment - Applied, with a minor change: getReportHostAddress now defers to getHostAddress if __reportActiveExternalHost is null.
        Hide
        Keigo Tanaka added a comment -

        According to HEAD of FTPClient.java(rev 1126692), when openDataConnection() called, it tries to open socket with createServerSocket on line 648, and it causes java.net.BindException.
        setActiveExternalIPAddress() set the address for PORT command, however, the address is also used for binding the actual socket in openDataConnection().
        This causes the problem when the client is inside of NAT.
        When using NAT, the gateway has a global IP address, and the client has a private IP address.
        The Exception from setActiveExternalIPAddress is caused by the mismatch of the global IP address and the private IP address.
        Because the client host does not have the global IP address as the local address, the client will fail to bind the address.
        I believe this patch is useful for such situation. Please merge it into the HEAD branch.

        Show
        Keigo Tanaka added a comment - According to HEAD of FTPClient.java(rev 1126692), when openDataConnection () called, it tries to open socket with createServerSocket on line 648, and it causes java.net.BindException. setActiveExternalIPAddress() set the address for PORT command, however, the address is also used for binding the actual socket in openDataConnection (). This causes the problem when the client is inside of NAT. When using NAT, the gateway has a global IP address, and the client has a private IP address. The Exception from setActiveExternalIPAddress is caused by the mismatch of the global IP address and the private IP address. Because the client host does not have the global IP address as the local address, the client will fail to bind the address. I believe this patch is useful for such situation. Please merge it into the HEAD branch.
        Hide
        Sebb added a comment -

        For reference: diff between patch and NET 2.2 versions of FTPClient.java

        Show
        Sebb added a comment - For reference: diff between patch and NET 2.2 versions of FTPClient.java
        Hide
        Sebb added a comment -

        What happens if you use

        ftp.setActiveExternalIPAddress("X.X.X.X");
        

        Does that not work?

        Show
        Sebb added a comment - What happens if you use ftp.setActiveExternalIPAddress( "X.X.X.X" ); Does that not work?
        Hide
        Kevin Samuel added a comment - - edited

        test code:

        boolean rc = false;

        String localFile = "test.txt";
        String remoteFile = "test.txt";

        FTPClient ftp = new FTPClient();

        ftp.connect( ftpServer );

        ftp.setActiveExternalIPAddress("192.168.1.5");
        ftp.setReportActiveExternalIPAddress("X.X.X.X");
        ftp.setActivePortRange(6000, 7000);

        rc = ftp.login( ftpUser , ftpPass );
        if(!rc)

        {ftp.disconnect();return;}

        rc = ftp.retrieveFile(remoteFile,new FileOutputStream(new File(localFile)));
        if(!rc){ftp.disconnect();return;}
        Show
        Kevin Samuel added a comment - - edited test code: boolean rc = false; String localFile = "test.txt"; String remoteFile = "test.txt"; FTPClient ftp = new FTPClient(); ftp.connect( ftpServer ); ftp.setActiveExternalIPAddress("192.168.1.5"); ftp.setReportActiveExternalIPAddress("X.X.X.X"); ftp.setActivePortRange(6000, 7000); rc = ftp.login( ftpUser , ftpPass ); if(!rc) {ftp.disconnect();return;} rc = ftp.retrieveFile(remoteFile,new FileOutputStream(new File(localFile))); if(!rc){ftp.disconnect();return;}
        Hide
        Kevin Samuel added a comment -

        again sorry this is not a proper patch file

        Show
        Kevin Samuel added a comment - again sorry this is not a proper patch file

          People

          • Assignee:
            Unassigned
            Reporter:
            Kevin Samuel
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 1h
              1h
              Remaining:
              Remaining Estimate - 1h
              1h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development