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

Fix importing CIDR blocks #1205

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix importing CIDR blocks #1205

wants to merge 4 commits into from

Conversation

pbehnke
Copy link

@pbehnke pbehnke commented Feb 26, 2024

Importing CIDR blocks as targets was broken in a few different ways.

  1. When adding CIDR targets via the multiple targets tab, there was a variable name collision that resulted in an infinite loop and memory leak being created which would quickly exhaust all memory on the system until the web process was killed. The name of the variable was changed from "ips" to "_ips" to correct this.
  2. When adding a CIDR block via the resolve IP tab, the CIDR block was passed directly to gethostbyaddr(), which does not accept CIDR blocks, resulting in an error. This PR fixes that issue by expanding the CIDR block and passing each IP to gethostbyaddr(), and returning the domains for any which were resolved. This required a small API change to the IPToDomain API to send back a list of objects for each IP in the CIDR block and any resulting found domains for each IP.
  3. When adding any domains found by resolving an IP or CIDR block, those domains were not added to the database of targets, resulting in an error stating "Oops! Could not import any targets, either targets already exists or is not a valid target." This PR adds the selected target domains to the database and updates the added_target_count. It also adds any IP targets to the IpAddress model, similar to when IPs are added to via the multiple targets tab.

Thanks to my employer, EasyPost, for allowing me the time to resolve these issues.

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

Successfully merging this pull request may close these issues.

None yet

3 participants