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



##########
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);`
   
   Yep!
   
   > 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; ...
   
   Good point. I agree it makes sense for this to remain at the info level, 
with any further handling of the client error naturally up to the client.




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


Reply via email to