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 displaying of chained domain names. #253

Merged
merged 7 commits into from
Jan 12, 2024
Merged

Fix displaying of chained domain names. #253

merged 7 commits into from
Jan 12, 2024

Conversation

partim
Copy link
Member

@partim partim commented Jan 9, 2024

This PR fixes the Display implementation of Chain<_,_> so it considers all possible combinations of empty, non-empty, and root names. It now behaves similarly to the Display implementation of Dname in that it only adds a final dot to an absolute name if it is the root label only. Similarly to Dname, it also adds Chain::fmt_with_dot which returns a temporary type that formats absolute names with a final dot.

Fixes #252.

@partim partim requested a review from a team January 9, 2024 11:57
write!(f, "{}.{}", self.left, self.right)
let mut empty = true;
for label in self.iter_labels() {
if label.is_root() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the last label should ever be a root, is that correct? Yet this is checked for every label, does that make sense?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can’t think of a way to only check on the last label. As in: how do I know a label is last? (This check is extremely cheap, basically checking if the label is empty.)

Copy link
Member

@ximon18 ximon18 Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe using split_last() ? Although that's on a slice rather than an iterator.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That and it also iterates over the whole thing to find the last label.

@partim partim merged commit 074ec4e into main Jan 12, 2024
24 checks passed
@partim partim deleted the display-chain-fix branch January 12, 2024 12:24
partim added a commit that referenced this pull request Apr 30, 2024
Breaking changes

* All types and functions referring to domain names have been changed from
  using the term “dname” to just “name.” For instance, `Dname` has become
  `Name`, `ToDname` has become `ToName`, and `ToDname::to_dname` has become
  `ToName::to_name`. ([#290])
* The `ToName` and `ToRelativeName` traits have been changed to have a
  pair of methods a la `try_to_name` and `to_name` for octets builders
  with limited and unlimited buffers, reflecting the pattern used
  elsewhere. ([#285])
* The types for IANA-registered parameters in `base::iana` have been
  changed from enums to a newtypes around their underlying integer type
  and associated constants for the registered values. (This was really
  always the better way to structure this.) ([#276], [#298])
* The `Txt` record data type now rejects empty record data as invalid. As
  a consequence `TxtBuilder` converts an empty builder into TXT record
  data consisting of one empty character string which requires
  `TxtBuilder::finish` to be able to return an error. ([#267])
* `Txt` record data serialization has been redesigned. It now serialized as
  a sequence of character strings. It also deserializes from such a sequence.
  If supported by the format, it alternatively deserializes from a string that
  is broken up into 255 octet chunks if necessary. ([#268])
* The text formatting for `CharStr` has been redesigned. The `Display`
  impl now uses a modified version of the representation format that
  doesn’t escape white space but also doesn’t enclose the string in
  quotes. Methods for explicitly formatting in quoted and unquoted
  presentation format are provided. ([#270])
* The `validate::RrsigExt` trait now accepts anything that impls
  `AsRef<Record<..>>` to allow the use of smart pointers. ([#288] by
  [@hunts])
* The stub resolver now uses the new client transports. This doesn’t change
  how it is used but does change how it queries the configured servers.
  ([#215])
* The sub resolver’s server configuration `Transport` type has been
  changed to be either `Transport::UdpTcp` for trying UDP and if that
  leads to a truncated answer try TCP and `Transport::Tcp` for only trying
  TCP. The stub resolver uses these accordingly now ([#296])
* Many error types have been changed from enums to structs that hide
  internal error details. Enums have been kept for errors where
  distinguishing variants might be meaningful for dealing with the error.
  ([#277])
* Renamed `Dnskey::is_zsk` to `is_zone_key`. ([#292])
* Split RRSIG timestamp handling from `Serial` into a new type
  `rdata::dnssec::Timestamp`. ([#294])
* Upgraded `octseq` to 0.5. ([#257])
* The minimum Rust version is now 1.70. ([#304])

New

* Add impls for `AsRef<RelativeDname<[u8]>>` and `Borrow<RelativeDname<[u8]>>`
  to `RelativeDname<_>`. ([#251] by [@torin-carey])
* Added `name::Chain::fmt_with_dots` to format an absolute chained name
  with a final dot. ([#253])
* Added a new `ParseAnyRecordData` trait for record data types that can
  parse any type of record data. ([#256])
* Added implementations of `OctetsFrom` and `Debug` to `AllOptData` and
  the specific options types that didn’t have them yet. ([#257])
* Added missing ordering impls to `ZoneRecordData`, `AllRecordData`,
  `Opt`, and `SvcbRdata`. ([#293])
* Added `Name::reverse_from_addr` that creates a domain name for the
  reverse lookup of an IP address. ([#289])
* Added `OptBuilder::clone_from` to replace the OPT record with the
  content of another OPT record. ([#299])
* Added `Message::for_slice_ref` that returns a `Message<&[u8]>`. ([#300])

Bug fixes

* Fixed the display implementation of `name::Chain<_, _>`. ([#253])
* Fixed the display implementation of `rdata::Txt<..>`. It now displays
  each embedded character string separately in quoted form. ([#259])
* Fixed the extended part returned by `OptRcode::to_parts` (it was shifted
  by 4 bits too many) and return all 12 bits for the `Int` variant in
  `OptRcode::to_int`. ([#258])
* Fixed a bug in the `inplace` zonefile parser that made it reject
  character string of length 255. ([#284])

Unstable features

* Added the module `net::client` with experimental support for client
  message transport, i.e., sending of requests and receiving responses
  as well as caching of responses.
  This is gated by the `unstable-client-transport` feature. ([#215],[#275])
* Added the module `net::server` with experimental support for server
  transports, processing requests through a middleware chain and a service
  trait.
  This is gated by the `unstable-server-transport` feature. ([#274])
* Added the module `zonetree` providing basic traits representing a
  collection of zones and their data. The `zonetree::in_memory` module 
  provides an in-memory implementation. The `zonetree::parsed` module
  provides a way to classify RRsets before inserting them into a tree.
  This is gated by the `unstable-zonetree` feature. ([#286])
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.

4 participants