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

Catalog response code #1207

Merged
merged 1 commit into from
Apr 10, 2022
Merged

Conversation

jonasbb
Copy link
Contributor

@jonasbb jonasbb commented Sep 16, 2020

This fixes how Catalog sets the BADVERS response code.

@jonasbb jonasbb closed this Oct 2, 2020
@jonasbb jonasbb reopened this Oct 2, 2020
@codecov
Copy link

codecov bot commented Oct 2, 2020

Codecov Report

Merging #1207 into main will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1207      +/-   ##
==========================================
- Coverage   85.62%   85.61%   -0.01%     
==========================================
  Files         125      125              
  Lines       13840    13923      +83     
==========================================
+ Hits        11850    11920      +70     
- Misses       1990     2003      +13     

@djc
Copy link
Collaborator

djc commented Oct 5, 2020

Did you look into combining this with a fix for proto's Message? Would be nice to have.

@jonasbb
Copy link
Contributor Author

jonasbb commented Oct 9, 2020

This was meant as a quick fix before I can take another look at #1203. The best way, for now, is probably to silently create the EDNS section whenever necessary.

jonasbb added a commit to jonasbb/trust-dns that referenced this pull request Oct 26, 2020
Some ResponseCodes have high bits which require EDNS to encode them.
This commit updates Message::set_response_code and
MessageResponseBuiler::error_msg to silently create a EDNS section if
required and sets the high bits there.

This also adds a warning to Header::set_response_code that this function
cannot set the high bits.

Closes hickory-dns#1203
Closes hickory-dns#1207
@bluejekyll
Copy link
Member

What's the status on this change?

@jonasbb
Copy link
Contributor Author

jonasbb commented Dec 29, 2020

@bluejekyll This 1 line change correctly sets the response code in the case of BADVERS.

This code remains correct also with #1265 applied, but this PR could then be reverted if wanted.
The problem with #1265 remains the same, that I do not understand why round-tripping the message does not work.

jonasbb added a commit to jonasbb/trust-dns that referenced this pull request Mar 11, 2022
Some ResponseCodes have high bits which require EDNS to encode them.
This commit updates Message::set_response_code and
MessageResponseBuiler::error_msg to silently create a EDNS section if
required and sets the high bits there.

This also adds a warning to Header::set_response_code that this function
cannot set the high bits.

Closes hickory-dns#1203
Closes hickory-dns#1207
jonasbb added a commit to jonasbb/trust-dns that referenced this pull request Mar 11, 2022
Some ResponseCodes have high bits which require EDNS to encode them.
This commit updates Message::set_response_code and
MessageResponseBuiler::error_msg to silently create a EDNS section if
required and sets the high bits there.

This also adds a warning to Header::set_response_code that this function
cannot set the high bits.

Closes hickory-dns#1203
Closes hickory-dns#1207
@bluejekyll bluejekyll merged commit a973039 into hickory-dns:main Apr 10, 2022
@bluejekyll
Copy link
Member

I didn't see the updates on this. Sorry for the delayed merging and thanks for the PR!

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.

None yet

3 participants