You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2022/02/11 10:46:13 UTC
[GitHub] [guacamole-server] bookread-he opened a new pull request #367: GUACAMOLE-1536: return file size and file permission information when uploading files and displaying file directories
bookread-he opened a new pull request #367:
URL: https://github.com/apache/guacamole-server/pull/367
return file size and file permission information when uploading files and displaying file directories
--
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: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] necouchman commented on a change in pull request #367: GUACAMOLE-1536: return file size and file permission information when uploading files and displaying file directories
Posted by GitBox <gi...@apache.org>.
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: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org