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.

Sign up for GitHub

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

Jump to bottom

Ensure DNS records are only requested for IP allowlist when needed #18285

Merged
merged 2 commits into from Nov 10, 2021
Merged

Ensure DNS records are only requested for IP allowlist when needed #18285

merged 2 commits into from Nov 10, 2021

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Nov 9, 2021

Description:

  • only request dns records if it's not an IP range
  • cache DNS results for 30 seconds to reduce DNS requests

Note: Tracking requests never go through the allow-list, as that's excluded here:

matomo/plugins/CoreHome/CoreHome.php

Lines 78 to 81 in bc75f9d

if (SettingsServer::isTrackerApiRequest()) {
// authenticated tracking requests should always work
return;
}

fixes #18244

Review

@sgiehl
Ensure DNS records are only requested for IP allowlist when needed
4faf5f5 
- only request dns records it it's not an IP range
- cache DNS results for 30 seconds to reduce DNS requests
@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Nov 9, 2021
@sgiehl sgiehl added this to the 4.6.0 milestone Nov 9, 2021
tsteur
tsteur requested changes Nov 9, 2021
View reviewed changes
Copy link
Member

@tsteur tsteur left a comment

Choose a reason for hiding this comment

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

Works overall @sgiehl . Few tweaks be needed though.

config/global.php Show resolved Hide resolved
config/global.php Outdated Show resolved Hide resolved
config/global.php Outdated Show resolved Hide resolved
@sgiehl
Copy link
Member Author

sgiehl commented Nov 10, 2021

@tsteur Thanks for the suggestions. I've updated the code accordingly.

@tsteur
Copy link
Member

tsteur commented Nov 10, 2021

Works 🚀

@tsteur tsteur merged commit e8e0f2a into 4.x-dev Nov 10, 2021
@tsteur tsteur deleted the fix18244 branch November 10, 2021 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsteur tsteur tsteur requested changes

Assignees
No one assigned
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.6.0
Development

Successfully merging this pull request may close these issues.

Ensure DNS records are only requested for IP allowlist when needed
2 participants
@sgiehl @tsteur