[GitHub] [guacamole-server] holograph commented on a change in pull request #271: GUACAMOLE-1047: Notify connecting client on unrecognized connection ID

[GitBox <gi...@apache.org>]

holograph commented on a change in pull request #271:
URL: https://github.com/apache/guacamole-server/pull/271#discussion_r575814820



##########
File path: src/guacd/connection.c
##########
@@ -278,10 +278,18 @@ static int guacd_route_connection(guacd_proc_map* map, guac_socket* socket) {
         proc = guacd_proc_map_retrieve(map, identifier);
         new_process = 0;
 
-        /* Warn if requested connection does not exist */
-        if (proc == NULL)
-            guacd_log(GUAC_LOG_INFO, "Connection \"%s\" does not exist.",
-                    identifier);
+        /* Warn and ward off client if requested connection does not exist */
+        if (proc == NULL) {
+            char message[2048];
+
+            snprintf(message, sizeof(message),
+                    "Connection \"%s\" does not exist", identifier);
+
+            guacd_log(GUAC_LOG_INFO, message);

Review comment:
       Oh, I missed that -- so I can basically safely log a format string and skip constructing the message entirely? i.e.
   ```guacd_log(GUAC_LOG_INFO, "Connection \"%s\" does not exist", identifier);```
   
   As for the level, this isn't a runtime error so much as a client error though, akin to HTTP 400 "bad request" or 404 "not found", which to me sounds more like an informative message if any; If you still think it's appropriate to log this an error, however, please ping me and I'll adjust the PR.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org