necouchman commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1595594515
##########
configure.ac:
##########
@@ -891,17 +909,162 @@ fi
# Updated certificate verification callback (introduced with 2.0.0, not present
# in 2.0.0-rc4 or earlier)
-if test "x${have_freerdp2}" = "xyes"
+if test "x${have_freerdp}" = "xyes"
then
AC_CHECK_MEMBERS([freerdp.VerifyCertificateEx],,,
- [[#include <freerdp/freerdp.h>]])
+ [[#include <freerdp/freerdp.h>]])
Review Comment:
There seem to be some alignment issues throughout the `configure.ac` file,
now. If we're sticking to how things are generally done throughout the file,
the start of this should be aligned with the `[freerdp.VerifyCertificateEx]`
line above, but it seems to be less indented, now.. There are several of these
occurrences throughout the changes in the file.
##########
src/libguac/string.c:
##########
@@ -124,18 +124,30 @@ char* guac_strdup(const char* str) {
/* Do not attempt to duplicate if the length is somehow magically so
* obscenely large that it will not be possible to add a null terminator */
size_t length;
- if (guac_mem_ckd_add(&length, strlen(str), 1))
+ size_t length_to_copy = strnlen(str, n);
+ if (guac_mem_ckd_add(&length, length_to_copy, 1))
return NULL;
- /* Otherwise just copy to a new string in same manner as strdup() */
- void* new_str = guac_mem_alloc(length);
- if (new_str != NULL)
- memcpy(new_str, str, length);
+ /* Otherwise just copy to a new string in same manner as strndup() */
+ char* new_str = (char*)guac_mem_alloc(length);
+ if (new_str != NULL) {
+ memcpy(new_str, str, length_to_copy);
+ new_str[length_to_copy] = '\0';
Review Comment:
This appears to change the behavior of the `guac_strdup()` function, which,
previously, did not add a null terminator to the end of the string. I don't
know if the previous behavior was intended or not, but it seems like this could
cause some issues throughout other parts of the code that depend on the lack of
a null terminator, either intentionally or not?
Also, the documentation for the function states that the provided string
should already be null-terminated - maybe this should be added only in the case
where the new string is shorter than the original one?
--
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]
