fix: Issue #631 raise an error when trying to extract a file (.zip_)#636
Conversation
…e) a file that does not have a valid extension
✅ Deploy Preview for superfile canceled.
|
|
sort of related but is it a good idea to remove the ".zip" case since extractCompressFile() can handle".zip" files as well? superfile/src/internal/handle_file_operations.go Lines 534 to 548 in 0cd46c4 Reduces the need to maintain the in-house unzip() function. |
|
@WarrenU See - https://superfile.netlify.app/configure/config-file-path/ |
|
|
||
| // isValidFileExtension checks if a string is a valid file extension. | ||
| // Returns nil if valid, otherwise returns an error with a descriptive message. | ||
| func isValidFileExtension(ext string) error { |
There was a problem hiding this comment.
This function should return a bool, not an error.
error denotes failures in execution. If you give an invalid extension to this function, it successfully detects that its invalid, and return false. There is no error.
| var err error | ||
| panel := &m.fileModel.filePanels[m.filePanelFocusIndex] | ||
| ext := strings.ToLower(filepath.Ext(panel.element[panel.cursor].location)) | ||
| err = isValidFileExtension(ext) |
There was a problem hiding this comment.
We dont wanna check if the extension is valid, but if it can be extracted or not. I dont think .jpg can be extracted. We should replace this with isExensionExtractable and just check for a few supported extenstions .zip, .tar.gz, .rar whichever we support.
There was a problem hiding this comment.
Updated to isExensionExtractable that makes sense and I agree.
@jachewz If it can handle, then we should remove it. Then we dont need the |
I removed the unzip, and opted to use xtractr default. I also added an explicit slog call for any error it raises. Good mention @jachewz |
|
I would like us a to do a bit of testing that it can handle .tar.gz , .rar and most of these extensions. I will try to test it out sometimes. It would be great if @WarrenU you could also perform some of the tests. |
|
.rar, .tar.gz, .zip are working. ✅ |
…uring decompression.
|
I just discovered during testing that if there is a directory in the decompression, permission denied will occur, so I added: x := &xtractr.XFile{
FilePath: src,
OutputDir: dest,
+ FileMode: 0644,
+ DirMode: 0755,
}Now it seems that it can be merge. Btw The reason why I separated the zip is actually because I want to allow users to see the real-time progress.But it is fine to remove it. |
Addresses issue #631 Unzipping a malformed/invalid zip file by raising an error if the extension is wrong.
isExensionExtractablefunction to check against file types we can extract from thextractrpackage. (return bool if we can extract or not)xtractrcan handle unzipping zip file.time=2025-02-21T10:53:00.459-08:00 level=ERROR msg="Error unexpected file extension type: .zip_" error="unsupported operation"Trial screenshot below (No progress in bottom left as also mentioned in issue):
❓ This is my first PR and I'm a brand new beginner to superfile.
Where would I see these logged errors? I ran it with VScode and put some breakpoints in to ensure the code I was hitting was working as expected. And it made some
__debug_bin3073481771.exefiles. I'm running it on windows. But not sure how to introspect that file or where to see the errors.