jmuehlner commented on code in PR #853:
URL: https://github.com/apache/guacamole-client/pull/853#discussion_r1186347839
##########
guacamole-common-js/src/main/webapp/modules/Parser.js:
##########
@@ -63,80 +92,135 @@ Guacamole.Parser = function() {
*
* @param {!string} packet
* The instruction data to receive.
+ *
+ * @param {!boolean} [isBuffer=false]
+ * Whether the provided data should be treated as an instruction buffer
+ * that grows continuously. If true, the data provided to receive()
+ * MUST always start with the data provided to the previous call. If
+ * false (the default), only the new data should be provided to
+ * receive(), and previously-received data will automatically be
+ * buffered by the parser as needed.
*/
- this.receive = function(packet) {
+ this.receive = function receive(packet, isBuffer) {
- // Truncate buffer as necessary
- if (start_index > 4096 && element_end >= start_index) {
+ if (isBuffer)
+ buffer = packet;
- buffer = buffer.substring(start_index);
+ else {
- // Reset parse relative to truncation
- element_end -= start_index;
- start_index = 0;
+ // Truncate buffer as necessary
+ if (startIndex > 4096 && elementEnd >= startIndex) {
- }
+ buffer = buffer.substring(startIndex);
+
+ // Reset parse relative to truncation
+ elementEnd -= startIndex;
+ startIndex = 0;
- // Append data to buffer
- buffer += packet;
+ }
+
+ // Append data to buffer ONLY if there is outstanding data
present. It
+ // is otherwise much faster to simply parse the received buffer
as-is,
+ // and tunnel implementations can take advantage of this by
preferring
+ // to send only complete instructions. Both the HTTP and WebSocket
+ // tunnel implementations included with Guacamole already do this.
+ if (buffer.length)
+ buffer += packet;
+ else
+ buffer = packet;
+
+ }
// While search is within currently received data
- while (element_end < buffer.length) {
+ while (elementEnd < buffer.length) {
// If we are waiting for element data
- if (element_end >= start_index) {
+ if (elementEnd >= startIndex) {
+
+ // If we have enough data in the buffer to fill the element
+ // value, but the number of codepoints in the expected
substring
+ // containing the element value value is less that its declared
+ // length, that can only be because the element contains
+ // characters split between high and low surrogates, and the
+ // actual end of the element value is further out. The minimum
+ // number of additional characters that must be read to satisfy
+ // the declared length is simply the difference between the
+ // number of codepoints actually present vs. the expected
+ // length.
+ var codepoints = Guacamole.Parser.codePointCount(buffer,
startIndex, elementEnd);
+ if (codepoints < elementCodepoints) {
+ elementEnd += elementCodepoints - codepoints;
+ continue;
+ }
+
+ // If the current element ends with a character involving both
+ // a high and low surrogate, elementEnd points to the low
+ // surrogate and NOT the element terminator. We must shift the
+ // end and reevaluate.
+ else if (elementCodepoints && buffer.codePointAt(elementEnd -
1) >= 0x10000) {
+ elementEnd++;
+ continue;
+ }
// We now have enough data for the element. Parse.
- var element = buffer.substring(start_index, element_end);
- var terminator = buffer.substring(element_end, element_end+1);
+ var element = buffer.substring(startIndex, elementEnd);
+ var terminator = buffer.substring(elementEnd, elementEnd+1);
// Add element to array
- element_buffer.push(element);
+ elementBuffer.push(element);
// If last element, handle instruction
- if (terminator == ";") {
+ if (terminator === ';') {
// Get opcode
- var opcode = element_buffer.shift();
+ var opcode = elementBuffer.shift();
// Call instruction handler.
- if (parser.oninstruction != null)
- parser.oninstruction(opcode, element_buffer);
+ if (parser.oninstruction !== null)
+ parser.oninstruction(opcode, elementBuffer);
// Clear elements
- element_buffer.length = 0;
+ elementBuffer = [];
+
+ // Immediately truncate buffer if its contents have been
+ // completely parsed, so that the next call to receive()
+ // need not append to the buffer unnecessarily
+ if (elementEnd + 1 === buffer.length) {
+ elementEnd = -1;
+ buffer = '';
+ }
}
- else if (terminator != ',')
- throw new Error("Illegal terminator.");
+ else if (terminator !== ',')
+ throw new Error('Element terminator of instruction was not
";" nor ",".');
// Start searching for length at character after
// element terminator
- start_index = element_end + 1;
+ startIndex = elementEnd + 1;
}
// Search for end of length
- var length_end = buffer.indexOf(".", start_index);
- if (length_end != -1) {
+ var lengthEnd = buffer.indexOf('.', startIndex);
+ if (lengthEnd !== -1) {
// Parse length
- var length = parseInt(buffer.substring(element_end+1,
length_end));
- if (isNaN(length))
- throw new Error("Non-numeric character in element
length.");
+ elementCodepoints = parseInt(buffer.substring(elementEnd+1,
lengthEnd));
Review Comment:
To match existing code style, there should probably be a space between the
`+` operator and its operands here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
