SAM: Fix parser to allow spaces in quoted values (tickets #1325, #1488)

Map keys to upper case
Catch some other parse errors

apps/sam/java/src/net/i2p/sam/SAMHandlerFactory.java

@@ -13,7 +13,6 @@ import java.net.Socket;
 import java.net.SocketTimeoutException;
 import java.nio.channels.SocketChannel;
 import java.util.Properties;
-import java.util.StringTokenizer;
 
 import net.i2p.I2PAppContext;
 import net.i2p.data.DataHelper;
@@ -41,7 +40,7 @@ class SAMHandlerFactory {
      */
     public static SAMHandler createSAMHandler(SocketChannel s, Properties i2cpProps,
                                               SAMBridge parent) throws SAMException {
-        StringTokenizer tok;
+        String line;
         Log log = I2PAppContext.getGlobalContext().logManager().getLog(SAMHandlerFactory.class);
 
         try {
@@ -49,9 +48,8 @@ class SAMHandlerFactory {
             sock.setKeepAlive(true);
             StringBuilder buf = new StringBuilder(128);
             ReadLine.readLine(sock, buf, HELLO_TIMEOUT);
-            String line = buf.toString();
             sock.setSoTimeout(0);
-            tok = new StringTokenizer(line.trim(), " ");
+            line = buf.toString();
         } catch (SocketTimeoutException e) {
             throw new SAMException("Timeout waiting for HELLO VERSION", e);
         } catch (IOException e) {
@@ -61,15 +59,13 @@ class SAMHandlerFactory {
         }
 
         // Message format: HELLO VERSION [MIN=v1] [MAX=v2]
-        if (tok.countTokens() < 2) {
+        Properties props = SAMUtils.parseParams(line);
+        if (!"HELLO".equals(props.getProperty(SAMUtils.COMMAND)) ||
+            !"VERSION".equals(props.getProperty(SAMUtils.OPCODE))) {
             throw new SAMException("Must start with HELLO VERSION");
         }
-        if (!tok.nextToken().equals("HELLO") ||
+        props.remove(SAMUtils.COMMAND);
-            !tok.nextToken().equals("VERSION")) {
+        props.remove(SAMUtils.OPCODE);
-            throw new SAMException("Must start with HELLO VERSION");
-        }
-
-        Properties props = SAMUtils.parseParams(tok);
 
         String minVer = props.getProperty("MIN");
         if (minVer == null) {

apps/sam/java/src/net/i2p/sam/SAMUtils.java

@@ -11,9 +11,9 @@ package net.i2p.sam;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Properties;
-import java.util.StringTokenizer;
 
 import net.i2p.I2PAppContext;
 import net.i2p.I2PException;
@@ -159,95 +159,176 @@ class SAMUtils {
     	return d;
     }
 
+    public static final String COMMAND = "\"\"COMMAND\"\"";
+    public static final String OPCODE = "\"\"OPCODE\"\"";
+
     /**
-     * Parse SAM parameters, and put them into a Propetries object
+     *  Parse SAM parameters, and put them into a Propetries object
      *
-     * @param tok A StringTokenizer pointing to the SAM parameters
+     *  Modified from EepGet.
+     *  All keys, major, and minor are mapped to upper case.
+     *  Double quotes around values are stripped.
      *
-     * @throws SAMException if the data was formatted incorrectly
+     *  Possible input:
-     * @return Properties with the parsed SAM params, never null
+     *<pre>
+     *  COMMAND
+     *  COMMAND OPCODE
+     *  COMMAND OPCODE [key=val]...
+     *  COMMAND OPCODE [key=" val with spaces "]...
+     *  PING
+     *  PONG
+     *  PING any   thing goes
+     *  PONG any   thing   goes
+     *
+     *  No escaping of '"' or anything else is allowed or defined
+     *  No spaces before or after '=' allowed
+     *  Keys may not be quoted
+     *  COMMAND and OPCODE may not have '='
+     *  Duplicate keys not allowed
+     *</pre>
+     *
+     *  A key without a value is not allowed by the spec, but is
+     *  returned with the value "true".
+     *
+     *  COMMAND is returned as the value of the key ""COMMAND"".
+     *  OPCODE, or the remainder of the PING/PONG line if any, is returned as the value of the key ""OPCODE"".
+     *
+     *  @param args non-null
+     *  @throws SAMException on some errors but not all
+     *  @return non-null, may be empty. Does not throw on missing COMMAND or OPCODE; caller must check.
      */
-    public static Properties parseParams(StringTokenizer tok) throws SAMException {
+    public static Properties parseParams(String args) throws SAMException {
-        int ntoks = tok.countTokens();
+        final Properties rv = new Properties();
-        Properties props = new Properties();
+        final StringBuilder buf = new StringBuilder(32);
-        
+        final int length = args.length();
-        StringBuilder value = new StringBuilder();
+        boolean isQuoted = false;
-        for (int i = 0; i < ntoks; ++i) {
+        String key = null;
-            String token = tok.nextToken();
+        // We go one past the end to force a fake trailing space
+        // to make things easier, so we don't need cleanup at the end
+        for (int i = 0; i <= length; i++) {
+            char c = (i < length) ? args.charAt(i) : ' ';
+            switch (c) {
+                case '"':
+                    if (isQuoted) {
+                        // keys never quoted
+                        if (key != null) {
+                            if (rv.setProperty(key, buf.length() > 0 ? buf.toString() : "true") != null)
+                                throw new SAMException("Duplicate parameter " + key);
+                            key = null;
+                        }
+                        buf.setLength(0);
+                    }
+                    isQuoted = !isQuoted;
+                    break;
 
-            int pos = token.indexOf("=");
+                case '\r':
-            if (pos <= 0) {
+                case '\n':
-                //_log.debug("Error in params format");
+                    break;
-                if (pos == 0) {
-                    throw new SAMException("No param specified [" + token + "]");
-                } else {
-                    throw new SAMException("Bad formatting for param [" + token + "]");
-                }
-            }
-            
-            String param = token.substring(0, pos);
-            value.append(token.substring(pos+1));
-            if (value.length() == 0)
-                throw new SAMException("Empty value for param " + param);
-            
-            // FIXME: The following code does not take into account that there
-            // may have been multiple subsequent space chars in the input that
-            // StringTokenizer treates as one.
-            if (value.charAt(0) == '"') {
-                while ( (i < ntoks) && (value.lastIndexOf("\"") <= 0) ) {
-                    value.append(' ').append(tok.nextToken());
-                    i++;
-                }
-            }
 
-            props.setProperty(param, value.toString());
+                case ' ':
-            value.setLength(0);
+                case '\b':
+                case '\f':
+                case '\t':
+                    // whitespace - if we're in a quoted section, keep this as part of the quote,
+                    // otherwise use it as a delim
+                    if (isQuoted) {
+                        buf.append(c);
+                    } else {
+                        if (key != null) {
+                            if (rv.setProperty(key, buf.length() > 0 ? buf.toString() : "true") != null)
+                                throw new SAMException("Duplicate parameter " + key);
+                            key = null;
+                        } else if (buf.length() > 0) {
+                            // key without value
+                            String k = buf.toString().trim().toUpperCase(Locale.US);
+                            if (rv.isEmpty()) {
+                                rv.setProperty(COMMAND, k);
+                                if (k.equals("PING") || k.equals("PONG")) {
+                                    // eat the rest of the line
+                                    if (i + 1 < args.length()) {
+                                        String pingData = args.substring(i + 1);
+                                        rv.setProperty(OPCODE, pingData);
+                                    }
+                                    // this will force an end of the loop
+                                    i = length + 1;
+                                }
+                            } else if (rv.size() == 1) {
+                                rv.setProperty(OPCODE, k);
+                            } else {
+                                if (rv.setProperty(k, "true") != null)
+                                    throw new SAMException("Duplicate parameter " + k);
+                            }
+                        }
+                        buf.setLength(0);
+                    }
+                    break;
+
+                case '=':
+                    if (isQuoted) {
+                        buf.append(c);
+                    } else {
+                        if (buf.length() == 0)
+                            throw new SAMException("Empty parameter name");
+                        key = buf.toString().toUpperCase(Locale.US);
+                        buf.setLength(0);
+                    }
+                    break;
+
+                default:
+                    buf.append(c);
+                    break;
+            }
         }
-
+        // nothing needed here, as we forced a trailing space in the loop
-        //if (_log.shouldLog(Log.DEBUG)) {
+        // unterminated quoted content will be lost
-        //    _log.debug("Parsed properties: " + dumpProperties(props));
+        if (isQuoted)
-        //}
+            throw new SAMException("Unterminated quote");
-
+        return rv;
-        return props;
     }
 
-    /* Dump a Properties object in an human-readable form */
-/****
-    private static String dumpProperties(Properties props) {
-        StringBuilder builder = new StringBuilder();
-        String key, val;
-        boolean firstIter = true;
-        
-        for (Map.Entry<Object, Object> entry : props.entrySet()) {
-            key = (String) entry.getKey();
-            val = (String) entry.getValue();
-            
-            if (!firstIter) {
-                builder.append(";");
-            } else {
-                firstIter = false;
-            }
-            builder.append(" \"" + key + "\" -> \"" + val + "\"");
-        }
-        
-        return builder.toString();
-    }
-****/
-    
 /****
     public static void main(String args[]) {
         try {
             test("a=b c=d e=\"f g h\"");
             test("a=\"b c d\" e=\"f g h\" i=\"j\"");
             test("a=\"b c d\" e=f i=\"j\"");
+            if (args.length == 0) {
+                System.out.println("Usage: CommandParser file || CommandParser text to parse");
+                return;
+            }
+            if (args.length > 1 || !(new java.io.File(args[0])).exists()) {
+                StringBuilder buf = new StringBuilder(128);
+                for (int i = 0; i < args.length; i++) {
+                    if (i != 0)
+                        buf.append(' ');
+                    buf.append(args[i]);
+                }
+                test(buf.toString());
+            } else {
+                java.io.InputStream in = new java.io.FileInputStream(args[0]);
+                String line;
+                while ((line = net.i2p.data.DataHelper.readLine(in)) != null) {
+                    try {
+                        test(line);
+                    } catch (Exception e) {
+                        e.printStackTrace();
+                    }
+                }
+            }
         } catch (Exception e) {
             e.printStackTrace();
         }
     }
+
     private static void test(String props) throws Exception {
-        StringTokenizer tok = new StringTokenizer(props);
+        System.out.println("Testing: " + props);
-        Properties p = parseParams(tok);
+        Properties m = parseParams(props);
-        System.out.println(p);
+        System.out.println("Found " + m.size() + " keys");
+        for (Map.Entry e : m.entrySet()) {
+            System.out.println(e.getKey() + "=[" + e.getValue() + ']');
+        }
+        System.out.println("-------------");
     }
 ****/
 }
+

apps/sam/java/src/net/i2p/sam/SAMv1Handler.java

@@ -18,7 +18,6 @@ import java.net.NoRouteToHostException;
 import java.nio.channels.SocketChannel;
 import java.nio.ByteBuffer;
 import java.util.Properties;
-import java.util.StringTokenizer;
 import java.util.concurrent.atomic.AtomicLong;
 
 import net.i2p.I2PException;
@@ -98,7 +97,6 @@ class SAMv1Handler extends SAMHandler implements SAMRawReceiver, SAMDatagramRece
         String domain = null;
         String opcode = null;
         boolean canContinue = false;
-        StringTokenizer tok;
         Properties props;
 
         this.thread.setName("SAMv1Handler " + _id);
@@ -132,32 +130,29 @@ class SAMv1Handler extends SAMHandler implements SAMRawReceiver, SAMDatagramRece
                     _log.info("Connection closed by client (line read : null)");
                     break;
                 }
-                msg = msg.trim();
 
                 if (_log.shouldLog(Log.DEBUG)) {
                     _log.debug("New message received: [" + msg + "]");
                 }
-
+                props = SAMUtils.parseParams(msg);
-                if(msg.equals("")) {
+                domain = props.getProperty(SAMUtils.COMMAND);
+                if (domain == null) {
                     if (_log.shouldLog(Log.DEBUG))
                         _log.debug("Ignoring newline");
                     continue;
                 }
-
+                opcode = props.getProperty(SAMUtils.OPCODE);
-                tok = new StringTokenizer(msg, " ");
+                if (opcode == null) {
-                if (tok.countTokens() < 2) {
-                    // This is not a correct message, for sure
                     if (_log.shouldLog(Log.DEBUG))
                         _log.debug("Error in message format");
                     break;
                 }
-                domain = tok.nextToken();
+                props.remove(SAMUtils.COMMAND);
-                opcode = tok.nextToken();
+                props.remove(SAMUtils.OPCODE);
                 if (_log.shouldLog(Log.DEBUG)) {
                     _log.debug("Parsing (domain: \"" + domain
                                + "\"; opcode: \"" + opcode + "\")");
                 }
-                props = SAMUtils.parseParams(tok);
 
                 if (domain.equals("STREAM")) {
                     canContinue = execStreamMessage(opcode, props);

apps/sam/java/src/net/i2p/sam/SAMv3DatagramServer.java

@@ -135,6 +135,7 @@ class SAMv3DatagramServer implements Handler {
 		public void run() {
 			try {
 				String header = DataHelper.readLine(is).trim();
+				// we cannot use SAMUtils.parseParams() here
 				StringTokenizer tok = new StringTokenizer(header, " ");
 				if (tok.countTokens() < 3) {
 					// This is not a correct message, for sure

apps/sam/java/src/net/i2p/sam/SAMv3Handler.java

@@ -24,7 +24,6 @@ import java.nio.channels.SocketChannel;
 import java.nio.ByteBuffer;
 import java.util.Properties;
 import java.util.HashMap;
-import java.util.StringTokenizer;
 
 import net.i2p.I2PAppContext;
 import net.i2p.I2PException;
@@ -262,7 +261,6 @@ class SAMv3Handler extends SAMv1Handler
 		String domain = null;
 		String opcode = null;
 		boolean canContinue = false;
-		StringTokenizer tok;
 		Properties props;
 
 		this.thread.setName("SAMv3Handler " + _id);
@@ -341,53 +339,46 @@ class SAMv3Handler extends SAMv1Handler
 						_log.debug("Connection closed by client (line read : null)");
 					break;
 				}
-				msg = line.trim();
 
 				if (_log.shouldLog(Log.DEBUG)) {
 					if (_log.shouldLog(Log.DEBUG))
 						_log.debug("New message received: [" + msg + "]");
 				}
-
+				props = SAMUtils.parseParams(line);
-				if(msg.equals("")) {
+				domain = props.getProperty(SAMUtils.COMMAND);
+				if (domain == null) {
 					if (_log.shouldLog(Log.DEBUG))
 						_log.debug("Ignoring newline");
 					continue;
 				}
-
+				opcode = props.getProperty(SAMUtils.OPCODE);
-				tok = new StringTokenizer(msg, " ");
+				props.remove(SAMUtils.COMMAND);
-				int count = tok.countTokens();
+				props.remove(SAMUtils.OPCODE);
-				if (count <= 0) {
+				if (_log.shouldLog(Log.DEBUG)) {
-					// This is not a correct message, for sure
+					_log.debug("Parsing (domain: \"" + domain
-					if (_log.shouldLog(Log.DEBUG))
+							+ "\"; opcode: \"" + opcode + "\")");
-						_log.debug("Ignoring whitespace");
-					continue;
 				}
-				domain = tok.nextToken();
+
 				// these may not have a second token
 				if (domain.equals("PING")) {
-					execPingMessage(tok);
+					execPingMessage(opcode);
 					continue;
 				} else if (domain.equals("PONG")) {
-					execPongMessage(tok);
+					execPongMessage(opcode);
 					continue;
 				} else if (domain.equals("QUIT") || domain.equals("STOP") ||
 				           domain.equals("EXIT")) {
 					writeString(domain + " STATUS RESULT=OK MESSAGE=bye\n");
 					break;
 				}
-				if (count <= 1) {
+
+				if (opcode == null) {
 					// This is not a correct message, for sure
 					if (writeString(domain + " STATUS RESULT=I2P_ERROR MESSAGE=\"command not specified\"\n"))
 						continue;
 					else
 						break;
 				}
-				opcode = tok.nextToken();
-				if (_log.shouldLog(Log.DEBUG)) {
-					_log.debug("Parsing (domain: \"" + domain
-							+ "\"; opcode: \"" + opcode + "\")");
-				}
-				props = SAMUtils.parseParams(tok);
 
 				if (domain.equals("STREAM")) {
 					canContinue = execStreamMessage(opcode, props);
@@ -909,13 +900,15 @@ class SAMv3Handler extends SAMv1Handler
 	/**
 	 * Handle a PING.
 	 * Send a PONG.
+	 *
+	 * @param msg to append, may be null
 	 * @since 0.9.24
 	 */
-	private void execPingMessage(StringTokenizer tok) {
+	private void execPingMessage(String msg) {
 		StringBuilder buf = new StringBuilder();
 		buf.append("PONG");
-		while (tok.hasMoreTokens()) {
+		if (msg != null) {
-			buf.append(' ').append(tok.nextToken());
+			buf.append(' ').append(msg);
 		}
 		buf.append('\n');
 		writeString(buf.toString());
@@ -923,13 +916,12 @@ class SAMv3Handler extends SAMv1Handler
 
 	/**
 	 * Handle a PONG.
+	 *
+	 * @param s received, may be null
 	 * @since 0.9.24
 	 */
-	private void execPongMessage(StringTokenizer tok) {
+	private void execPongMessage(String s) {
-		String s;
+		if (s == null) {
-		if (tok.hasMoreTokens()) {
-			s = tok.nextToken();
-		} else {
 			s = "";
 		}
 		if (_lastPing > 0) {