From a672229dada96f8036aa6f2443f53b718528cc72 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 25 Feb 2025 12:06:25 -0800 Subject: [PATCH] GUACAMOLE-2036: Reuse buffers received by parser when converting instructions back to character arrays. --- .../guacamole/io/ReaderGuacamoleReader.java | 3 +- .../protocol/GuacamoleInstruction.java | 145 ++++++++++++++---- .../guacamole/protocol/GuacamoleParser.java | 79 +++++++++- 3 files changed, 191 insertions(+), 36 deletions(-) diff --git a/guacamole-common/src/main/java/org/apache/guacamole/io/ReaderGuacamoleReader.java b/guacamole-common/src/main/java/org/apache/guacamole/io/ReaderGuacamoleReader.java index 357612947..207b1cc16 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/io/ReaderGuacamoleReader.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/io/ReaderGuacamoleReader.java @@ -24,7 +24,6 @@ import java.io.IOException; import java.io.Reader; import java.net.SocketException; import java.net.SocketTimeoutException; -import java.util.Arrays; import org.apache.guacamole.GuacamoleConnectionClosedException; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleServerException; @@ -92,7 +91,7 @@ public class ReaderGuacamoleReader implements GuacamoleReader { if (instruction == null) return null; - return instruction.toString().toCharArray(); + return instruction.toCharArray(); } @Override diff --git a/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleInstruction.java b/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleInstruction.java index 10078d4a0..fcce42b2a 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleInstruction.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleInstruction.java @@ -41,18 +41,31 @@ public class GuacamoleInstruction { private final List args; /** - * The cached result of converting this GuacamoleInstruction to the format - * used by the Guacamole protocol. + * The cached String result of converting this GuacamoleInstruction to the + * format used by the Guacamole protocol. + * + * @see #toString() */ - private String protocolForm = null; + private String rawString = null; /** - * Creates a new GuacamoleInstruction having the given Operation and - * list of arguments values. + * The cached char[] result of converting this GuacamoleInstruction to the + * format used by the Guacamole protocol. * - * @param opcode The opcode of the instruction to create. - * @param args The list of argument values to provide in the new - * instruction if any. + * @see #toCharArray() + */ + private char[] rawChars = null; + + /** + * Creates a new GuacamoleInstruction having the given opcode and list of + * argument values. + * + * @param opcode + * The opcode of the instruction to create. + * + * @param args + * The list of argument values to provide in the new instruction, if + * any. */ public GuacamoleInstruction(String opcode, String... args) { this.opcode = opcode; @@ -60,22 +73,59 @@ public class GuacamoleInstruction { } /** - * Creates a new GuacamoleInstruction having the given Operation and - * list of arguments values. The list given will be used to back the - * internal list of arguments and the list returned by getArgs(). + * Creates a new GuacamoleInstruction having the given opcode and list of + * argument values. The list given will be used to back the internal list of + * arguments and the list returned by {@link #getArgs()}. + *

+ * The provided argument list may not be modified in any way after being + * provided to this constructor. Doing otherwise will result in undefined + * behavior. * - * @param opcode The opcode of the instruction to create. - * @param args The list of argument values to provide in the new - * instruction if any. + * @param opcode + * The opcode of the instruction to create. + * + * @param args + * The list of argument values to provide in the new instruction, if + * any. */ public GuacamoleInstruction(String opcode, List args) { this.opcode = opcode; this.args = Collections.unmodifiableList(args); } + /** + * Creates a new GuacamoleInstruction having the given opcode, list of + * argument values, and underlying protocol representation. The list given + * will be used to back the internal list of arguments and the list + * returned by {@link #getArgs()}. The provided protocol representation + * will be used to back the internal protocol representation and values + * returned by {@link #toCharArray()} and {@link #toString()}. + *

+ * Neither the provided argument list nor the provided protocol + * representation may be modified in any way after being provided to this + * constructor. Doing otherwise will result in undefined behavior. + * + * @param opcode + * The opcode of the instruction to create. + * + * @param args + * The list of argument values to provide in the new instruction, if + * any. + * + * @param raw + * The underlying representation of this instruction as would be sent + * over the network via the Guacamole protocol. + */ + public GuacamoleInstruction(String opcode, List args, char[] raw) { + this(opcode, args); + this.rawChars = raw; + } + /** * Returns the opcode associated with this GuacamoleInstruction. - * @return The opcode associated with this GuacamoleInstruction. + * + * @return + * The opcode associated with this GuacamoleInstruction. */ public String getOpcode() { return opcode; @@ -86,8 +136,9 @@ public class GuacamoleInstruction { * GuacamoleInstruction. Note that the List returned is immutable. * Attempts to modify the list will result in exceptions. * - * @return A List of all argument values specified for this - * GuacamoleInstruction. + * @return + * An unmodifiable List of all argument values specified for this + * GuacamoleInstruction. */ public List getArgs() { return args; @@ -122,28 +173,58 @@ public class GuacamoleInstruction { // Avoid rebuilding Guacamole protocol form of instruction if already // known - if (protocolForm == null) { + if (rawString == null) { - StringBuilder buff = new StringBuilder(); + // Prefer to construct String from existing char array, rather than + // reconstruct protocol from scratch + if (rawChars != null) + rawString = new String(rawChars); - // Write opcode - appendElement(buff, opcode); + // Reconstruct protocol details only if truly necessary + else { + + StringBuilder buff = new StringBuilder(); + + // Write opcode + appendElement(buff, opcode); + + // Write argument values + for (String value : args) { + buff.append(','); + appendElement(buff, value); + } + + // Write terminator + buff.append(';'); + + // Cache result for future calls + rawString = buff.toString(); - // Write argument values - for (String value : args) { - buff.append(','); - appendElement(buff, value); } - // Write terminator - buff.append(';'); - - // Cache result for future calls - protocolForm = buff.toString(); - } - return protocolForm; + return rawString; + + } + + /** + * Returns this GuacamoleInstruction in the form it would be sent over the + * Guacamole protocol. The returned char[] MUST NOT be modified. If the + * returned char[] is modified, the results of doing so are undefined. + * + * @return + * This GuacamoleInstruction in the form it would be sent over the + * Guacamole protocol. The returned char[] MUST NOT be modified. + */ + public char[] toCharArray() { + + // Avoid rebuilding Guacamole protocol form of instruction if already + // known + if (rawChars == null) + rawChars = toString().toCharArray(); + + return rawChars; } diff --git a/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleParser.java b/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleParser.java index c51f164c4..378b471cc 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleParser.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleParser.java @@ -108,6 +108,18 @@ public class GuacamoleParser implements Iterator { */ private final String elements[] = new String[INSTRUCTION_MAX_ELEMENTS]; + /** + * A copy of the raw protocol data that has been parsed for the current + * instruction. This value is maintained by {@link #append(char[], int, int)}. + */ + private final char rawInstruction[] = new char[INSTRUCTION_MAX_LENGTH]; + + /** + * The offset within {@link #rawInstruction} that new data should be + * appended. This value is maintained by {@link #append(char[], int, int)}. + */ + private int rawInstructionOffset = 0; + /** * Appends data from the given buffer to the current instruction. * @@ -130,6 +142,71 @@ public class GuacamoleParser implements Iterator { */ public int append(char chunk[], int offset, int length) throws GuacamoleException { + int originalOffset = offset; + int originalLength = length; + + // Process as much of the received chunk as possible + while (length > 0) { + + int appended = processElement(chunk, offset, length); + if (appended == 0) + break; + + length -= appended; + offset += appended; + } + + // Update the raw copy of the received instruction with whatever data + // has now been processed + int charsParsed = originalLength - length; + if (charsParsed > 0) { + + System.arraycopy(chunk, originalOffset, rawInstruction, rawInstructionOffset, charsParsed); + rawInstructionOffset += charsParsed; + + // If the instruction is now complete, we're good to store the + // parsed instruction for future retrieval via next() + if (state == State.COMPLETE) { + parsedInstruction = new GuacamoleInstruction(elements[0], Arrays.asList(elements).subList(1, elementCount), + Arrays.copyOf(rawInstruction, rawInstructionOffset)); + rawInstructionOffset = 0; + } + + } + + return charsParsed; + + } + + /** + * Processes additional data from the given buffer, potentially adding + * another element to the current instruction being parsed. This function + * will need to be invoked multiple times per instruction until all data + * for that instruction is ready. + *

+ * This function DOES NOT update {@link #parsedInstruction}. The caller + * ({@link #append(char[], int, int)}) must update this as necessary when + * the parser {@link #state} indicates the instruction is complete. + * + * @param chunk + * The buffer containing the data to append. + * + * @param offset + * The offset within the buffer where the data begins. + * + * @param length + * The length of the data to append. + * + * @return + * The number of characters appended, or 0 if complete instructions + * have already been parsed and must be read via next() before more + * data can be appended. + * + * @throws GuacamoleException + * If an error occurs while parsing the new data. + */ + private int processElement(char chunk[], int offset, int length) throws GuacamoleException { + int charsParsed = 0; // Do not exceed maximum number of elements @@ -215,8 +292,6 @@ public class GuacamoleParser implements Iterator { // If semicolon, store end-of-instruction case ';': state = State.COMPLETE; - parsedInstruction = new GuacamoleInstruction(elements[0], - Arrays.asList(elements).subList(1, elementCount)); break; // If comma, move on to next element