-
Notifications
You must be signed in to change notification settings - Fork 391
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
refactoring to fix InterfaceError of DB #400
Conversation
a2b5a44
to
e98d048
Compare
Updated. Accidentally delete |
@dazzgt nice, thanks for this PR. Have you run the e2e tests against any hosts? |
I ran it against the same subnet where i found this bug. Also there some rdp, winrm, smb test hosts, so i tried to run this protocols too. All works perfectly fine |
Thanks for the PR! |
e98d048
to
be27b22
Compare
@NeffIsBack fixed two ruff errors. |
dd11405
to
355becd
Compare
355becd
to
96df458
Compare
If this solution is not good, then i open to suggestion how to fix it more elegant) |
Hi, I am currently on vacation so I am not reviewing PRs at the moment. There are also a lot of PRs still waiting for a review, but I will take a deeper look at this one when am back at home. |
I'm sorry I haven't been able to test this PR on my side, I'm lacking time to do so but I believe it's a good improvement |
96df458
to
3b1eac7
Compare
I will test the performance of this next week, if there isn't a drastic change we should be good to go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Issue topic #395
Summary
When netexec find a lot of suitable target hosts it start too many calls to DB at once and because of nature of SQLite it leads to
InterfaceError
. The same can happen with much fewer hosts but get that timing is much harder.This PR don't fix IndexError mentioned in issue topic because it require dirty hack in current realization.
@AkechiShiro you can try to test it.