search

codingo / VHostScan

A virtual host scanner that performs reverse lookups, can be used with pivot tools, detect catch-all scenarios, work around wildcards, aliases and dynamic default pages.
GNU General Public License v3.0
1.19k stars 231 forks source link

added ratelimit - not tested #39

Closed cryguy closed 6 years ago

cryguy commented 6 years ago

added ratelimit, might need fixing , not the cleanest

codingo commented 6 years ago

@cryguy We just had two pr's for this (in a space of 20m!). Could you take a look at the pr from @grimd34th and let me know if you feel there's an edge case your PR could handle that wouldn't be managed by #40?

At the moment I'm leaning towards #40 due to the lower dependencies / code base but I'm happy to be persuaded if this is functionality more flexible. Still going to add you to /CREDITS either way!

cryguy commented 6 years ago

I feel that my method is better? As some apis allow for 100 runs in an hour, the code can be modified to allow for that

cryguy commented 6 years ago

Im new to this and english is not my first language so im sorry if the grammar is bad :)

codingo commented 6 years ago

Potentially that's an edge case - I'll have to read into it more. No dramas about the grammar, code speaks :) I'm going hold back on this for a while to give @grimd34th a chance to respond.

Happy to pull these both down into VM's and test them over the weekend in different scenarios as well. If we do go with this option I'll probably split it out into a new helper class to make it a bit easier to manage over the longer term.

cryguy commented 6 years ago

:) happy to help

grimd34th commented 6 years ago

I suppose it is up to @codingo, based on how many dependencies he wants to introduce and code complexity increases. My solution can have the same outcome as cryguy's with mine being at the end of every request instead of spaced throughout, so its up to the user to know the time they want to sleep after each request.

codingo commented 6 years ago

Not ignoring either of these - going to have to build out a new test case for them so will review in 24 hours.

codingo commented 6 years ago

I'm going to close this thread - for a few reasons.

Primarily both @timkent and I want to limit the dependencies we have within the application to keep it as lightweight as possible. I find @grimd34th's solution to be the better of the two here as I was also unable to find any use case where this branch would provide functionality above @grimd34th's.

I also took a bit of an issue with code being directly copied (https://github.com/tomasbasham/ratelimit/blob/master/ratelimit/__init__.py) without a citation. In future commits I'd suggest also citing where any significant blocks of code have come from so we can add it to /CREDITS. I'm a lover of open source, but I also think it's very important to cite your sources and give credit where credit due.

All of that aside, thank-you for taking the time. There's still a lot of other issues remaining in this project if you want to throw your hat in again!