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 --class not being respected by CLI #430

Merged
merged 3 commits into from
Aug 23, 2024
Merged

Conversation

phillip-stephens
Copy link
Contributor

Closes #426

NOTE - presently, all modules hard-code INET as the class, except BINDVERSION which sets it as CHAOS. This is not changed in this PR.

Description

  • populate lookup module's Class field so user's --class choice is respected

Testing

It's not really possible (that I can figure) how to test this in integration tests. Running tcpdump and evaluating the output seems iffy on random CI systems and no DNS server seemed to respond when I set the class as anything other than INET, so I ended up just showing both in Wireshark.

With Wireshark, I ran ./zdns A zdns-testing.com --name-servers=1.1.1.1 --class=CHAOS with the Display Filter sudo tcpdump -i en0 udp dst port 53 and dst host 1.1.1.1

main

16:57:09.209413 IP phillips-laptop.lan.56789 > one.one.one.one.domain: 53589+ [1au] A? zdns-testing.com. (45)

Phillip/426...

16:57:19.112017 IP phillips-laptop.lan.60563 > one.one.one.one.domain: 14018+ [1au] A CHAOS? zdns-testing.com. (45)

@phillip-stephens
Copy link
Contributor Author

@zakird WDYT, should we explicitly warn users with non-basic modules that --class is not supported or should I respect --class no matter the module?

@zakird zakird marked this pull request as ready for review August 23, 2024 22:27
@zakird zakird requested a review from a team as a code owner August 23, 2024 22:27
@zakird
Copy link
Member

zakird commented Aug 23, 2024

I don't have strong opinions, I don't actually know the use case when you'd want this to be different, so I think take the path of least resistance and we can revisit if need be.

@zakird zakird merged commit 570a102 into main Aug 23, 2024
3 checks passed
@zakird zakird deleted the phillip/426-class-not-respected branch August 23, 2024 23:21
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.

--class CLI option isn't respected
2 participants