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

Provide the option to specify which search engines to use #50

Merged
merged 1 commit into from
Nov 16, 2016

Conversation

Hainish
Copy link
Contributor

@Hainish Hainish commented Nov 11, 2016

This is useful for https://github.com/EFForg/https-everywhere, see issue EFForg/https-everywhere#7277 (comment)

With this, we can specify what search engines to use - best configuration in our case is

python sublist3r.py -d example.com -e Baidu -e Yahoo -e Google -e Bing -e Ask -e Netcraft -e Virustotal -e "SSL Certificates"

@Hainish
Copy link
Contributor Author

Hainish commented Nov 11, 2016

In the default configuration, all engines are used.

Copy link
Collaborator

@the-st0rm the-st0rm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind to merge that pull. The only thing I am not really comfortable with is the eval() you are using. I would probably create just a dictionary and do mapping between the engines and the actual objects.

I will still also discuss this with @aboul3la and see ig that affect the tool from any other perspective.

Thanks a lot for the pull request though :)

@Hainish
Copy link
Contributor Author

Hainish commented Nov 11, 2016

@the-storm: I definitely get that. The reason why I considered it acceptable in this case is because the conditionals effectively whitelist what will ever pass through to the eval statement. I can open another PR following your suggestion if you like, though.

@the-st0rm
Copy link
Collaborator

@Hainish I think that would be very good. So I can merge the PR right away :)

@Hainish
Copy link
Contributor Author

Hainish commented Nov 14, 2016

@the-st0rm: done

@aboul3la
Copy link
Owner

aboul3la commented Nov 15, 2016

Hello @Hainish,

Thanks for your PR. We're glad to support EFF and https-everywhere.

I just have some few suggestions to improve this PR before I can merge it.

  1. I don't think it's good to use many if conditions hard-coded inside the code. I think the optimal way is to use a dict that stores all the possible engines and match the user input against it.
  2. it's better to use a comma separated list for the engine argument rather than using multiple -e args such as-e yahoo,google,bing,baidu it would be better than doing -e yahoo -e google -e bing -e baidu
  3. It's better to use case insensitive matching for the engines names. As your current code will only work if the argument is like -e Yahoo but it won't work if it's -e yahoo

Below is a little code snippet i wrote on how the code should be:

    supported_engines = {'baidu':BaiduEnum,
                         'yahoo':YahooEnum,
                         'google':GoogleEnum,
                         'bing':BingEnum,
                         'ask':AskEnum,
                         'netcraft':NetcraftEnum,
                         'dnsdumpster':DNSdumpster,
                         'virstotal':Virustotal,
                         'threatcrowd':ThreatCrowd,
                         'ssl':CrtSearch,
                         'passivedns':PassiveDNS
                         }

    chosenEnums = []

    if engines == None:
        chosenEnums = [BaiduEnum, YahooEnum, GoogleEnum, BingEnum, AskEnum,
                       NetcraftEnum, DNSdumpster, Virustotal, ThreatCrowd, CrtSearch, PassiveDNS]
    else:
        engines = engines.split(',')
        for engine in engines:
            if supported_engines.has_key(engine.lower()):
                chosenEnums.append(supported_engines[engine.lower()])

Kindly update the PR so I can merge it.
Thanks

@Hainish
Copy link
Contributor Author

Hainish commented Nov 16, 2016

@aboul3la: Thanks for the words of support! I've updated this PR.

@aboul3la aboul3la merged commit ef37c2c into aboul3la:master Nov 16, 2016
@aboul3la
Copy link
Owner

Thank you @Hainish. It's merged now

else:
engines = engines.split(',')
for engine in engines:
if supported_engines.has_key(engine.lower()):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aboul3la This breaks Python3 compatibility because 'has_key' function was removed in Python 3.

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

Successfully merging this pull request may close these issues.

4 participants