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

DRAFT: Findcdn CDN Engine Massive Rework (v1.0.0) #56

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Pascal-0x90
Copy link
Collaborator

🗣 Description

findcdn's old CDNEngine was clunky and took forever to run. An immense rework has been undertaken
to merge in a lot of the ides from:

To rework the entire code-base and allow the following key points:

  • More extendable and modular analyzers.
  • Using requests library instead of urllib.
  • No confusing Chef -> DomainPot -> .... relationships.
  • Argument based parameters to disable different analyses to give you the choice on which analysis to run and not run.
  • Improvement in scanning performance by not requiring the need for all data but just when the first sight of a CDN is found.
    • Also, stop analysis on domain if IP could not be resolved (dead domain avoidance)
  • Handle improper domains more gracefully by adding it to standard output for user to grab.
  • Give total runtime in output to help with timing performance.

💭 Motivation and context

Main goal: make findcdn better, faster, stronger. 😎

Findcdn has been untouched for a while now without any significant improvements.
Thankfully to people such as @Yablargo, @kz0ltan, @MC189, and @mcdonnnj for making issues
and pull requests to help improve the quality and longevity of this repo.

Main Issues

  • Hard to extend with new analyzers.
  • Code was hard to develop on due to complexity in class relations.
  • Analyzing even one domain was super slow.

🧪 Testing

View initial testing in #43 (comment)

Test creation is still in progress.

  • Test creation completed (in progress)

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All future TODOs are captured in issues, which are referenced
    in code comments.
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated
    to reflect the changes in this PR.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

✅ Pre-merge checklist

  • Revert dependencies to default branches.
  • Finalize version.

✅ Post-merge checklist

  • Add a tag or create a release.

Pascal-0x90 and others added 3 commits November 23, 2022 12:24
Co-authored-by: Yablargo <Yablargo@users.noreply.github.com>
Co-authored-by: kz0ltan <kz0ltan@users.noreply.github.com>
@Pascal-0x90 Pascal-0x90 marked this pull request as draft November 27, 2022 00:20
@lgtm-com
Copy link

lgtm-com bot commented Nov 27, 2022

This pull request introduces 2 alerts when merging 7cb5c87 into 7bbf50e - view on LGTM.com

new alerts:

  • 2 for Explicit export is not defined

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@kz0ltan kz0ltan mentioned this pull request Dec 3, 2023
8 tasks
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.

1 participant