Skip to content

Commit

Permalink
Add Duration APIs to specify timeouts.
Browse files Browse the repository at this point in the history
- [FTP] Add FTPClient.getDataTimeout().
- [FTP] Add FTPClient.setDataTimeout(Duration).
  • Loading branch information
garydgregory committed Feb 20, 2021
1 parent fd1bc35 commit f5edd3c
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 23 deletions.
6 changes: 6 additions & 0 deletions src/changes/changes.xml
Expand Up @@ -82,6 +82,12 @@ The <action> type attribute can be add,update,fix,remove.
<action type="add" dev="ggregory" due-to="Gary Gregory">
[FTP] Add FTPClient.setControlKeepAliveTimeout(Duration).
</action>
<action type="add" dev="ggregory" due-to="Gary Gregory">
[FTP] Add FTPClient.getDataTimeout().
</action>
<action type="add" dev="ggregory" due-to="Gary Gregory">
[FTP] Add FTPClient.setDataTimeout(Duration).
</action>
<!-- UPDATE -->
<action type="update" dev="ggregory" due-to="Dependabot">
Bump junit from 4.13.1 to 4.13.2 #74.
Expand Down
75 changes: 56 additions & 19 deletions src/main/java/org/apache/commons/net/ftp/FTPClient.java
Expand Up @@ -548,7 +548,7 @@ static String parsePathname(final String reply)
}

private int dataConnectionMode;
private int dataTimeoutMillis;
private Duration dataTimeout;

private int passivePort;
private String passiveHost;
Expand Down Expand Up @@ -645,7 +645,7 @@ static String parsePathname(final String reply)
public FTPClient()
{
initDefaults();
dataTimeoutMillis = -1;
dataTimeout = Duration.ofMillis(-1);
remoteVerificationEnabled = true;
parserFactory = new DefaultFTPFileEntryParserFactory();
configuration = null;
Expand Down Expand Up @@ -768,6 +768,7 @@ protected Socket _openDataConnection_(final String command, final String arg)

final Socket socket;

final int soTimeoutMillis = DurationUtils.toMillisInt(dataTimeout);
if (dataConnectionMode == ACTIVE_LOCAL_DATA_CONNECTION_MODE)
{
// if no activePortRange was set (correctly) -> getActivePort() = 0
Expand Down Expand Up @@ -802,14 +803,14 @@ protected Socket _openDataConnection_(final String command, final String arg)
// the data connection. It may be desirable to let this be a
// separately configurable value. In any case, we really want
// to allow preventing the accept from blocking indefinitely.
if (dataTimeoutMillis >= 0) {
server.setSoTimeout(dataTimeoutMillis);
if (soTimeoutMillis >= 0) {
server.setSoTimeout(soTimeoutMillis);
}
socket = server.accept();

// Ensure the timeout is set before any commands are issued on the new socket
if (dataTimeoutMillis >= 0) {
socket.setSoTimeout(dataTimeoutMillis);
if (soTimeoutMillis >= 0) {
socket.setSoTimeout(soTimeoutMillis);
}
if (receiveDataSocketBufferSize > 0) {
socket.setReceiveBufferSize(receiveDataSocketBufferSize);
Expand Down Expand Up @@ -861,8 +862,8 @@ protected Socket _openDataConnection_(final String command, final String arg)
// the data connection. It may be desirable to let this be a
// separately configurable value. In any case, we really want
// to allow preventing the accept from blocking indefinitely.
if (dataTimeoutMillis >= 0) {
socket.setSoTimeout(dataTimeoutMillis);
if (soTimeoutMillis >= 0) {
socket.setSoTimeout(soTimeoutMillis);
}

socket.connect(new InetSocketAddress(passiveHost, passivePort), connectTimeout);
Expand Down Expand Up @@ -1823,6 +1824,22 @@ public int getDataConnectionMode()
return dataConnectionMode;
}

/**
* Gets the timeout to use when reading from the data connection. This timeout will be set immediately after opening
* the data connection, provided that the value is &ge; 0.
* <p>
* <b>Note:</b> the timeout will also be applied when calling accept() whilst establishing an active local data
* connection.
* </p>
*
* @return The default timeout used when opening a data connection socket. The value 0 means an infinite timeout.
* @since 3.9.0
*/
public Duration getDataTimeout()
{
return dataTimeout;
}

// Method for use by unit test code only
FTPFileEntryParser getEntryParser() {
return entryParser;
Expand Down Expand Up @@ -3360,6 +3377,7 @@ public boolean retrieveFile(final String remote, final OutputStream local)
return _retrieveFile(FTPCmd.RETR.getCommand(), remote, local);
}


/**
* Returns an InputStream from which a named file from the server
* can be read. If the current file type is ASCII, the returned
Expand Down Expand Up @@ -3394,7 +3412,6 @@ public InputStream retrieveFileStream(final String remote) throws IOException
return _retrieveFileStream(FTPCmd.RETR.getCommand(), remote);
}


/**
* Sends a NOOP command to the FTP server. This is useful for preventing
* server timeouts.
Expand Down Expand Up @@ -3480,25 +3497,25 @@ public void setBufferSize(final int bufSize) {
/**
* Sets how long to wait for control keep-alive message replies.
*
* @deprecated Use {@link #setControlKeepAliveReplyTimeout(Duration)}.
* @param timeoutMillis number of milliseconds to wait (defaults to 1000)
* @param timeout number of milliseconds to wait (defaults to 1000)
* @since 3.0
* @see #setControlKeepAliveTimeout(long)
* @see #setControlKeepAliveTimeout(Duration)
*/
@Deprecated
public void setControlKeepAliveReplyTimeout(final int timeoutMillis) {
controlKeepAliveReplyTimeout = Duration.ofMillis(timeoutMillis);
public void setControlKeepAliveReplyTimeout(final Duration timeout) {
controlKeepAliveReplyTimeout = DurationUtils.zeroIfNull(timeout);
}

/**
* Sets how long to wait for control keep-alive message replies.
*
* @deprecated Use {@link #setControlKeepAliveReplyTimeout(Duration)}.
* @param timeoutMillis number of milliseconds to wait (defaults to 1000)
* @since 3.0
* @see #setControlKeepAliveTimeout(Duration)
* @see #setControlKeepAliveTimeout(long)
*/
public void setControlKeepAliveReplyTimeout(final Duration timeoutMillis) {
controlKeepAliveReplyTimeout = DurationUtils.zeroIfNull(timeoutMillis);
@Deprecated
public void setControlKeepAliveReplyTimeout(final int timeoutMillis) {
controlKeepAliveReplyTimeout = Duration.ofMillis(timeoutMillis);
}

/**
Expand Down Expand Up @@ -3544,19 +3561,39 @@ public void setCopyStreamListener(final CopyStreamListener listener){
copyStreamListener = listener;
}

/**
* Sets the timeout to use when reading from the
* data connection. This timeout will be set immediately after
* opening the data connection, provided that the value is &ge; 0.
* <p>
* <b>Note:</b> the timeout will also be applied when calling accept()
* whilst establishing an active local data connection.
* @param timeout The default timeout that is used when
* opening a data connection socket. The value 0 (or null) means an infinite timeout.
* @since 3.9.0
*/
public void setDataTimeout(final Duration timeout)
{
dataTimeout = DurationUtils.zeroIfNull(timeout);
}

/**
* Sets the timeout in milliseconds to use when reading from the
* data connection. This timeout will be set immediately after
* opening the data connection, provided that the value is &ge; 0.
* <p>
* <b>Note:</b> the timeout will also be applied when calling accept()
* whilst establishing an active local data connection.
* </p>
*
* @deprecated Use {@link #setDataTimeout(Duration)}.
* @param timeoutMillis The default timeout in milliseconds that is used when
* opening a data connection socket. The value 0 means an infinite timeout.
*/
@Deprecated
public void setDataTimeout(final int timeoutMillis)
{
dataTimeoutMillis = timeoutMillis;
dataTimeout = Duration.ofMillis(timeoutMillis);
}

/**
Expand Down
13 changes: 9 additions & 4 deletions src/test/java/org/apache/commons/net/ftp/FTPSClientTest.java
Expand Up @@ -178,13 +178,18 @@ private FTPSClient loginClient() throws SocketException, IOException {
//
client.setControlKeepAliveReplyTimeout(null);
assertEquals(0, client.getControlKeepAliveReplyTimeoutDuration().getSeconds());
client.setControlKeepAliveReplyTimeout(Duration.ofSeconds(30));
assertEquals(30, client.getControlKeepAliveReplyTimeoutDuration().getSeconds());
client.setControlKeepAliveReplyTimeout(Duration.ofSeconds(60));
assertEquals(60, client.getControlKeepAliveReplyTimeoutDuration().getSeconds());
//
client.setControlKeepAliveTimeout(null);
assertEquals(0, client.getControlKeepAliveTimeoutDuration().getSeconds());
client.setControlKeepAliveTimeout(Duration.ofSeconds(40));
assertEquals(40, client.getControlKeepAliveTimeoutDuration().getSeconds());
client.setControlKeepAliveTimeout(Duration.ofSeconds(61));
assertEquals(61, client.getControlKeepAliveTimeoutDuration().getSeconds());
//
client.setDataTimeout(null);
assertEquals(0, client.getDataTimeout().getSeconds());
client.setDataTimeout(Duration.ofSeconds(62));
assertEquals(62, client.getDataTimeout().getSeconds());
//
client.setEndpointCheckingEnabled(endpointCheckingEnabled);
client.connect("localhost", SocketPort);
Expand Down

0 comments on commit f5edd3c

Please sign in to comment.