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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
