mike-jumper commented on code in PR #1066: URL: https://github.com/apache/guacamole-client/pull/1066#discussion_r1984191611
########## guacamole-common-js/src/main/webapp/modules/SessionRecording.js: ########## @@ -352,11 +381,12 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) */ var getElementSize = function getElementSize(value) { Review Comment: The `@returns` documentation for this function needs to be updated for the change in behavior (number of bytes instead of number of Unicode characters). ########## guacamole-common-js/src/main/webapp/modules/SessionRecording.js: ########## @@ -352,11 +381,12 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) */ var getElementSize = function getElementSize(value) { - var valueLength = value.length; - // Calculate base size, assuming at least one digit, the "." // separator, and the "," or ";" terminator - var protocolSize = valueLength + 3; + var protocolSize = getUtf8StringByteSize(value) + 3; + + // We need this to calculate the size of the length substring. + var valueLength = value.length; Review Comment: Won't this potentially not be correct for elements containing Unicode characters that involve surrogate pairs in their JavaScript string representations? If so, I think you need to use `Guacamole.Parser.codePointCount()` instead. ########## guacamole-common-js/src/main/webapp/modules/SessionRecording.js: ########## @@ -334,9 +334,38 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) }; + /** + * Calculates the size in bytes of the given UTF8 string. + * + * @private + * @param {!string} str + * The string to calculate the size in bytes. + * + * @returns {!number} + * The size in bytes of the given string. + */ + var getUtf8StringByteSize = function(str) { + + var byteSize = str.length; + for (var i = str.length - 1; i >= 0; i--) { + var code = str.charCodeAt(i); + if (code > 0x7F && code <= 0x7FF) + byteSize++; + else if (code > 0x7FF && code <= 0xFFFF) + byteSize += 2; + if (code >= 0xDC00 && code <= 0xDFFF) + i--; // trail surrogate Review Comment: It is unclear what "trail surrogate" means here. ########## guacamole-common-js/src/main/webapp/modules/SessionRecording.js: ########## @@ -352,11 +381,12 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) */ var getElementSize = function getElementSize(value) { - var valueLength = value.length; - // Calculate base size, assuming at least one digit, the "." // separator, and the "," or ";" terminator - var protocolSize = valueLength + 3; + var protocolSize = getUtf8StringByteSize(value) + 3; Review Comment: The original name `protocolSize` refers to the size of the element as represented by the protocol (in characters). Since this value is in bytes here, the variable should be renamed accordingly. ########## guacamole-common-js/src/main/webapp/modules/SessionRecording.js: ########## @@ -334,9 +334,38 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) }; + /** + * Calculates the size in bytes of the given UTF8 string. + * + * @private + * @param {!string} str + * The string to calculate the size in bytes. + * + * @returns {!number} + * The size in bytes of the given string. + */ + var getUtf8StringByteSize = function(str) { + + var byteSize = str.length; + for (var i = str.length - 1; i >= 0; i--) { + var code = str.charCodeAt(i); + if (code > 0x7F && code <= 0x7FF) + byteSize++; + else if (code > 0x7FF && code <= 0xFFFF) + byteSize += 2; + if (code >= 0xDC00 && code <= 0xDFFF) Review Comment: There are a good few magic numbers here. Comments describing the high-level intent of each section are needed to de-magic the logic. -- 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: dev-unsubscr...@guacamole.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org