necouchman commented on a change in pull request #367:
URL: https://github.com/apache/guacamole-server/pull/367#discussion_r804605022



##########
File path: src/common-ssh/sftp.c
##########
@@ -653,10 +654,19 @@ static int guac_common_ssh_sftp_ls_ack_handler(guac_user* 
user,
 
         /* Determine mimetype */
         const char* mimetype;
-        if (LIBSSH2_SFTP_S_ISDIR(attributes.permissions))
-            mimetype = GUAC_USER_STREAM_INDEX_MIMETYPE;
-        else
-            mimetype = "application/octet-stream";
+        //adding file size and permission
+        char tmpstr[150];
+       if (LIBSSH2_SFTP_S_ISDIR(attributes.permissions)) {
+               sprintf(tmpstr, "{\"mime\":\"%s\",\"size\":%llu,\"perm\":%lu}",
+                       GUAC_USER_STREAM_INDEX_MIMETYPE, attributes.filesize,
+                       attributes.permissions);
+       }
+       else {
+               sprintf(tmpstr, "{\"mime\":\"%s\",\"size\":%llu,\"perm\":%lu}",
+                       "application/octet-stream", attributes.filesize,
+                       attributes.permissions);
+       }
+       mimetype = tmpstr;

Review comment:
       I don't think we should just re-assign this string to `mimetype`, as the 
name `mimetype` implies that the data carried is the MIME type, not a string of 
information with various attributes about the file.

##########
File path: src/common-ssh/sftp.c
##########
@@ -653,10 +654,19 @@ static int guac_common_ssh_sftp_ls_ack_handler(guac_user* 
user,
 
         /* Determine mimetype */
         const char* mimetype;
-        if (LIBSSH2_SFTP_S_ISDIR(attributes.permissions))
-            mimetype = GUAC_USER_STREAM_INDEX_MIMETYPE;
-        else
-            mimetype = "application/octet-stream";
+        //adding file size and permission
+        char tmpstr[150];
+       if (LIBSSH2_SFTP_S_ISDIR(attributes.permissions)) {
+               sprintf(tmpstr, "{\"mime\":\"%s\",\"size\":%llu,\"perm\":%lu}",

Review comment:
       I'm not sure how JSON data is assembled elsewhere in the code - if 
`sprintf` is routinely used like this, or if there's a library that handles it? 
Might be worth checking that out - I'd be a little surprised if all of the 
other places in the guacd code that assemble JSON do it like this.

##########
File path: src/protocols/rdp/ls.c
##########
@@ -85,10 +86,17 @@ int guac_rdp_ls_ack_handler(guac_user* user, guac_stream* 
stream,
 
         /* Determine mimetype */
         const char* mimetype;
-        if (file->attributes & FILE_ATTRIBUTE_DIRECTORY)
-            mimetype = GUAC_USER_STREAM_INDEX_MIMETYPE;
-        else
-            mimetype = "application/octet-stream";
+        char tmpstr[150];
+        //adding file size
+        if (file->attributes & FILE_ATTRIBUTE_DIRECTORY) {
+            sprintf(tmpstr, "{\"mime\":\"%s\",\"size\":%lu}",
+                GUAC_USER_STREAM_INDEX_MIMETYPE, file->size);
+        }
+        else {
+            sprintf(tmpstr, "{\"mime\":\"%s\",\"size\":%lu}",
+                "application/octet-stream", file->size);
+        }
+        mimetype = tmpstr;

Review comment:
       As with above - just assigning this `tmpstr` back to `mimetype` doesn't 
really make sense.




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