From 60b1bd16f28584ff8333fab3870504915ec53cd7 Mon Sep 17 00:00:00 2001 From: Gideon le Grange Date: Sun, 29 Mar 2015 12:56:23 +0200 Subject: [PATCH 1/9] Working on fix for issue #16 --- .gitignore | 3 ++ src/main/java/examples/Config.java | 2 +- src/main/java/examples/Example9.java | 28 ++++++++++ .../me/legrange/mikrotik/ApiConnection.java | 8 +-- .../mikrotik/impl/ApiConnectionImpl.java | 54 ++++++++++--------- 5 files changed, 66 insertions(+), 29 deletions(-) create mode 100644 src/main/java/examples/Example9.java diff --git a/.gitignore b/.gitignore index ae4703f..dee2255 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,6 @@ nbactions.xml .classpath .project + +# Maven +pom.xml.versionsBackup diff --git a/src/main/java/examples/Config.java b/src/main/java/examples/Config.java index 67dcab9..020c596 100644 --- a/src/main/java/examples/Config.java +++ b/src/main/java/examples/Config.java @@ -7,7 +7,7 @@ package examples; public class Config { - public static final String HOST = "10.0.1.134"; + public static final String HOST = "192.168.1.1"; public static final String USERNAME = "admin"; public static final String PASSWORD = ""; diff --git a/src/main/java/examples/Example9.java b/src/main/java/examples/Example9.java new file mode 100644 index 0000000..5870a9c --- /dev/null +++ b/src/main/java/examples/Example9.java @@ -0,0 +1,28 @@ +package examples; + +import java.util.List; +import java.util.Map; +import me.legrange.mikrotik.MikrotikApiException; + +/** + * Example 9: Test special characters in usernames + * + * @author gideon + */ +public class Example9 extends Example { + + public static void main(String... args) throws Exception { + Example9 ex = new Example9(); + ex.connect(); + ex.test(); + ex.disconnect(); + } + + private void test() throws MikrotikApiException, InterruptedException { + List> res = con.execute("/user/add name=çãáõ"); + for (Map r : res) { + System.out.println(r); + } +// con.execute("/ip/firewall/filter/add chain=forward hotspot=!auth protocol=tcp src-port=8000-8084"); + } +} diff --git a/src/main/java/me/legrange/mikrotik/ApiConnection.java b/src/main/java/me/legrange/mikrotik/ApiConnection.java index 29fa64c..9bef672 100644 --- a/src/main/java/me/legrange/mikrotik/ApiConnection.java +++ b/src/main/java/me/legrange/mikrotik/ApiConnection.java @@ -20,7 +20,7 @@ public abstract class ApiConnection { * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting */ public static ApiConnection connectTLS(String host, int port) throws MikrotikApiException { - return ApiConnectionImpl.connect(host, port, true); + return ApiConnectionImpl.connect(host, port, true, DEFAULT_CONNECTION_TIMEOUT); } @@ -31,7 +31,7 @@ public abstract class ApiConnection { * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting */ public static ApiConnection connectTLS(String host) throws MikrotikApiException { - return ApiConnectionImpl.connect(host, DEFAULT_TLS_PORT, true); + return ApiConnectionImpl.connect(host, DEFAULT_TLS_PORT, true, DEFAULT_CONNECTION_TIMEOUT); } @@ -43,7 +43,7 @@ public abstract class ApiConnection { * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting */ public static ApiConnection connect(String host, int port) throws MikrotikApiException { - return ApiConnectionImpl.connect(host, port, false); + return ApiConnectionImpl.connect(host, port, false, DEFAULT_CONNECTION_TIMEOUT); } /** @@ -104,5 +104,7 @@ public abstract class ApiConnection { private static final int DEFAULT_PORT = 8728; /** default TCP TLS port used by Mikrotik API */ private static final int DEFAULT_TLS_PORT = 8729; + /** default connection timeout to use when opening the connection */ + private static final int DEFAULT_CONNECTION_TIMEOUT = 60000; } \ No newline at end of file diff --git a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java index bee6bcb..b6dff4f 100644 --- a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java +++ b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java @@ -5,7 +5,9 @@ import java.io.DataOutputStream; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.InetAddress; +import java.net.InetSocketAddress; import java.net.Socket; +import java.net.SocketAddress; import java.net.UnknownHostException; import java.util.Arrays; import java.util.LinkedList; @@ -38,9 +40,9 @@ public final class ApiConnectionImpl extends ApiConnection { * @throws me.legrange.mikrotik.ApiConnectionException Thrown if there is a * problem connecting */ - public static ApiConnection connect(String host, int port, boolean secure) throws ApiConnectionException { + public static ApiConnection connect(String host, int port, boolean secure, int timeOut) throws ApiConnectionException { ApiConnectionImpl con = new ApiConnectionImpl(); - con.open(host, port, secure); + con.open(host, port, secure, timeOut); return con; } @@ -123,8 +125,10 @@ public final class ApiConnectionImpl extends ApiConnection { /** * cancel a command + * * @param tag - * @throws me.legrange.mikrotik.MikrotikApiException Thrown if an error is experienced while canceling the + * @throws me.legrange.mikrotik.MikrotikApiException Thrown if an error is + * experienced while canceling the */ @Override public void cancel(String tag) throws MikrotikApiException { @@ -155,23 +159,16 @@ public final class ApiConnectionImpl extends ApiConnection { this.listeners = new ConcurrentHashMap<>(); } - /** - * Start the API. Connects to the Mikrotik without using encryption - */ - private void open(String host, int port) throws ApiConnectionException { - open(host, port, false); - } - /** * Start the API. Connects to the Mikrotik */ - private void open(String host, int port, boolean secure) throws ApiConnectionException { + private void open(String host, int port, boolean secure, int conTimeout) throws ApiConnectionException { try { InetAddress ia = InetAddress.getByName(host.trim()); if (secure) { - sock = openSSLSocket(ia, port); + sock = openSSLSocket(ia, port, conTimeout); } else { - sock = new Socket(ia, port); + sock = openClearSocket(ia, port, conTimeout); } in = new DataInputStream(sock.getInputStream()); out = new DataOutputStream(sock.getOutputStream()); @@ -191,11 +188,19 @@ public final class ApiConnectionImpl extends ApiConnection { } } + private Socket openClearSocket(InetAddress ia, int port, int timeOut) throws IOException { + Socket clear = new Socket(); + SocketAddress addr = new InetSocketAddress(ia, port); + clear.connect(new InetSocketAddress(ia, port), timeOut); + return clear; + } + /** * open and configure a SSL socket. */ - private Socket openSSLSocket(InetAddress ia, int port) throws IOException { - SSLSocket ssl = (SSLSocket) SSLSocketFactory.getDefault().createSocket(ia, port); + private Socket openSSLSocket(InetAddress ia, int port, int timeOut) throws IOException { + SSLSocket ssl = (SSLSocket) SSLSocketFactory.getDefault().createSocket(); + ssl.connect(new InetSocketAddress(ia, port), timeOut); List cs = new LinkedList<>(); // not happy with this code. Without it, SSL throws a "Remote host closed connection during handshake" error // caused by a "SSL peer shut down incorrectly" error @@ -212,7 +217,7 @@ public final class ApiConnectionImpl extends ApiConnection { _tag++; return Integer.toHexString(_tag); } - private static final int DEFAULT_PORT = 8728; + private Socket sock = null; private DataOutputStream out = null; private DataInputStream in = null; @@ -318,15 +323,15 @@ public final class ApiConnectionImpl extends ApiConnection { return !lines.isEmpty() || !reader.isEmpty(); } - private String peekLine() throws ApiConnectionException, ApiDataException { - if (lines.isEmpty()) { + private String peekLine() throws ApiConnectionException, ApiDataException { + if (lines.isEmpty()) { String block = reader.take(); String parts[] = block.split("\n"); lines.addAll(Arrays.asList(parts)); } return lines.get(0); } - + private Response unpack() throws MikrotikApiException { if (line == null) { nextLine(); @@ -340,7 +345,7 @@ public final class ApiConnectionImpl extends ApiConnection { return unpackError(); case "!halt": return unpackError(); - case "" : + case "": System.out.printf("sock.isClosed() = %s, sock.isInputShutdown() = %s\n", sock.isClosed(), sock.isInputShutdown()); default: throw new ApiDataException(String.format("Unexpected line '%s'", line)); @@ -379,8 +384,8 @@ public final class ApiConnectionImpl extends ApiConnection { } return res; } - - private String unpackResult(String first )throws ApiConnectionException, ApiDataException { + + private String unpackResult(String first) throws ApiConnectionException, ApiDataException { StringBuilder buf = new StringBuilder(first); line = null; @@ -390,8 +395,7 @@ public final class ApiConnectionImpl extends ApiConnection { nextLine(); buf.append("\n"); buf.append(line); - } - else { + } else { break; } } @@ -498,7 +502,7 @@ public final class ApiConnectionImpl extends ApiConnection { private List> getResults() throws MikrotikApiException { try { - synchronized (this) { // don't wait if we already have a result. + synchronized (this) { // don't wait if we already have a result. if ((err == null) && results.isEmpty()) { wait(); } From 5d05b76fe831575169f5dc1824363a06479f6005 Mon Sep 17 00:00:00 2001 From: Gideon le Grange Date: Sun, 29 Mar 2015 13:05:50 +0200 Subject: [PATCH 2/9] Working on fix for issue #16 --- .../me/legrange/mikrotik/ApiConnection.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/main/java/me/legrange/mikrotik/ApiConnection.java b/src/main/java/me/legrange/mikrotik/ApiConnection.java index 9bef672..5fb37ad 100644 --- a/src/main/java/me/legrange/mikrotik/ApiConnection.java +++ b/src/main/java/me/legrange/mikrotik/ApiConnection.java @@ -11,6 +11,26 @@ import me.legrange.mikrotik.impl.ApiConnectionImpl; * @author GideonLeGrange */ public abstract class ApiConnection { + + /** default TCP port used by Mikrotik API */ + public static final int DEFAULT_PORT = 8728; + /** default TCP TLS port used by Mikrotik API */ + public static final int DEFAULT_TLS_PORT = 8729; + /** default connection timeout to use when opening the connection */ + public static final int DEFAULT_CONNECTION_TIMEOUT = 60000; + + /** + * Create a new API connection to the give device on the supplied port, using anonymous TLS for encryption. + * @param host The host to which to connect. + * @param port The TCP port to use. + * @param timeOut The connection timeout to use when opening the connection. + * @return The ApiConnection + * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting + */ + public static ApiConnection connectTLS(String host, int port, int timeOut) throws MikrotikApiException { + return ApiConnectionImpl.connect(host, port, true, timeOut); + } + /** * Create a new API connection to the give device on the supplied port, using anonymous TLS for encryption. @@ -34,6 +54,17 @@ public abstract class ApiConnection { return ApiConnectionImpl.connect(host, DEFAULT_TLS_PORT, true, DEFAULT_CONNECTION_TIMEOUT); } + /** + * Create a new API connection to the give device on the supplied port + * @param host The host to which to connect. + * @param port The TCP port to use. + * @param timeOut The connection timeout to use when opening the connection. + * @return The ApiConnection + * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting + */ + public static ApiConnection connect(String host, int port, int timeOut) throws MikrotikApiException { + return ApiConnectionImpl.connect(host, port, false, timeOut); + } /** * Create a new API connection to the give device on the supplied port From 9aa06f3380ee2df7dbc3a2f6b8da6c91b47eb139 Mon Sep 17 00:00:00 2001 From: Gideon le Grange Date: Sun, 29 Mar 2015 13:06:34 +0200 Subject: [PATCH 3/9] Working on fix for issue #16 --- src/main/java/me/legrange/mikrotik/ApiConnection.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/main/java/me/legrange/mikrotik/ApiConnection.java b/src/main/java/me/legrange/mikrotik/ApiConnection.java index 5fb37ad..1a11807 100644 --- a/src/main/java/me/legrange/mikrotik/ApiConnection.java +++ b/src/main/java/me/legrange/mikrotik/ApiConnection.java @@ -131,11 +131,4 @@ public abstract class ApiConnection { * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem canceling the command */ public abstract void cancel(String tag) throws MikrotikApiException; - /** default TCP port used by Mikrotik API */ - private static final int DEFAULT_PORT = 8728; - /** default TCP TLS port used by Mikrotik API */ - private static final int DEFAULT_TLS_PORT = 8729; - /** default connection timeout to use when opening the connection */ - private static final int DEFAULT_CONNECTION_TIMEOUT = 60000; - } \ No newline at end of file From c297aba43e0e5484c6ed6000ccb68ea9eae7df5e Mon Sep 17 00:00:00 2001 From: Gideon le Grange Date: Sun, 29 Mar 2015 22:34:42 +0200 Subject: [PATCH 4/9] Added command timeouts as requested in #16 --- src/main/java/examples/Config.java | 6 +- src/main/java/examples/Example.java | 2 +- src/main/java/examples/Example2.java | 1 + .../me/legrange/mikrotik/ApiConnection.java | 17 +++- .../mikrotik/impl/ApiConnectionImpl.java | 80 ++++++++----------- 5 files changed, 54 insertions(+), 52 deletions(-) diff --git a/src/main/java/examples/Config.java b/src/main/java/examples/Config.java index 020c596..57c81aa 100644 --- a/src/main/java/examples/Config.java +++ b/src/main/java/examples/Config.java @@ -7,9 +7,9 @@ package examples; public class Config { - public static final String HOST = "192.168.1.1"; - public static final String USERNAME = "admin"; - public static final String PASSWORD = ""; + public static final String HOST = "192.168.1.34"; + public static final String USERNAME = "gideon"; + public static final String PASSWORD = "minapp"; } diff --git a/src/main/java/examples/Example.java b/src/main/java/examples/Example.java index 33dea60..ff25eae 100644 --- a/src/main/java/examples/Example.java +++ b/src/main/java/examples/Example.java @@ -9,7 +9,7 @@ import me.legrange.mikrotik.ApiConnection; abstract class Example { protected void connect() throws Exception { - con = ApiConnection.connect(Config.HOST); + con = ApiConnection.connect(Config.HOST, ApiConnection.DEFAULT_PORT, 2000); con.login(Config.USERNAME, Config.PASSWORD); } diff --git a/src/main/java/examples/Example2.java b/src/main/java/examples/Example2.java index 5391d56..95f63a8 100644 --- a/src/main/java/examples/Example2.java +++ b/src/main/java/examples/Example2.java @@ -19,6 +19,7 @@ public class Example2 extends Example { } private void test() throws MikrotikApiException { + con.setTimeout(50); List> results = con.execute("/interface/print"); for (Map result : results) { System.out.println(result); diff --git a/src/main/java/me/legrange/mikrotik/ApiConnection.java b/src/main/java/me/legrange/mikrotik/ApiConnection.java index 1a11807..b9b6b1e 100644 --- a/src/main/java/me/legrange/mikrotik/ApiConnection.java +++ b/src/main/java/me/legrange/mikrotik/ApiConnection.java @@ -18,7 +18,9 @@ public abstract class ApiConnection { public static final int DEFAULT_TLS_PORT = 8729; /** default connection timeout to use when opening the connection */ public static final int DEFAULT_CONNECTION_TIMEOUT = 60000; - + /** default command timeout used for synchronous commands */ + public static final int DEFAULT_COMMAND_TIMEOUT = 60000; + /** * Create a new API connection to the give device on the supplied port, using anonymous TLS for encryption. * @param host The host to which to connect. @@ -130,5 +132,18 @@ public abstract class ApiConnection { * @param tag The tag of the command to cancel * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem canceling the command */ public abstract void cancel(String tag) throws MikrotikApiException; + + /** get the command timeout. The command timeout is used to time out API + * commands after a specific time. + * @return The time out in milliseconds. + */ + public abstract int getTimeout(); + + /** set the command timeout. The command timeout is used to time out API + * commands after a specific time. + * @param timeout The time out in milliseconds. + * @throws MikrotikApiException Thrown if the timeout specified is invalid. + */ + public abstract void setTimeout(int timeout) throws MikrotikApiException; } \ No newline at end of file diff --git a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java index b6dff4f..699a209 100644 --- a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java +++ b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java @@ -36,6 +36,7 @@ public final class ApiConnectionImpl extends ApiConnection { * @param host The host to which to connect. * @param port The TCP port to use. * @param secure Is TLS required + * @param timeOut The connection timeout * @return The ApiConnection * @throws me.legrange.mikrotik.ApiConnectionException Thrown if there is a * problem connecting @@ -46,22 +47,11 @@ public final class ApiConnectionImpl extends ApiConnection { return con; } - /** - * Check the state of connection. - * - * @return if connection is established to router it returns true. - */ @Override public boolean isConnected() { return connected; } - /** - * Disconnect from the remote API - * - * @throws me.legrange.mikrotik.ApiConnectionException Thrown if there is a - * problem disconnecting - */ @Override public void disconnect() throws ApiConnectionException { if (!connected) { @@ -77,14 +67,6 @@ public final class ApiConnectionImpl extends ApiConnection { } } - /** - * Log in to the remote router. - * - * @param username - username of the user on the router - * @param password - password for the user - * @throws me.legrange.mikrotik.MikrotikApiException - * @throws java.lang.InterruptedException - */ @Override public void login(String username, String password) throws MikrotikApiException, InterruptedException { if (username.trim().isEmpty()) { @@ -98,47 +80,40 @@ public final class ApiConnectionImpl extends ApiConnection { execute("/login name=" + username + " response=00" + chal); } - /** - * execute a command and return a list of results. - * - * @param cmd Command to execute - * @return The list of results - * @throws me.legrange.mikrotik.MikrotikApiException - */ @Override public List> execute(String cmd) throws MikrotikApiException { - return execute(Parser.parse(cmd)); + return execute(Parser.parse(cmd), timeOut); } - /** - * execute a command and attach a result listener to receive it's results. - * - * @param cmd Command to execute - * @param lis ResultListener that will receive the results - * @return A command object that can be used to cancel the command. - * @throws MikrotikApiException - */ @Override public String execute(String cmd, ResultListener lis) throws MikrotikApiException { return execute(Parser.parse(cmd), lis); } - /** - * cancel a command - * - * @param tag - * @throws me.legrange.mikrotik.MikrotikApiException Thrown if an error is - * experienced while canceling the - */ @Override public void cancel(String tag) throws MikrotikApiException { execute(String.format("/cancel tag=%s", tag)); } - private List> execute(Command cmd) throws MikrotikApiException { + @Override + public int getTimeout() { + return timeOut; + } + + @Override + public void setTimeout(int timeout) throws MikrotikApiException { + if (timeout >=0) { + timeOut = timeout; + } + else { + throw new MikrotikApiException(String.format("Invalid timeout value '%d'; must be postive or 0", timeout)); + } + } + + private List> execute(Command cmd, int timeOut) throws MikrotikApiException { SyncListener l = new SyncListener(); execute(cmd, l); - return l.getResults(); + return l.getResults(timeOut); } private String execute(Command cmd, ResultListener lis) throws MikrotikApiException { @@ -226,6 +201,7 @@ public final class ApiConnectionImpl extends ApiConnection { private Processor processor; private final Map listeners; private Integer _tag = 0; + private int timeOut = ApiConnection.DEFAULT_COMMAND_TIMEOUT; /** * thread to read data from the socket and process it into Strings @@ -483,6 +459,7 @@ public final class ApiConnectionImpl extends ApiConnection { @Override public synchronized void completed() { + complete = true; notify(); } @@ -492,6 +469,7 @@ public final class ApiConnectionImpl extends ApiConnection { res.put("ret", done.getHash()); results.add(res); } + complete = true; notify(); } @@ -500,11 +478,17 @@ public final class ApiConnectionImpl extends ApiConnection { results.add(result); } - private List> getResults() throws MikrotikApiException { + private List> getResults(int timeOut) throws MikrotikApiException { try { synchronized (this) { // don't wait if we already have a result. - if ((err == null) && results.isEmpty()) { - wait(); + int waitTime = timeOut; + while (!complete && (waitTime > 0)) { + long start = System.currentTimeMillis(); + wait(waitTime); + waitTime = waitTime - (int)(System.currentTimeMillis() - start); + if ((waitTime < 0) && !complete) { + err = new ApiConnectionException(String.format("Command timed out after %d ms", timeOut)); + } } } } catch (InterruptedException ex) { @@ -515,7 +499,9 @@ public final class ApiConnectionImpl extends ApiConnection { } return results; } + private final List> results = new LinkedList<>(); private MikrotikApiException err; + private boolean complete = false; } } From 6a73b28b59c2b05c5aee2cb6272f8ceca70dfda7 Mon Sep 17 00:00:00 2001 From: Gideon le Grange Date: Sun, 29 Mar 2015 22:40:01 +0200 Subject: [PATCH 5/9] Added command timeouts as requested in #16 --- src/main/java/examples/Config.java | 4 ++-- .../me/legrange/mikrotik/ApiConnection.java | 22 ++++++++++++++----- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/main/java/examples/Config.java b/src/main/java/examples/Config.java index 57c81aa..7497b21 100644 --- a/src/main/java/examples/Config.java +++ b/src/main/java/examples/Config.java @@ -8,8 +8,8 @@ public class Config { public static final String HOST = "192.168.1.34"; - public static final String USERNAME = "gideon"; - public static final String PASSWORD = "minapp"; + public static final String USERNAME = "admin"; + public static final String PASSWORD = ""; } diff --git a/src/main/java/me/legrange/mikrotik/ApiConnection.java b/src/main/java/me/legrange/mikrotik/ApiConnection.java index b9b6b1e..a69394e 100644 --- a/src/main/java/me/legrange/mikrotik/ApiConnection.java +++ b/src/main/java/me/legrange/mikrotik/ApiConnection.java @@ -25,12 +25,12 @@ public abstract class ApiConnection { * Create a new API connection to the give device on the supplied port, using anonymous TLS for encryption. * @param host The host to which to connect. * @param port The TCP port to use. - * @param timeOut The connection timeout to use when opening the connection. + * @param timeout The connection timeout to use when opening the connection. * @return The ApiConnection * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting */ - public static ApiConnection connectTLS(String host, int port, int timeOut) throws MikrotikApiException { - return ApiConnectionImpl.connect(host, port, true, timeOut); + public static ApiConnection connectTLS(String host, int port, int timeout) throws MikrotikApiException { + return ApiConnectionImpl.connect(host, port, true, timeout); } @@ -60,12 +60,12 @@ public abstract class ApiConnection { * Create a new API connection to the give device on the supplied port * @param host The host to which to connect. * @param port The TCP port to use. - * @param timeOut The connection timeout to use when opening the connection. + * @param timeout The connection timeout to use when opening the connection. * @return The ApiConnection * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting */ - public static ApiConnection connect(String host, int port, int timeOut) throws MikrotikApiException { - return ApiConnectionImpl.connect(host, port, false, timeOut); + public static ApiConnection connect(String host, int port, int timeout) throws MikrotikApiException { + return ApiConnectionImpl.connect(host, port, false, timeout); } /** @@ -135,12 +135,22 @@ public abstract class ApiConnection { /** get the command timeout. The command timeout is used to time out API * commands after a specific time. + * + * Note: This is not the same as the timeout value passed in the connect() and + * connectTLS() methods. This timeout is specific to synchronous commands, that + * timeout is applied to opening the API socket. + * * @return The time out in milliseconds. */ public abstract int getTimeout(); /** set the command timeout. The command timeout is used to time out API * commands after a specific time. + * + * Note: This is not the same as the timeout value passed in the connect() and + * connectTLS() methods. This timeout is specific to synchronous commands, that + * timeout is applied to opening the API socket. + * * @param timeout The time out in milliseconds. * @throws MikrotikApiException Thrown if the timeout specified is invalid. */ From afeeec67db863c137a67877ddcff4fd5273f0d28 Mon Sep 17 00:00:00 2001 From: Gideon le Grange Date: Sun, 29 Mar 2015 23:14:32 +0200 Subject: [PATCH 6/9] Some cleanups. Fixed timeout corner case in #16 --- .../mikrotik/impl/ApiConnectionImpl.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java index 699a209..43c75d7 100644 --- a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java +++ b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java @@ -82,7 +82,7 @@ public final class ApiConnectionImpl extends ApiConnection { @Override public List> execute(String cmd) throws MikrotikApiException { - return execute(Parser.parse(cmd), timeOut); + return execute(Parser.parse(cmd), timeout); } @Override @@ -97,23 +97,23 @@ public final class ApiConnectionImpl extends ApiConnection { @Override public int getTimeout() { - return timeOut; + return timeout; } @Override public void setTimeout(int timeout) throws MikrotikApiException { - if (timeout >=0) { - timeOut = timeout; + if (timeout > 0) { + this.timeout = timeout; } else { - throw new MikrotikApiException(String.format("Invalid timeout value '%d'; must be postive or 0", timeout)); + throw new MikrotikApiException(String.format("Invalid timeout value '%d'; must be postive", timeout)); } } - private List> execute(Command cmd, int timeOut) throws MikrotikApiException { + private List> execute(Command cmd, int timeout) throws MikrotikApiException { SyncListener l = new SyncListener(); execute(cmd, l); - return l.getResults(timeOut); + return l.getResults(timeout); } private String execute(Command cmd, ResultListener lis) throws MikrotikApiException { @@ -201,7 +201,7 @@ public final class ApiConnectionImpl extends ApiConnection { private Processor processor; private final Map listeners; private Integer _tag = 0; - private int timeOut = ApiConnection.DEFAULT_COMMAND_TIMEOUT; + private int timeout = ApiConnection.DEFAULT_COMMAND_TIMEOUT; /** * thread to read data from the socket and process it into Strings @@ -478,16 +478,16 @@ public final class ApiConnectionImpl extends ApiConnection { results.add(result); } - private List> getResults(int timeOut) throws MikrotikApiException { + private List> getResults(int timeout) throws MikrotikApiException { try { synchronized (this) { // don't wait if we already have a result. - int waitTime = timeOut; + int waitTime = timeout; while (!complete && (waitTime > 0)) { long start = System.currentTimeMillis(); wait(waitTime); waitTime = waitTime - (int)(System.currentTimeMillis() - start); - if ((waitTime < 0) && !complete) { - err = new ApiConnectionException(String.format("Command timed out after %d ms", timeOut)); + if ((waitTime <= 0) && !complete) { + err = new ApiConnectionException(String.format("Command timed out after %d ms", timeout)); } } } From b02bf2eaeee3e1cc16ee623da63e4ee666d66871 Mon Sep 17 00:00:00 2001 From: GideonLeGrange Date: Mon, 30 Mar 2015 16:27:21 +0200 Subject: [PATCH 7/9] Removed old debug printf and improved disconnect() --- src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java index 43c75d7..af2d7dd 100644 --- a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java +++ b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java @@ -61,6 +61,8 @@ public final class ApiConnectionImpl extends ApiConnection { processor.interrupt(); reader.interrupt(); try { + in.close(); + out.close(); sock.close(); } catch (IOException ex) { throw new ApiConnectionException(String.format("Error closing socket: %s", ex.getMessage()), ex); @@ -322,7 +324,6 @@ public final class ApiConnectionImpl extends ApiConnection { case "!halt": return unpackError(); case "": - System.out.printf("sock.isClosed() = %s, sock.isInputShutdown() = %s\n", sock.isClosed(), sock.isInputShutdown()); default: throw new ApiDataException(String.format("Unexpected line '%s'", line)); } From eac62b0945ce7ece85f6fe01d9e9953f55af7ead Mon Sep 17 00:00:00 2001 From: GideonLeGrange Date: Mon, 30 Mar 2015 16:43:00 +0200 Subject: [PATCH 8/9] Updated README with information about the timeouts --- README.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/README.md b/README.md index 8f9da69..3168cc0 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,21 @@ To open an encrypted (TLS) connection is as simple, assuming the default API-SSL ApiConnection con = ApiConnection.connectTLS("10.0.1.1"); // connect to router using TLS ``` +By default, the API will generate an exception if it cannot connect to the specified router. This can take place immediately (typically if the router returns a 'Connection refused' error), but can also take up to 60 seconds if the router host is firewalled or if there are other network problems. This 60 seconds is the 'default connection timeout' an can be overridded by passing the preferred timeout to the APi as last parameter in a ```connect()``` or ```connectTLS()``` call. Here is the non-TLS example: + +```java + ApiConnection con = ApiConnection.connect("10.0.1.1", ApiConnection.DEFAULT_PORT, 2000); // connect to router on the default API port and fail in 2 seconds +``` + +Connecting using TLS is similar: + +```java + ApiConnection con = ApiConnection.connect("10.0.1.1", ApiConnection.DEFAULT_TLS_PORT, 2000); // connect to router on the default TLS API port and fail in 2 seconds +``` + +Note that ```ApiConnection.DEFAULT_PORT``` and ```ApiConnection.DEFAULT_TLS_PORT``` are provided to allow users who use the default ports to safely use the overloaded timeout method. + + #### Notes about TLS: * Currently only anonymous TLS is supported, not certificates. * There is a compatibility problem between the current versions of RouterOS supporting API over TLS and the Java Cryptography Extension (JCE) in Java 7 and earlier. TLS encryption works in Java 8 and later. For more information, feel free to contact me. @@ -185,6 +200,21 @@ con.cancel(tag); From version 2.0.0 of the API the error() and completed() methods are part of the ResultListener interface. +Command timeouts +---------------- + +Command timeouts can be used to make sure that synchronous commands either return or fail within a specific time. Command timeouts are separate from the connection timeout used in ```connect()``` and ```connectTLS()```, and can be set using ```setTimeout()```. Here is an example: + +```java +ApiConnection con = ApiConnection.connect("10.0.1.1"); // connect to router +con.setTimeout(5000); // set command timeout to 5 seconds +con.login("admin","password"); // log in to router +con.execute("/system/reboot"); // execute a command +``` +It is important to note that command timeouts can be set before ```login()``` is called, and can therefore influence the behaviour of login. + +The default command timeout, if none is set by the user, is 60 seconds. + References ========== From d86e59c340c4cecacba41a1b2772cbd3ce8b7acc Mon Sep 17 00:00:00 2001 From: GideonLeGrange Date: Mon, 30 Mar 2015 16:47:35 +0200 Subject: [PATCH 9/9] Removed getTimeout() until somebody wants it. Don't want bloat --- src/main/java/me/legrange/mikrotik/ApiConnection.java | 10 ---------- .../me/legrange/mikrotik/impl/ApiConnectionImpl.java | 5 ----- 2 files changed, 15 deletions(-) diff --git a/src/main/java/me/legrange/mikrotik/ApiConnection.java b/src/main/java/me/legrange/mikrotik/ApiConnection.java index a69394e..47ac836 100644 --- a/src/main/java/me/legrange/mikrotik/ApiConnection.java +++ b/src/main/java/me/legrange/mikrotik/ApiConnection.java @@ -133,16 +133,6 @@ public abstract class ApiConnection { * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem canceling the command */ public abstract void cancel(String tag) throws MikrotikApiException; - /** get the command timeout. The command timeout is used to time out API - * commands after a specific time. - * - * Note: This is not the same as the timeout value passed in the connect() and - * connectTLS() methods. This timeout is specific to synchronous commands, that - * timeout is applied to opening the API socket. - * - * @return The time out in milliseconds. - */ - public abstract int getTimeout(); /** set the command timeout. The command timeout is used to time out API * commands after a specific time. diff --git a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java index af2d7dd..2fa6ae8 100644 --- a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java +++ b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java @@ -97,11 +97,6 @@ public final class ApiConnectionImpl extends ApiConnection { execute(String.format("/cancel tag=%s", tag)); } - @Override - public int getTimeout() { - return timeout; - } - @Override public void setTimeout(int timeout) throws MikrotikApiException { if (timeout > 0) {