8000 Master issue295 tsig fix by frankarinnet · Pull Request #296 · dnsjava/dnsjava · GitHub
[go: up one dir, main page]
Skip to content

Master issue295 tsig fix #296

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
Closed
wants to merge 5 commits into from
Closed

Master issue295 tsig fix #296

wants to merge 5 commits into from

Conversation

frankarinnet
Copy link
Contributor

This pull request is in reference to: #295

There are still some TODOs, however, because I need some guidance on how to deal with thread safety in the TSIG.

So, don't merge this without commenting.

@frankarinnet
Copy link
Contributor Author

Fixing the remaining code formatting issues makes it a bit harder to see what I've changed vs the original.
I'll make sure to fix them up before a final commit.

@ibauersachs
Copy link
Member

Thank you! I'll need some time to work on this.

Could you in the meantime please help me out to make this easier:

  • target the 3.5.x branch
  • create a unit test that fails without your changes

If that's too complicated, then some sample code and zone file with keys pasted or attached here would also help a lot.

@frankarinnet frankarinnet mentioned this pull request Oct 23, 2023
Closed
@frankarinnet
Copy link
Contributor Author
frankarinnet commented Oct 23, 2023 •
edited
Loading

I'll give the unit test a shot on Tuesday; these sorts of gory-DNS details are a little outside my experience, though. My current testing has been against an NSD server in our staging environment.

Also tomorrow, I'll work on targetting the 3.5.x branch.

@frankarinnet
Copy link
Contributor Author
frankarinnet commented Oct 24, 2023 •
edited
Loading

I've created: https://github.com/frankarinnet/axfr

Which might be helpful for testing this. It was the simplest program I could write to demonstrate this bug for manual testing.

But you will still need to:

  • set up a DNS server
  • with TSIG
  • that allows full zone transfers (AXFR)
  • configured with a few different-sized zones

For zone sizes, I've been using:

  • a small zone (with 8 lines)
  • a medium zone (with 8k lines)
  • a large zone (with 160k lines)

@ibauersachs
Copy link
Member

Great, thank you!

@frankarinnet frankarinnet changed the base branch from master to release/3.5.x October 24, 2023 18:56
@frankarinnet frankarinnet changed the base branch from release/3.5.x to master October 24, 2023 18:57
@frankarinnet frankarinnet reopened this Oct 24, 2023
@frankarinnet
Copy link
Contributor Author

Since my fix branched from master, I may need to create a third PR properly branched from 3.5.x.

@frankarinnet frankarinnet mentioned this pull request Oct 24, 2023
Closed
@frankarinnet
Copy link
Contributor Author
frankarinnet commented Oct 24, 2023 •
edited
Loading

Created #298, properly pointed against 3.5.x.

Any objections to me closing this (obselete) PR?

@frankarinnet frankarinnet mentioned this pull request Oct 24, 2023
Closed
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

87.2% 87.2% Coverage
0.0% 0.0% Duplication

@nguichon nguichon deleted the master_issue295_tsig_fix branch October 26, 2023 13:58
@frankarinnet
Copy link
Contributor Author

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@frankarinnet @ibauersachs @nguichon
0