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

Disable gethostbyname and getaddrinfo too. #64

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

shaleh
Copy link

@shaleh shaleh commented May 13, 2021

When socket is disabled, disable these address lookups as well.

Addresses #43

When socket is disabled, disable these address lookups as well.
@miketheman miketheman added the enhancement New feature or request label May 13, 2021
@miketheman
Copy link
Owner

Thanks @shaleh ! Looks interesting.
There's at least one test failure, as surfaced by the Actions. I'm not entirely sure why the rest were cancelled.

Please take a look at the failure and see if you can resolve?

I'd also appreciate a new test case for each of the specific sockets being added, since I think the logic will apply to all sockets equally, so if one is blocked, all would be blocked, so the tests should explicitly ensure that if something is calling getaddrinfo() or gethostname() alone, that it's handled correctly

@shaleh
Copy link
Author

shaleh commented May 13, 2021

@miketheman no problem. I did not put a ton of time in case the implementation was not the desired approach. Happy to ensure tests pass and are as desired.

@shaleh
Copy link
Author

shaleh commented May 13, 2021

I am more accustomed to a tox based test world so I do not need to install the tooling in my own environment. I did not see a clean way to test this project as is.

@shaleh
Copy link
Author

shaleh commented May 13, 2021

Fun, works on Linux (where I experimented) and fails everywhere else.

@miketheman miketheman added the more-info-needed Further information is requested label Nov 6, 2021
@codeclimate
Copy link

codeclimate bot commented Dec 23, 2021

Code Climate has analyzed commit 69321a6 and detected 0 issues on this pull request.

View more on Code Climate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request more-info-needed Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants