[GitHub] [guacamole-client] necouchman commented on a diff in pull request #838: GUACAMOLE-926: Fix attribute handling bug and improve error handling for invalid files.

["necouchman (via GitHub)" <gi...@apache.org>]

necouchman commented on code in PR #838:
URL: https://github.com/apache/guacamole-client/pull/838#discussion_r1167570462


##########
guacamole/src/main/frontend/src/app/import/controllers/importConnectionsController.js:
##########
@@ -640,8 +655,23 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$
 
             else {
 
+                const fileData = e.target.result;
+
+                // Check if the file has a header of a known-bad type
+                if (_.some(ZIP_SIGNATURES,
+                        signature => fileData.startsWith(signature))) {
+
+                    // Throw an error and abort processing
+                    handleError(new ParseError({
+                        message: "Invalid file type detected",
+                        key: 'IMPORT.ERROR_DETECTED_INVALID_TYPE'
+                    }));
+                    return;

Review Comment:
   I guess I'm a little conflicted about this change. On the one hand, I understand that the likelihood of someone trying to upload, for example, an XLSX or ODS file in place of a CSV may be higher than someone trying to upload, say, a MP4 video file.
   
   On the other hand, what is the behavior if someone tries to drop a MP4 or tar file, and why is it important to handle errors with common spreadsheets differently from errors with completely unrelated files?



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