Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Delete feature on the Search History form can delete all files and folders on the entire disk (in certain scenarios...) #9

Closed
grkncl opened this issue Mar 19, 2021 · 14 comments
Assignees
Labels

Comments

@grkncl
Copy link

grkncl commented Mar 19, 2021

Here is the initial comment @grkncl left to report this issue, then he seems to have deleted the text later, so I have added it back to have it on record:

You fool! All folders and files inside entire disk have been deleted because of your stupid app. Check the commands you made to clear "history" with the "delete" command.

@mattslay

@mattslay
Copy link
Owner

mattslay commented Mar 19, 2021

Well @grkncl , please help me understand exactly what you are referring to. Are you referring to the "Delete" button on the bottom of the Search History form?

If so, here is the code, so let's see what could have gone wrong...

If the problem is indeed with the GoFish code, then we need to figure it out... I see a reference to gf_SearchHistory.SearchHistoryFolder which should be a folder path that gets passed to the RemoveFolder() function.

Can you tell me exactly what your search history folder and history set was??

Delete Button Click() method:

lcPath = Alltrim(gf_SearchHistory.SearchHistoryFolder)

If RemoveFolder(m.lcPath)
	Delete In gf_SearchHistory
Else
	Messagebox('Unable to delete folder "' + m.lcPath + '"', 16, 'Unable to delete folder')
Endif

Here is the code for the RemoveFolder() function that is called above. Maybe we need to add some testing logic to the passed path name so this error cannot happen. Perhaps if an empty string is passed that could lead to a problem, and that scenario is not tested.

Procedure RemoveFolder(lcFolderName)

Procedure RemoveFolder(lcFolderName)
	Local laFiles[1], lcFileName, lcFileNameWithPath, llFailure, lnFileCount, lnI, loException

	Declare Integer SetFileAttributes In kernel32 String, Integer
	lcFolderName = Trim(m.lcFolderName)

	Try
		lnFileCount = Adir(laFiles, m.lcFolderName + '\*', 'DH')
		For lnI = 1 To m.lnFileCount
			lcFileName = m.laFiles[m.lnI, 1]
			If Left(m.lcFileName, 1) # '.'
				lcFileNameWithPath = m.lcFolderName + '\' + m.lcFileName
				SetFileAttributes(m.lcFileNameWithPath, 0)
				If 'D' $ m.laFiles[m.lnI, 5] && directory?
					RemoveFolder(m.lcFileNameWithPath)
				Else
					Delete File(m.lcFileNameWithPath)
				Endif
			Endif
		Endfor
		Rmdir(m.lcFolderName)

	Catch To m.loException
		llFailure = .T.
	Endtry

	Return m.llFailure = .F.

EndProc

image

@bturski
Copy link

bturski commented Mar 19, 2021

@mattslay - thank you for volunteering your time to investigate this issue and maintain this extremely useful, free, open-source tool.

@mattslay
Copy link
Owner

@grkncl Sir, you have indeed found an unfortunate bug. I will fix it, and I'm glad it has been discovered, sadly at your expense and loss. I truly regret that.

Notwithstanding, you need to cease your insulting of me. This is a very well-written and useful app that has served many VFP developers for over a decade. You have found one issue, albeit admittedly a painful one.

Your anger has brought you to foolishness, which is a worse position than you were in when you had only lost files on your disk.

@mattslay mattslay added the bug label Mar 19, 2021
@mattslay mattslay self-assigned this Mar 19, 2021
@DougHennig
Copy link

@grkncl Matt gave his time freely to create an extremely useful FREE utility that I use all the time. It's unfortunate that it had a bug that caused you problems and I am sure Matt regrets that. However, attacking him personally is unprofessional and has no place in a forum like this.

@mattslay
Copy link
Owner

mattslay commented Mar 19, 2021

@grkncl Can I ask you some questions to help me understand something?

  1. Did you indeed have some records in the Search History grid when the form launched?

  2. If so, how did the form get into a state where no record was selected in the grid? I have tried to accomplish this on my machine, and I cannot get the grid to have no row selected.

Of course I understand what guard code needs to be added to protect against a mass-delete situation now that you have reported it, but I am still curious about # 1 and # 2 above in your specific case.

@grkncl
Copy link
Author

grkncl commented Mar 19, 2021

@mattslay I tried the application for the first time. I made a few test calls. then I started deleting them. I guess I hit the "delete" button again without realizing it was the last record. the application was locked for a few minutes. After I terminated the entire hard disk was wiped.

@mattslay
Copy link
Owner

@grkncl Thanks for this info. I can see now how this could lead to the UI grid being empty. I will test against this scenario as well, and disable the the Delete button if there are no more records in the grid. And of course, I will add the important guard code in the RemoveFolder() so that it will simply Return out if the passed parameter is empty.

I will be posting an update on Tues 2021-02-23 or Wednesday 2021-03-24 through the Thor distribution channel, and I will be pushing the updated code to GitHub with full changelog notes.

@grkncl - I am truly sorry this unfortunate thing happened to you. Indeed when writing that code, I never imagined that the UI would allow the user to pass an empty string to the function, nor what the horrible results would be if they did. I cannot apologize enough for this bug.

Regardless of this unfortunate oversight on my part, I encourage you to further explore the app as many people have found it to be a very helpful tool for searching their code base for the past 11 years.

@mattslay
Copy link
Owner

@grkncl You said to @DougHennig "I hope it happens to you."

@grkncl Sir, I now invite you to simply leave the entire FoxPro community forever. This kind malicious spewing is absolutely not welcome here. You are obviously not one of us.

Goodbye!

mattslay added a commit that referenced this issue Mar 20, 2021
… GF to delete (essentially) all files and folders starting at the root path of the disk. Error was reported in Issue #9 on the GitHub Issue Tracker.

Added text source code files for VCX and SCX files, as generated by FoxBin2Prg
@mattslay
Copy link
Owner

mattslay commented Mar 20, 2021

This issue has been fixed and source code pushed to GitHub as ver 5.0.164. (bug was fixed within 13 hours of being reported).

I am working with the VFPx Team leaders to get the update released via Thor Check For Updates.

@mattslay mattslay changed the title this stupid app is very dangerous The Delete feature on the Search History form can delete all files and folders on the entire disk (in certain scenarios...) Mar 20, 2021
@GerhardSc
Copy link

Hello Matt!
I'm a happy user of your tool for years now.
@grkncl: No backup - no mercy.

@grkncl
Copy link
Author

grkncl commented Mar 22, 2021

okay cringing @GerhardSc . I hope it happens to you as soon as possible. you restore terabyte backups

@mattslay
Copy link
Owner

mattslay commented Mar 23, 2021

I have blocked user @grkncl from all of my GitHub repos.

Twice he has wished "I hope it happens to you" on other VFP folks here in the comments of this issue. There is no place for that poison here.

Indeed there was an error in my coding of the RemoveFolder() method, that I did not realize what potential issue could be from a passed empty string, and I regret that I did not recognize this potential at the time. Regardless, he took his replies to an unacceptable level by wishing the same havoc on other more reasonable users.

@GerhardSc
Copy link

Well done, Matt!
Never thought that it would be necessary here.

Gerhard Schmidbauer

@mattslay
Copy link
Owner

This message was emailed to the GoFish and Thor Google Groups on 2021-03-24:

Hi GoFish users! I need to alert everyone about a very critical bug in GoFish that was reported last Friday March 19 2021. The error is only in a certain condition when using the "Delete" button at the bottom of the Search History form. Sadly, if this specific error condition is triggered, GoFish will begin deleting all files and folders at the root level of the disk. Yes, all of them! Please read on...

The error condition is that if you use the Delete button to delete all of the Search History records from the grid, even after the last record is deleted, the Delete button will still remain enabled, and if you click it again, while there are no rows in the grid, it will pass an empty string to the GF utility function named RemoveFolder(tcFolder), and the code in that method does not test for empty string, and it uses the ADir() function on that empty string, which results in the starting folder for deletion being the root folder of the disk, and it iterates through all the folders from there attempting to delete them and the contents. This actually happened to a new user who was testing GF and then deleted his initial 3 or 4 Search History records, then he clicked the Delete button again, and the mass-delete unfolded from there. He was (is), understandably, very upset with me (just read the Issue #9 comment stream and you will see.)

The error condition was reported first hand by user @grkncl as reported in Issue #9 on the GitHub repo. If you want to read all the comments about his experience with this issue you can find it here: #9

The good news is that the error has been safely and thoroughly fixed in the latest release of GoFish (ver 5.0.169) which you can get through Thor Check For Updates. Other features have been added in this release also, so I recommend upgrading as soon as you can. Of course, the full and latest source code is available at: https://github.com/mattslay/GoFish

You can see a the recent updates listed in the Change Log section at the bottom of the page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants