Skip to content

Add 3rd parameter to getRecords to optionally skip filtering records #54

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xorinzor
Copy link

Fix for #53

Adds a 3rd parameter to getRecords to optionally skip filtering records.
Backwards compatible with the original code.

This allows a lookup for any kind of record type (for example TXT) to also return CNAME records. This is consistent with how dig also returns records.

Add 3rd parameter to getRecords to optionally skip filtering records
@remotelyliving
Copy link
Owner

I will take a look at this and the issue tonight, thanks for submitting!

@xorinzor
Copy link
Author

Turns out this doesn't work for RemotelyLiving\PHPDNS\Resolvers\Dig since that class appears to filter records before returning them regardless. That resolver class would require (breaking) rework in order to just return the record-set from the lookup.

Currently Dig is incapable of returning the full record-set unless the ANY type is provided (in which case it'll also query all types individually).

The CloudFlare resolver however works different, as it does return the full record set and doesn't get filtered by type within the class itself. Arguably this is the correct way for resolvers to work as you'd want to manage the filtering within your application, as well as the fact that Dig is in it's current form incapable of returning the actual results as the command dig would.

I do think my PR still adds value, but I thought it'd be important to mention this.

@xorinzor
Copy link
Author

@remotelyliving did you have a chance to look at this yet?

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.

2 participants