GUACAMOLE-615: Add more thorough unit tests for protocol parsing.

guacamole-common-js/src/test/javascript/ParserSpec.js

@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ */
+
+/* global Guacamole, jasmine, expect */
+
+describe('Guacamole.Parser', function ParserSpec() {
+
+    /**
+     * A single Unicode high surrogate character (any character between U+D800
+     * and U+DB7F).
+     *
+     * @constant
+     * @type {!string}
+     */
+    const HIGH_SURROGATE = '\uD802';
+
+    /**
+     * A single Unicode low surrogate character (any character between U+DC00
+     * and U+DFFF).
+     *
+     * @constant
+     * @type {!string}
+     */
+    const LOW_SURROGATE = '\uDF00';
+
+    /**
+     * A Unicode surrogate pair, consisting of a high and low surrogate.
+     *
+     * @constant
+     * @type {!string}
+     */
+    const SURROGATE_PAIR = HIGH_SURROGATE + LOW_SURROGATE;
+
+    /**
+     * A 4-character test string containing Unicode characters that require
+     * multiple bytes when encoded as UTF-8, including at least one character
+     * that is encoded as a surrogate pair in UTF-16.
+     *
+     * @constant
+     * @type {!string}
+     */
+    const UTF8_MULTIBYTE = '\u72AC' + SURROGATE_PAIR + 'z\u00C1';
+
+    /**
+     * The Guacamole.Parser instance to test. This instance is (re)created prior
+     * to each test via beforeEach().
+     *
+     * @type {Guacamole.Parser}
+     */
+    var parser;
+
+    // Provide each test with a fresh parser
+    beforeEach(function() {
+        parser = new Guacamole.Parser();
+    });
+
+    // Empty instruction
+    describe('when an empty instruction is received', function() {
+
+        it('should parse the single empty opcode and invoke oninstruction', function() {
+            parser.oninstruction = jasmine.createSpy('oninstruction');
+            parser.receive('0.;');
+            expect(parser.oninstruction).toHaveBeenCalledOnceWith('', [ ]);
+        });
+
+    });
+
+    // Instruction using basic Latin characters
+    describe('when an instruction is containing only basic Latin characters', function() {
+
+        it('should correctly parse each element and invoke oninstruction', function() {
+            parser.oninstruction = jasmine.createSpy('oninstruction');
+            parser.receive('5.test2,'
+                + '10.hellohello,'
+                + '15.worldworldworld;'
+            );
+            expect(parser.oninstruction).toHaveBeenCalledOnceWith('test2', [
+                'hellohello',
+                'worldworldworld'
+            ]);
+        });
+
+    });
+
+    // Instruction using characters requiring multiple bytes in UTF-8 and
+    // surrogate pairs in UTF-16, including an element ending with a surrogate
+    // pair
+    describe('when an instruction is received containing elements that '
+           + 'contain characters involving surrogate pairs', function() {
+
+        it('should correctly parse each element and invoke oninstruction', function() {
+            parser.oninstruction = jasmine.createSpy('oninstruction');
+            parser.receive('4.test,'
+                + '6.a' + UTF8_MULTIBYTE + 'b,'
+                + '5.1234' + SURROGATE_PAIR + ','
+                + '10.a' + UTF8_MULTIBYTE + UTF8_MULTIBYTE + 'c;'
+            );
+            expect(parser.oninstruction).toHaveBeenCalledOnceWith('test', [
+                'a' + UTF8_MULTIBYTE + 'b',
+                '1234' + SURROGATE_PAIR,
+                'a' + UTF8_MULTIBYTE + UTF8_MULTIBYTE + 'c'
+            ]);
+        });
+
+    });
+
+    // Instruction with an element values ending with an incomplete surrogate
+    // pair (high or low surrogate only)
+    describe('when an instruction is received containing elements that end '
+           + 'with incomplete surrogate pairs', function() {
+
+        it('should correctly parse each element and invoke oninstruction', function() {
+            parser.oninstruction = jasmine.createSpy('oninstruction');
+            parser.receive('4.test,'
+                + '5.1234' + HIGH_SURROGATE + ','
+                + '5.4567' + LOW_SURROGATE + ';'
+            );
+            expect(parser.oninstruction).toHaveBeenCalledOnceWith('test', [
+                '1234' + HIGH_SURROGATE,
+                '4567' + LOW_SURROGATE
+            ]);
+        });
+
+    });
+
+    // Instruction with element values containing incomplete surrogate pairs,
+    describe('when an instruction is received containing incomplete surrogate pairs', function() {
+
+        it('should correctly parse each element and invoke oninstruction', function() {
+            parser.oninstruction = jasmine.createSpy('oninstruction');
+            parser.receive('5.te' + LOW_SURROGATE + 'st,'
+                + '5.12' + HIGH_SURROGATE + '3' + LOW_SURROGATE + ','
+                + '6.5' + LOW_SURROGATE + LOW_SURROGATE + '4' + HIGH_SURROGATE + HIGH_SURROGATE + ','
+                + '10.' + UTF8_MULTIBYTE + HIGH_SURROGATE + UTF8_MULTIBYTE + HIGH_SURROGATE + ';',
+            );
+            expect(parser.oninstruction).toHaveBeenCalledOnceWith('te' + LOW_SURROGATE + 'st', [
+                '12' + HIGH_SURROGATE + '3' + LOW_SURROGATE,
+                '5' + LOW_SURROGATE + LOW_SURROGATE + '4' + HIGH_SURROGATE + HIGH_SURROGATE,
+                UTF8_MULTIBYTE + HIGH_SURROGATE + UTF8_MULTIBYTE + HIGH_SURROGATE
+            ]);
+        });
+
+    });
+
+
+
+});

guacamole-common/src/test/java/org/apache/guacamole/protocol/GuacamoleInstructionTest.java

@@ -0,0 +1,205 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 org.apache.guacamole.protocol;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import static org.junit.Assert.*;
+import org.junit.Test;
+
+/**
+ * Unit test for GuacamoleParser. Verifies that parsing of the Guacamole
+ * protocol works as required.
+ */
+public class GuacamoleInstructionTest {
+
+    /**
+     * A single test case for verifying that Guacamole protocol implementations
+     * correctly parse or encode Guacamole instructions.
+     */
+    public static class TestCase extends GuacamoleInstruction {
+
+        /**
+         * The full and correct Guacamole protocol representation of this
+         * instruction.
+         */
+        public final String UNPARSED;
+
+        /**
+         * The opcode that should be present in the Guacamole instruction;
+         */
+        public final String OPCODE;
+
+        /**
+         * All arguments that should be present in the Guacamole instruction;
+         */
+        public final List<String> ARGS;
+
+        /**
+         * Creates a new TestCase representing the given Guacamole instruction.
+         *
+         * @param unparsed
+         *     The full and correct Guacamole protocol representation of this
+         *     instruction.
+         *
+         * @param opcode
+         *     The opcode of the Guacamole instruction.
+         *
+         * @param args
+         *     The arguments of the Guacamole instruction, if any.
+         */
+        public TestCase(String unparsed, String opcode, String... args) {
+            super(opcode, Arrays.copyOf(args, args.length));
+            this.UNPARSED = unparsed;
+            this.OPCODE = opcode;
+            this.ARGS = Collections.unmodifiableList(Arrays.asList(args));
+        }
+
+    }
+
+    /**
+     * A single Unicode high surrogate character (any character between U+D800
+     * and U+DB7F).
+     */
+    public static final String HIGH_SURROGATE = "\uD802";
+
+    /**
+     * A single Unicode low surrogate character (any character between U+DC00
+     * and U+DFFF).
+     */
+    public static final String LOW_SURROGATE = "\uDF00";
+
+    /**
+     * A Unicode surrogate pair, consisting of a high and low surrogate.
+     */
+    public static final String SURROGATE_PAIR = HIGH_SURROGATE + LOW_SURROGATE;
+
+    /**
+     * A 4-character test string containing Unicode characters that require
+     * multiple bytes when encoded as UTF-8, including at least one character
+     * that is encoded as a surrogate pair in UTF-16.
+     */
+    public static final String UTF8_MULTIBYTE = "\u72AC" + SURROGATE_PAIR + "z\u00C1";
+
+    /**
+     * Pre-defined set of test cases for verifying Guacamole instructions are
+     * correctly parsed and encoded.
+     */
+    public static List<TestCase> TEST_CASES = Collections.unmodifiableList(Arrays.asList(
+
+        // Empty instruction
+        new TestCase(
+            "0.;",
+            ""
+        ),
+
+        // Instruction using basic Latin characters
+        new TestCase(
+
+              "5.test2,"
+            + "10.hellohello,"
+            + "15.worldworldworld;",
+
+            "test2",
+            "hellohello",
+            "worldworldworld"
+
+        ),
+
+        // Instruction using characters requiring multiple bytes in UTF-8 and
+        // surrogate pairs in UTF-16, including an element ending with a surrogate
+        // pair
+        new TestCase(
+
+              "4.ab" + HIGH_SURROGATE + HIGH_SURROGATE + ","
+            + "6.a" + UTF8_MULTIBYTE + "b,"
+            + "5.12345,"
+            + "10.a" + UTF8_MULTIBYTE + UTF8_MULTIBYTE + "c;",
+
+            "ab" + HIGH_SURROGATE + HIGH_SURROGATE,
+            "a" + UTF8_MULTIBYTE + "b",
+            "12345",
+            "a" + UTF8_MULTIBYTE + UTF8_MULTIBYTE + "c"
+
+        ),
+
+        // Instruction with an element values ending with an incomplete surrogate
+        // pair (high or low surrogate only)
+        new TestCase(
+
+              "4.test,"
+            + "5.1234" + HIGH_SURROGATE + ","
+            + "5.4567" + LOW_SURROGATE + ";",
+
+            "test",
+            "1234" + HIGH_SURROGATE,
+            "4567" + LOW_SURROGATE
+
+        ),
+
+        // Instruction with element values containing incomplete surrogate pairs
+        new TestCase(
+
+              "5.te" + LOW_SURROGATE + "st,"
+            + "5.12" + HIGH_SURROGATE + "3" + LOW_SURROGATE + ","
+            + "6.5" + LOW_SURROGATE + LOW_SURROGATE + "4" + HIGH_SURROGATE + HIGH_SURROGATE + ","
+            + "10." + UTF8_MULTIBYTE + HIGH_SURROGATE + UTF8_MULTIBYTE + HIGH_SURROGATE + ";",
+
+            "te" + LOW_SURROGATE + "st",
+            "12" + HIGH_SURROGATE + "3" + LOW_SURROGATE,
+            "5" + LOW_SURROGATE + LOW_SURROGATE + "4" + HIGH_SURROGATE + HIGH_SURROGATE,
+            UTF8_MULTIBYTE + HIGH_SURROGATE + UTF8_MULTIBYTE + HIGH_SURROGATE
+
+        )
+
+    ));
+
+    /**
+     * Verifies that instruction opcodes are represented correctly.
+     */
+    @Test
+    public void testGetOpcode() {
+        for (TestCase testCase : TEST_CASES) {
+            assertEquals(testCase.OPCODE, testCase.getOpcode());
+        }
+    }
+
+    /**
+     * Verifies that instruction arguments are represented correctly.
+     */
+    @Test
+    public void testGetArgs() {
+        for (TestCase testCase : TEST_CASES) {
+            assertEquals(testCase.ARGS, testCase.getArgs());
+        }
+    }
+
+    /**
+     * Verifies that instructions are encoded correctly.
+     */
+    @Test
+    public void testToString() {
+        for (TestCase testCase : TEST_CASES) {
+            assertEquals(testCase.UNPARSED, testCase.toString());
+        }
+    }
+
+}

guacamole-common/src/test/java/org/apache/guacamole/protocol/GuacamoleParserTest.java

@@ -20,6 +20,8 @@
 package org.apache.guacamole.protocol;
 
 import org.apache.guacamole.GuacamoleException;
+import static org.apache.guacamole.protocol.GuacamoleInstructionTest.TEST_CASES;
+import org.apache.guacamole.protocol.GuacamoleInstructionTest.TestCase;
 import static org.junit.Assert.*;
 import org.junit.Test;
 
@@ -35,82 +37,46 @@ public class GuacamoleParserTest {
     private final GuacamoleParser parser = new GuacamoleParser();
 
     /**
-     * Test of append method, of class GuacamoleParser.
+     * Verify that GuacamoleParser correctly parses each of the instruction
+     * test cases included in the GuacamoleInstruction test.
      *
-     * @throws GuacamoleException If a parse error occurs while parsing the
+     * @throws GuacamoleException
-     *                            known-good test string.
+     *     If a parse error occurs.
      */
     @Test
     public void testParser() throws GuacamoleException {
 
-        // Test string
+        // Build buffer containing all of the instruction test cases, one after
-        char buffer[] = "1.a,2.bc,3.def,10.helloworld;4.test,5.test2;0.;3.foo;".toCharArray();
+        // the other
+        StringBuilder allTestCases = new StringBuilder();
+        for (TestCase testCase : TEST_CASES)
+            allTestCases.append(testCase.UNPARSED);
+
+        // Prepare buffer and offsets for feeding the data into the parser as
+        // if received over the network
+        char buffer[] = allTestCases.toString().toCharArray();
         int offset = 0;
         int length = buffer.length;
 
-        GuacamoleInstruction instruction;
+        // Verify that each of the expected instructions is received in order
+        for (TestCase testCase : TEST_CASES) {
+
+            // Feed data into parser until parser refuses to receive more data
             int parsed;
-
-        // Parse more data
             while (length > 0 && (parsed = parser.append(buffer, offset, length)) != 0) {
                 offset += parsed;
                 length -= parsed;
             }
 
-        // Validate first test instruction
+            // An instruction should now be parsed and ready for retrieval
             assertTrue(parser.hasNext());
-        instruction = parser.next();
+
+            // Verify instruction contains expected opcode and args
+            GuacamoleInstruction instruction = parser.next();
             assertNotNull(instruction);
-        assertEquals(3, instruction.getArgs().size());
+            assertEquals(testCase.OPCODE, instruction.getOpcode());
-        assertEquals("a", instruction.getOpcode());
+            assertEquals(testCase.ARGS, instruction.getArgs());
-        assertEquals("bc", instruction.getArgs().get(0));
-        assertEquals("def", instruction.getArgs().get(1));
-        assertEquals("helloworld", instruction.getArgs().get(2));
 
-        // Parse more data
-        while (length > 0 && (parsed = parser.append(buffer, offset, length)) != 0) {
-            offset += parsed;
-            length -= parsed;
-        }
-
-        // Validate second test instruction
-        assertTrue(parser.hasNext());
-        instruction = parser.next();
-        assertNotNull(instruction);
-        assertEquals(1, instruction.getArgs().size());
-        assertEquals("test", instruction.getOpcode());
-        assertEquals("test2", instruction.getArgs().get(0));
-
-        // Parse more data
-        while (length > 0 && (parsed = parser.append(buffer, offset, length)) != 0) {
-            offset += parsed;
-            length -= parsed;
-        }
-
-        // Validate third test instruction
-        assertTrue(parser.hasNext());
-        instruction = parser.next();
-        assertNotNull(instruction);
-        assertEquals(0, instruction.getArgs().size());
-        assertEquals("", instruction.getOpcode());
-
-        // Parse more data
-        while (length > 0 && (parsed = parser.append(buffer, offset, length)) != 0) {
-            offset += parsed;
-            length -= parsed;
-        }
-
-        // Validate fourth test instruction
-        assertTrue(parser.hasNext());
-        instruction = parser.next();
-        assertNotNull(instruction);
-        assertEquals(0, instruction.getArgs().size());
-        assertEquals("foo", instruction.getOpcode());
-
-        // Parse more data
-        while (length > 0 && (parsed = parser.append(buffer, offset, length)) != 0) {
-            offset += parsed;
-            length -= parsed;
         }
 
         // There should be no more instructions