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 memory leak in scripthttprequesthandler.cpp #831

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

Conversation

xCynDev
Copy link

@xCynDev xCynDev commented Nov 30, 2024

Adds a few calls to freeaddrinfo() that were missing (mostly when returning false in IsHttpDestinationHostAllowed()).

Code review:

Not much, check where I've added freeaddrinfo().

Testing:

Check that it works on your end by making an HTTP call to a non-local, IPv6 address (without -allowlocalhttp!)
And then also make a call to a local IPv4 host, again without -allowlocalhttp.

If you don't crash and the requests work get rejected, it's all good.

@github-actions github-actions bot added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels Nov 30, 2024
@Alystrasz
Copy link
Contributor

@xCynDev can you provide detailed instructions to test your PR?
Describing how you make HTTP calls (CLI command? Mod?) would be a good start! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs code review Changes from PR still need to be reviewed in code needs testing Changes from the PR still need to be tested
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants