Merge pull request #6 from GideonLeGrange/parser-fix

Fixed parser problems described in issues #3, #4 and #5

README.md

@@ -8,9 +8,10 @@ This project provides a Java client to manipulate Mikrotik routers using the rem
 Versions
 --------
 
-The current stable version 1.1.2, which adds support for handling multi-line results, like for example /file print. 
+The current stable version 1.1.3, which fixes two severe command line parsing bugs, #3 and #4
 
-Version 1.1 added TLS (SSL) support to encrypt API traffic. 
+* 1.1.2 added support for handling multi-line results, like for example /file print. 
+* 1.1 added TLS (SSL) support to encrypt API traffic. 
 
 
 Examples

src/main/java/examples/Config.java

@@ -7,7 +7,7 @@ package examples;
 public class Config {
     
     
-   public static final String HOST = "10.0.1.3";
+   public static final String HOST = "10.0.1.134";
     public static final String USERNAME = "admin";
     public static final String PASSWORD = "";
     

src/main/java/examples/Example6.java

@@ -17,8 +17,13 @@ public class Example6 extends Example {
     }
 
     private void test() throws MikrotikApiException, InterruptedException {
-        con.execute("/interface/gre/add remote-address=10.0.1.1 name=gre1 keepalive=10");
+        System.out.println("Creating interface gre1");
+        con.execute("/interface/gre/add remote-address=1.2.3.4 name=gre1 keepalive=10 comment='test comment'");
+        System.out.println("Adding firewall rule for interface gre1");
+        con.execute("/ip/firewall/filter/add action=drop chain=forward in-interface=gre1 protocol=udp dst-port=78,80");//,80,32");
+        System.out.println("Waiting 10 seconds");
         Thread.sleep(10000); // 10 seconds for the user to look on the router to see the interface with /interface gre print
+        System.out.println("Changing IP for interface gre1");
         con.execute("/interface/gre/set remote-address=172.16.1.1 .id=gre1");
         // now look again and the IP has changed
     }

src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java

@@ -486,7 +486,7 @@ public final class ApiConnectionImpl extends ApiConnection {
                 throw new ApiConnectionException(ex.getMessage(), ex);
             }
             if (err != null) {
-                throw err;
+                throw new MikrotikApiException(err.getMessage(), err);
             }
             return results;
         }

src/main/java/me/legrange/mikrotik/impl/Parser.java

@@ -1,11 +1,9 @@
 package me.legrange.mikrotik.impl;
 
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
-import java.util.StringTokenizer;
+import me.legrange.mikrotik.impl.Scanner.Token;
 
 /**
  * Parse the pseudo-command line into command objects. 
@@ -21,11 +19,8 @@ class Parser {
 
     /** run parse on the internal data and return the command object */
     private Command parse() throws ParseException {
-        next();
-        expect(Token.COMMAND);
-        cmd = new Command(text);
-        next();
-        while (!((token == Token.WHERE) || (token == Token.RETURN) || (token == Token.EOL))) {
+        command();
+        while (!is(Token.WHERE, Token.RETURN, Token.EOL)) { 
             param();
         }
         if (token == Token.WHERE) {
@@ -38,14 +33,35 @@ class Parser {
         return cmd;
     }
     
+    private void command() throws ParseException {
+        StringBuilder path = new StringBuilder();
+        do {
+            expect(Token.SLASH);
+            path.append("/");
+            next();
+            expect(Token.TEXT);
+            path.append(text);
+            next();
+        } while (token == Token.SLASH);
+        cmd = new Command(path.toString());
+    }
+
     private void param() throws ParseException {
         String name = text;
         next();
         if (token == Token.EQUALS) {
             next();
-            expect(Token.NAME);
-            cmd.addParameter(new Parameter(name, text));
+            expect(Token.TEXT);
+            StringBuilder val = new StringBuilder(text);
             next();
+            while (is(Token.COMMA, Token.SLASH)) {
+                val.append(token);
+                next();
+                expect(Token.TEXT);
+                val.append(text);
+                next();
+            }
+            cmd.addParameter(new Parameter(name, val.toString()));
         }
         else {
             cmd.addParameter(new Parameter(name));
@@ -58,12 +74,12 @@ class Parser {
     }
 
     private void expr() throws ParseException {
-        expect(Token.NOT, Token.NAME);
+        expect(Token.NOT, Token.TEXT);
         switch (token) {
             case NOT:
                 notExpr();
                 break;
-            case NAME: {
+            case TEXT: {
                 String name = text;
                 next();
                 expect(Token.EQUALS, Token.LESS, Token.MORE);
@@ -110,19 +126,20 @@ class Parser {
         cmd.addQuery("?#!");
     }
 
-    private void eqExpr(String name) {
+    private void eqExpr(String name) throws  ParseException {
         next(); // eat = 
+        expect(Token.TEXT);
         cmd.addQuery(String.format("?%s=%s", name, text));
         next();
     }
 
-    private void lessExpr(String name) {
+    private void lessExpr(String name) throws ScanException {
         next(); // eat < 
         cmd.addQuery(String.format("?<%s=%s", name, text));
         next();
     }
 
-    private void moreExpr(String name) {
+    private void moreExpr(String name) throws ScanException {
         next(); // eat >
         cmd.addQuery(String.format("?>%s=%s", name, text));
         next();
@@ -134,8 +151,8 @@ class Parser {
 
     private void returns() throws ParseException {
         next();
-        expect(Token.NAME);
-        List<String> props = new LinkedList<String>();
+        expect(Token.TEXT);
+        List<String> props = new LinkedList<>();
         while (!(token == Token.EOL)) {
             if (token != Token.COMMA) {
                 props.add(text);
@@ -146,63 +163,35 @@ class Parser {
     }
 
     private void expect(Token...tokens) throws ParseException {
+        if (!is(tokens)) 
+            throw new ParseException(String.format("Expected %s but found %s at position %d", Arrays.asList(tokens), this.token, scanner.pos()));
+    }
+    
+    private boolean is(Token...tokens) {
         for (Token want : tokens) {
-            if (this.token == want) return;
+            if (this.token == want) return true;
         }
-        throw new ParseException(String.format("Expected %s but found %s", Arrays.asList(tokens), this.token));
+        return false;
     }
 
-    private void next() {
-        if (!words.isEmpty()) {
-            text = words.remove(0);
-            if (text.startsWith("/")) {
-                token = Token.COMMAND;
-            } else {
-                Token t = lookup.get(text);
-                if (t != null) {
-                    token = t;
-                    text = "";
-                } else {
-                    token = Token.NAME;
-                }
-            }
-        } else {
-            token = Token.EOL;
-            text = "";
+    /** move to the next token returned by the scanner */
+    private void next() throws ScanException {
+        token = scanner.next();
+        while (token == Token.WS) {
+            token = scanner.next();
         }
-       // System.out.printf("%s: %s\n", token, text);
+        text = scanner.text();
     }
 
-    private Parser(String text) {
-        text = text.trim();
-        StringTokenizer st = new StringTokenizer(text, " \t,=", true);
-        while (st.hasMoreElements()) {
-            String t = st.nextToken().trim();
-            if (!t.equals("")) {
-                words.add(t);
-            }
-        }
+    private Parser(String line) throws ScanException {
+        line = line.trim();
+        scanner = new Scanner(line);
+        next();
     }
-    private final List<String> words = new LinkedList<String>();
-    private String text;
+    
+    private final Scanner scanner;
     private Token token;
+    private String text;
     private Command cmd;
-    private static final Map<String, Token> lookup = new HashMap<String, Token>();
 
-    private enum Token {
-
-        COMMAND, WHERE, RETURN, EOL, NOT, AND, OR, NAME, EQUALS, MORE, LESS, COMMA;
-    }
-
-    static {
-        lookup.put("where", Token.WHERE);
-        lookup.put("return", Token.RETURN);
-        lookup.put("not", Token.NOT);
-        lookup.put("and", Token.AND);
-        lookup.put("or", Token.OR);
-        lookup.put("=", Token.EQUALS);
-        lookup.put(">", Token.MORE);
-        lookup.put("<", Token.LESS);
-        lookup.put(",", Token.COMMA);
-    }
 }

src/main/java/me/legrange/mikrotik/impl/ScanException.java

@@ -0,0 +1,21 @@
+package me.legrange.mikrotik.impl;
+
+import me.legrange.mikrotik.MikrotikApiException;
+
+/**
+ * Exception thrown if the scanner encounters an error while scanning a command line.
+ * @author GideonLeGrange
+ */
+public class ScanException extends ParseException {
+
+    ScanException(String msg) {
+        super(msg);
+    }
+
+    ScanException(String msg, Throwable err) {
+        super(msg, err);
+    }
+    
+    
+    
+}

src/main/java/me/legrange/mikrotik/impl/Scanner.java

@@ -0,0 +1,156 @@
+/*
+ * Copyright 2014 GideonLeGrange.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package me.legrange.mikrotik.impl;
+
+/**
+ * A simple scanner.
+ * @author gideon
+ */
+class Scanner {
+    
+    enum Token {
+        SLASH("/"), COMMA(","), EOL(), WS, TEXT, 
+        LESS("<"), MORE(">"), EQUALS("="),
+        WHERE, NOT, AND, OR, RETURN;
+
+        @Override
+        public String toString() {
+            return (symb == null) ? name() : symb;
+        }
+
+        private Token(String symb) {
+            this.symb = symb;
+        }
+        
+        private Token() {
+            symb = null;
+        }
+        
+        
+        private final String symb;
+     }
+    
+    /** create a scanner for the given line of text */
+    Scanner(String line) {
+        this.line = line;
+        nextChar();
+    }
+    
+    /** return the next token from the text */
+    Token next() throws ScanException {
+        text = null;
+        switch (c) {
+            case '\n' : return Token.EOL;
+            case ' '  : 
+            case '\t' : 
+                return whiteSpace();
+            case ',' :
+                nextChar();
+                return Token.COMMA;
+            case '/' : 
+                nextChar(); 
+                return Token.SLASH;
+            case '<' : 
+                nextChar(); 
+                return Token.LESS;
+            case '>' : 
+                nextChar(); 
+                return Token.MORE;
+            case '=' : 
+                nextChar(); 
+                return Token.EQUALS;
+            case '"' : 
+                return quotedText('"');
+            case '\'' : 
+                return quotedText('\'');
+            default :
+                return name();
+        }
+        
+    }
+    
+    /** return the text associated with the last token returned */
+    String text()  {
+        if (text != null) return text.toString(); 
+        return "";
+    }
+    
+    /** return the position of the scanner */
+    int pos() { return pos; }
+    
+    /** process 'name' tokens which could be key words or text */
+    private Token name() throws ScanException {
+        text = new StringBuilder();
+        while (in(c,"[A-Za-z0-9-\\.]")) {
+            text.append(c);
+            nextChar();
+        }
+        String val = text.toString().toLowerCase();
+        switch  (val) {
+            case "where" : return Token.WHERE;
+            case "not" : return Token.NOT;
+            case "and" : return Token.AND;
+            case "or" : return Token.OR;
+            case "return" : return Token.RETURN;
+        }
+        return Token.TEXT;
+    }
+    
+    /** process quoted text */
+    private Token quotedText(char quote) throws ScanException {
+        nextChar(); // eat the '"'
+        text = new StringBuilder();
+        while (c != quote) {
+            if (c == '\n') {
+                throw new ScanException("Unclosed quoted text, reached end of line.");
+            }
+            text.append(c);
+            nextChar();
+        }
+        nextChar(); // eat the '"'
+        return Token.TEXT;
+    }
+    
+    /** process white space */
+    private Token whiteSpace() {
+        while ((c == ' ') || (c == '\t')) {
+            nextChar();
+        }
+        return Token.WS;
+    }
+    
+    /** return the next character from the line of text */
+    private void nextChar() {
+        if (pos < line.length()) {
+           c = line.charAt(pos);
+            pos ++;
+        }
+        else {
+            c = '\n';
+        }
+    }
+    
+    /** check if the character matches the give expression */
+    private boolean in(char c, String cs) {
+        return ("" + c).matches(cs);
+    }
+    
+    private final String line;
+    private int pos = 0;
+    private char c;
+    private StringBuilder text;
+}