YAFD force quits Keepass when trying to fetch favicons.

[Swoy]

Hello, I'm facing a rather problematic issue:

Steps to reproduce:

If I activate the option to use both Automatic prefix URLs with http:// and Use title field if URL field is empty in the order below:

I then mark all items in a specific group (notice the visible columns):

and select the option to download:

Result:

Keepass will suddenly behave as if the entire list is deleted, then the app quit unexpectedly. Luckily, I didn't make any changes to the database during this run, but any unsaved changes is confirmed to be lost if these repro-steps are preformed on an unsaved db.

I'm using it with the following plugins on KeePass 2.40:

plugin	Version
KeeChallenge	1.5.0.0
QuickConnectPlugin	0.4.4.0
ExeIconPicker	1.0.2.0
HIBPOflineCheck	1.3.1.0
KeeAnywhere	1.5.1.0
KeePassHttp	1.8.4.2
KeeTrayTOTP	0.9.5.0
PasswordChangeAssistant	1.1.0.0
QualityColumn	1.2.0.0
QuickConnectPlugin	0.4.4.0
YetAnother Favicon Downloader	1.2.0.0

[navossoc]

Hi, thanks for your report.

Can you please tell me more information about your system? (Windows I guess)

Also to make my life easier, if possible:

1. Have you tried to reproduce the same steps without the other plugins loaded?

2. Is it happening with another group of entries? (different sites or maybe just a subset of that selection)

This will help me to narrow down the issue.

Since you said it is crashing the program, make sure you have a backup first. Just in case 😎

[]'s

[Swoy]

Hello, thanks for the quick answer.

0. I'm running windows 10 yes, latest build.
1. Confirmed issue with all but TOTP plugin disabled.
2. I just tried on another group of entries. Still crashes.

Note: I always make backups :)
Note2: i don't dare to deactivate TOTP plugin, in case something goes awry. But if given more time, I can make a new db and try with a few items (unless you try yourself).

[navossoc]

To be honest, I already tried a few scenarios here.
So far I had no luck to reproduce it.

All I can think about is some of your password entries (title or url field) have any kind of special character or something like that.

Just to make sure: the crash happens only if you enable both options at the same time?

Another tests can be:

1. no options at all
2. just automatic prefix
3. just use title if url is empty
4. prefix + title (you already did, crashed)

By the way, have you tried to select just two random entries and see what happens?
Are you being able to see the form dialog downloading the icons before it crashes?

[]'s

[Swoy]

I've looked further into the issue.
I've also managed to crash KeePass without those settings enabled now.

The issue is not stable, but I've boiled down a few bits; and somehow, it makes it easier to crash KeePass if the following things are among the entries tried:

• Two entries with identical URLs (in URL field)
• KeePassHttp Settings entry
• an entry with dropbox:// protocol in URL field
• an entry with normal string in title field and http://something in URL field.

Sometimes I get an exception window:

Note here: I haven't managed to click details as the window is destroyed too quickly.

When your extension is doing it's job it also looks like there could be some optimization of when it repaints the list (maybe this can be part of the culprit?) As you can see, I've captured a screen of when it crashes and the repaint fails:

It seems that the extension fails to handle some issues correctly either when repainting or when it receives some unknown error during fetching.

[navossoc]

Nice...

This makes sense:

Two entries with identical URLs (in URL field)

Probably a race condition trying to save the same icon.

I'll look further later tonight.

PS: Not 100% sure, but I think this crashes are logged on Event View -> Application.

[]'s

[Swoy]

Yes, it seems like part of the culprit. It also doesn't manage to repaint the list properly and duplicates _(only in view, not in db) all entries after two entries with identical URLs in URL field (dupe URL) is in the job.

I also noticed the following:
It seems that when you do this over an entry where KeeTrayTOTP is storing data and has it displayed in it's own column, it also crashes (unrelated to the dupe URL issue). When I disable the column, it won't crash anymore (as long as there is no dupe URLs in the job).

It took me a few tries to get this screenshot, but I hope what is there could shed some light. unfortunately, it is impossible (for me at least) to scroll in that list before the window is destroyed.

Edit:
Thanks! I've never really cared for windows, thus not considered they actually log issues so the users can see. Anyway, I pulled this from the event viewer:

Application: KeePass.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.NullReferenceException
   at KeePass.UI.UIUtil.SetAlternatingBgColors(System.Windows.Forms.ListView, System.Drawing.Color, Boolean)
   at KeePass.Forms.MainForm.UpdateEntryList(KeePassLib.PwGroup, Boolean)
   at KeePass.Forms.MainForm.UpdateUI(Boolean, KeePass.UI.PwDocument, Boolean, KeePassLib.PwGroup, Boolean, KeePassLib.PwGroup, Boolean, System.Windows.Forms.Control)
   at KeePass.Forms.MainForm.UpdateUI(Boolean, KeePass.UI.PwDocument, Boolean, KeePassLib.PwGroup, Boolean, KeePassLib.PwGroup, Boolean)
   at HIBPOfflineCheck.HIBPOfflineColumnProv.UpdateStatus()
   at HIBPOfflineCheck.HIBPOfflineColumnProv.PwdTouchedHandler(System.Object, KeePassLib.ObjectTouchedEventArgs)
   at KeePassLib.PwEntry.Touch(Boolean, Boolean)
   at YetAnotherFaviconDownloader.FaviconDialog+<>c__DisplayClass6.<BgWorker_DoWork>b__0(System.Object)
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()

[navossoc]

Great, this should be enough information for now.
I let you know if I discover something too...

Looking at the stack is something like you said: refreshing the list.
Probably this in an TOTP plugin issue, but I'll look it anyway.

I believe any refresh on the entry list may trigger this crash.

I don't recall any plugin that refresh the list right now, just this one:
https://github.com/luckyrat/KeePass-Favicon-Downloader

If you want to give it a try too, feel free...

[]'s

[Swoy]

Great! Here's hope we get to the bottom of this. The TOTP can refresh cells that should display TOTP's from what I can see.

Nah, I like this extension more ❤️

[navossoc]

Well... well... well...

I did a few more tests:
KeePass + KeeTrayTOTP = no issues, with or without the plugin column.
KeePass + HIBPOfflineCheck = some issues, with or without the plugin column.
KeePass + KeeTrayTOTP + HIBPOfflineCheck = crash while the download is running (columns show).

I suspect that TOTP refreshes the list and HIBPOfflineCheck too, both at the same time.

From HIBPOfflineCheck page:

Entries are checked automatically after being updated

The call stack you posted on the image is different from the call stack of event log.
One is triggered by TOTP plugin and the other one is trigged by HIBP plugin.

Did you check the event viewer for more stack traces? They help a lot.
You can filter it by severity: Error and source: .NET Framework.

Both plugins has some kinda of refresh feature:

https://github.com/mihaifm/HIBPOfflineCheck/blob/02bf23a8a6c5f4c88b7e5390476e354a0ad5249e/HIBPOfflineCheckExt.cs#L303
(this one tries to scroll the list, maybe is that effect that you see?)

https://github.com/victor-rds/KeeTrayTOTP/blob/a6a431b7e5f8db21bcac6037205687f5b77b827b/TrayTOTP_Plugin.cs#L544

I didn't refresh the list while the download is running.
My code waits all the job is done, then it refreshes it only once.

KeePass-Yet-Another-Favicon-Downloader/YAFD/FaviconDialog.cs

Line 257 in 55d8c05

pluginHost.MainWindow.UIBlockInteraction(false);

Not sure exactly who is fault here, since we have 3 plugins trying to update the same thing at the same time.

Can you reproduce it on a clean environment? I did...

1. Download KeePass (portable).
2. Download and install HIBPOfflineCheck.
3. Download and install KeeTrayTOTP.
4. Create a new database and add a few entries (with some random username and valid urls).
5. Set up TOTP on one of them, just use any random secret.
6. Configure columns to show Have I been pwoned?and TOTP.
7. Try to download favicons.

[Swoy]

Indeed, TOTP is the culprit. and boy do it present a performance hit too. It updates every second to display the count down for each TOTP in there, no matter if it is visible or hidden ****. 😠

I will write up an issue over there for this and a few other things.

I've made a test DB with 2000 entries to test stability and the like (password is 123):

Database.zip

When using this and downloading favicons for all 2000 entries, I experienced no issue whatsoever. The list update after was buttersmooth as well.

I am so sorry btw:

I also noticed a rookie mistake I did, I forgot to clear the plugin cache. After clearing it, and disabling TOTP (and all the other plugins) your plugin worked with no issues so far. This seems to have been why, as for some reason TOTP was still working in the back? I have no idea as to why. But I apologize for the time I stole from you.

One last thing, does your plugin consider multiple of the same domain?? On that DB I attached, would it maybe complete quicker if you first ran a check for similar domains and group them as one during prep? Since there are ~400 unique domains, and that corresponds with the number of icons found in the Use custom icon menu.

[navossoc]

I've made a test DB with 2000 entries to test stability and the like (password is 123):

I have a DB with ~10k entries which I was using for testing while developing YAFD.

KeePass itself already has a good drop in performance if you have too many entries, especially if you have the sort / grouping features enabled.

Also, if you have many custom icons in the database, it may take a while to save.

When using this and downloading favicons for all 2000 entries, I experienced no issue whatsoever. The list update after was buttersmooth as well.

That is why I decided to lock the user interface while the downloading is running, to prevent entries being added/removed and all related repainting/refreshing.

One last thing, does your plugin consider multiple of the same domain??

Just when saving the custom icon (to avoid duplicates).

On that DB I attached, would it maybe complete quicker if you first ran a check for similar domains and group them as one during prep?

To be honest, I thought about that while designing it, but the cost to list all URLs and check if the TLD (top level domain) was the same, it ended up being much higher than downloading the icon again.

Therefore, I doubt that a normal user has so many repeated entries to the same site.
So I thought it was not worth implementing it.

---

HIBPOfflineCheck has an event that is triggered every time an entry is touched (updated).
During my tests, he was primarily responsible for crashing the program while the list of icons was being updated.

I think I can make some improvements on my end to help my plugin play nice with other plugins while I'm updating the icons.

[Swoy]

Thanks for looking into this anyway!

KeePass itself already has a good drop in performance if you have too many entries, especially if you have the sort / grouping features enabled.

I haven't experienced any performance issues what-so-ever, I do however get application hang errors when I try to import csv's larger than 20MB (I haven't tried 10MB etc.). But I digress;

HIBPOfflineCheck has an event that is triggered every time an entry is touched (updated).
During my tests, he was primarily responsible for crashing the program while the list of icons was being updated.

It seems that HIBPOfflineCheck indeed causes havoc during your plugins update. I simply disabled it since I only use it once in a while. While KeeTrayTOTP is in development I suppose he will look into these issues on his side as well - considering how useful his plugin is to many people.

[navossoc]

I haven't experienced any performance issues what-so-ever.

Try to put all the entries on the same group or use the option view -> show entries of subgroups.
It will hang for a while, especially if sort and group is enabled.

Anyway...

[navossoc]

@Swoy not sure if you get luck with the other plugins, but I did a small change that may help in cases like this.