refactoring: remove the duplicated TXT decoding in dns_parser - #500
Merged
Merged
Conversation
dns_parser and service_info each carried their own copy of decode_txt, the TxtProperty struct, its five From impls, its Display and Debug impls, HEX_TABLE and u8_slice_to_hex -- about 145 lines, identical apart from the log macro (trace! vs debug!) and the wording of one message. Two copies of a decoder for attacker-supplied bytes will drift, and a fix applied to one would not reach the other. dns_parser now uses the service_info copy, which is the canonical one: it is the public, serde- derived TxtProperty that the crate re-exports, and it has the key() and val() accessors the dns_parser copy lacked. dns_parser already depends on service_info, so this adds no new coupling. The only behavior change is that the "TXT record contains invalid data" message, on the Debug and rdata_print paths, is now logged at debug rather than trace, matching what the resolve path has always used. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
dns_parser and service_info each carried their own copy of
decode_txt, theTxtPropertystruct, its five From impls, its Display and Debug impls, HEX_TABLE and u8_slice_to_hex, identical apart from the log macro (trace! vs debug!) and the wording of one message.This was due to historical reason: we tried to separate dns_parser out into own crate. Now we should remove the duplicated copy.
dns_parsernow uses theservice_infocopy, which is the canonical one: it is the public, serde- derived TxtProperty that the crate re-exports, and it has the key() and val() accessors the dns_parser copy lacked.dns_parseralready depends on service_info, so this adds no new coupling.