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



##########
File path: src/protocols/rdp/rdp.c
##########
@@ -227,10 +236,70 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, 
char** username,
 
     rdpContext* context = instance->context;
     guac_client* client = ((rdp_freerdp_context*) context)->client;
+    guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+    guac_rdp_settings* settings = rdp_client->settings;
+    char* params[4] = {NULL};
+    int i = 0;
+    
+    /* If the client does not support the "required" instruction, warn and
+     * quit.
+     */
+    if (!guac_client_owner_supports_required(client)) {
+        guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the 
"
+                "\"required\" instruction. No authentication parameters will "
+                "be requested.");
+        return TRUE;
+    }
 
-    /* Warn if connection is likely to fail due to lack of credentials */
-    guac_client_log(client, GUAC_LOG_INFO,
-            "Authentication requested but username or password not given");
+    /* If the username is undefined, add it to the requested parameters. */
+    if (settings->username == NULL) {
+        guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, 
NULL, 0);
+        params[i] = GUAC_RDP_ARGV_USERNAME;
+        i++;
+        
+        /* If username is undefined and domain is also undefined, request 
domain. */
+        if (settings->domain == NULL) {
+            guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, 
NULL, 0);
+            params[i] = GUAC_RDP_ARGV_DOMAIN;
+            i++;
+        }
+        
+    }
+    
+    /* If the password is undefined, add it to the requested parameters. */
+    if (settings->password == NULL) {
+        guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, 
NULL, 0);
+        params[i] = GUAC_RDP_ARGV_PASSWORD;
+        i++;
+    }
+    
+    /* NULL-terminate the array. */
+    params[i] = NULL;
+    
+    if (i > 0) {
+        
+        /* Send required parameters to the owner. */
+        guac_client_owner_send_required(client, (const char**) params);
+        
+        guac_argv_await((const char**) params);
+        
+        /* Free old values and get new values from settings. */
+        if (*username != NULL)
+            free(*username);
+        
+        if (*password != NULL)
+            free(*password);
+        
+        if (*domain != NULL)
+            free(*domain);

Review comment:
       By the way, there's no need to check a pointer against `NULL` before 
invoking `free()`. If the pointer passed to `free()` is `NULL`, no operation is 
performed.
   




----------------------------------------------------------------
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:
[email protected]


Reply via email to