mjpclab / go-http-file-server

Simple command line based HTTP file server to share local file system
MIT License
370 stars 56 forks source link

.mp3 and .mp4 files getting deleted if user has video downloading browser extension #5

Closed laggykiller closed 3 years ago

laggykiller commented 3 years ago

Greetings.

Let's say I have some files in /home/user/test ls /home/user/test 1.mp3 2.mp4 3.txt 4.docx

I launch the server with sudo ghfs -r /home/user --global-delete

Then I visit /home/user/test from browser (localhost:80/test)

This cause certain files in /home/user/test getting deleted. ls /home/user/test 3.txt 4.docx From testing, files with extension .mp3 and .mp4 are affected. More extensions might be affected but this require more testing.

Thanks to this bug, I lost so many videos!

marjune163 commented 3 years ago

Hi, I'm sorry that you lost files. When deleting is enabled, each delete button is a URL link, which will delete file when visited. I guess there is some anti-virus software or security plugins that try to visit the delete link, which will cause send a delete request to the server. Please make sure not accessing that kind of link.

image

laggykiller commented 3 years ago

This is embarrassing... Done some testing just now, turns out the offending browser extension is called Video Downloader Plus

If I disable that extension, the 'bug' disappear. Now I won't blame ghfs devs for my lost files!

Here is my hypothesis:

The extension search for url for media files by finding url that end with .mp3, .mp4 etc, then automatically visit the url of media file to get media info (e.g. Size of the video file).

Since the url to delete file ends with mp3/mp4, the extension mistaken the url to delete file as a url to downloadable media file. Hence, when I load the page, the extension visit the url to delete file automatically. This cause the mp3/mp4 files in the page to get deleted by the extension.

I propose that the url to delete file be changed to ./?name=filetodelete.extension&delete such that similar browser extension is less likely to misrecognize the url as mp3/mp4 file and prevent such catastrophy (However I think those browser extension can still search for the extension name of mp3/mp4 even if it is not in the end of url). Or even better, find a way to not make the file name appear in link to delete file (e.g. Assign file ID that correspond to a file in directory? Javascript?) (Seems like an overkill though)

ropucyka commented 3 years ago

Filename with ext. can be replaced with blake3 fastest hash, or added confirmation window for delete command ...

laggykiller commented 3 years ago

added confirmation window for delete command

Imagine a user with similar video downloader browser extension, greeted with hundreds of 'Are you sure to delete XXX.mp4' alert windows when opening a directory full of media files...

blake3 hash sounds like a good idea though

laggykiller commented 3 years ago

added confirmation window for delete command

I have done some testing. There is an alert pop up for delete confirmation already. However if I directly visit the url to delete file, there is no such pop up (File get deleted directly, which I think that it should do so to help people who want to automate stuff using that webpage).

I propose that the url to delete file be changed to ./?name=filetodelete.extension&delete

I made a html page with hyperlink to http://localhost:8080/test/?name=2.mp4&delete, then hosted the html page on localhost:8080 with nginx. When I visit localhost:8080, the video downloader browser extension can still detect the url as media file.

However, if I change the hyperlink to http://localhost:8080/test/?name=2.mp4&action=delete, the video downloader borwser extension does not detect the url as media file. Hence, a change to the POST method might help prevent such problem. I am not sure if other browser extension would misrecognize it as url to media file though.

laggykiller commented 3 years ago

Update: I tried to modify the code such that the url for delete file is http://localhost:8080/test/?name=2.mp4&ask=true

However, the video downloader browser extension still mistaken it as media url and deleted the file.

marjune163 commented 3 years ago

I found a way maybe workaround, just use form submission:

<form class="delete" action="./" onsubmit="return confirmDelete(this)"><input type="hidden" name="delete"/><button type="submit" name="name" value="test.mp4">x</button></form>

But some further work need to do: e.g. extract filename into the data object.

laggykiller commented 3 years ago

@marjune163 Nice!

Here's an additional suggestion if you are planning to change to form submission: Add checkboxes next to each file items, then pop up buttons for operations (e.g. deletion, archive) when the user select item(s)? I think this can improve the UI (Looks sort of like email web client). Hope this won't be too hard to work on!

marjune163 commented 3 years ago

The commit above uses form submission to workaround. However it does not work on IE7 and bellow. I suppose it is not the final solution.

laggykiller commented 3 years ago

By the way, other functions such as download, archive, mkdir and upload are using similar method as delete. Would it be a good idea to change all of those to use form submission, as this can unify stuff and make future development easier...?

marjune163 commented 3 years ago

For download and archive, there is no extra side effect on server side. It is the right way that just using regular link. If this increase server's work load, it's plugin's duty to optimize its behavior by using HEAD method.

Already using form submission for mkdir and upload in current release.

marjune163 commented 3 years ago

The improvements is released as version 1.10.14.

laggykiller commented 3 years ago

Using 1.10.14, can confirm the 'bug' is fixed. Thanks dev!