From bb6e2c4f4cae46a3875bb97d418f9f602aa95449 Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Fri, 10 Apr 2026 02:05:18 +0300 Subject: [PATCH] test: cover label cap and empty-label rollback; trim doc comments Closes coverage gaps left by PR #54: - write_rejects_label_over_63_bytes: pins the incremental 63-byte cap inside write_qname's inner loop (boundary at 63 vs 64). - write_skips_empty_labels: pins the rollback branch (pos = len_pos) triggered by leading or consecutive dots. Doc comments tightened: - write_qname: drop the streaming-impl walkthrough and the escape-grammar restatement (already documented on read_qname). - name_to_wire: drop the implementation explanation; keep the post-escape lowercasing rationale, which pins behavior a future refactor could silently break. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/buffer.rs | 32 +++++++++++++++++++++++++------- src/dnssec.rs | 10 +++------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/buffer.rs b/src/buffer.rs index 2308813..4e954c9 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -180,13 +180,7 @@ impl BytePacketBuffer { } /// Write a qname in wire format, parsing RFC 1035 §5.1 text escapes. - /// - /// Dots are label separators unless escaped as `\.`. `\\` yields a literal - /// backslash, and `\DDD` (three decimal digits) yields an arbitrary byte. - /// - /// Streams directly into the buffer by reserving a length byte, writing - /// the label body, then backpatching the length. Zero intermediate - /// allocations on the common path. + /// See `read_qname` for the escape grammar. pub fn write_qname(&mut self, qname: &str) -> Result<()> { if qname.is_empty() || qname == "." { self.write_u8(0)?; @@ -368,6 +362,19 @@ mod tests { assert_eq!(&buf.buf[..buf.pos], b"\x04\x00foo\x03com\x00"); } + #[test] + fn write_skips_empty_labels() { + // Leading dot — first (empty) label is rolled back. + let mut buf = BytePacketBuffer::new(); + buf.write_qname(".foo.com").unwrap(); + assert_eq!(&buf.buf[..buf.pos], b"\x03foo\x03com\x00"); + + // Consecutive dots — middle empty label is rolled back. + let mut buf = BytePacketBuffer::new(); + buf.write_qname("foo..com").unwrap(); + assert_eq!(&buf.buf[..buf.pos], b"\x03foo\x03com\x00"); + } + #[test] fn write_rejects_out_of_range_decimal_escape() { let mut buf = BytePacketBuffer::new(); @@ -386,6 +393,17 @@ mod tests { assert!(buf.write_qname("\\1").is_err()); } + #[test] + fn write_rejects_label_over_63_bytes() { + // 64 bytes exceeds the wire-format label cap. + let mut buf = BytePacketBuffer::new(); + assert!(buf.write_qname(&"a".repeat(64)).is_err()); + + // 63 bytes is the maximum permitted label length. + let mut buf = BytePacketBuffer::new(); + assert!(buf.write_qname(&"a".repeat(63)).is_ok()); + } + #[test] fn roundtrip_preserves_dot_in_label() { assert_eq!(write_then_read("exa\\.mple.com"), "exa\\.mple.com"); diff --git a/src/dnssec.rs b/src/dnssec.rs index 8da1e6c..8614810 100644 --- a/src/dnssec.rs +++ b/src/dnssec.rs @@ -722,14 +722,10 @@ pub fn verify_ds(ds: &DnsRecord, dnskey: &DnsRecord, owner: &str) -> bool { // -- Canonical wire format -- /// Encode a DNS name in canonical wire form per RFC 4034 §6.2: -/// uncompressed, with all ASCII letters lowercased. +/// uncompressed, with ASCII letters lowercased. /// -/// Label parsing and RFC 1035 §5.1 escape handling live in -/// `BytePacketBuffer::write_qname`; this function then walks the emitted -/// wire bytes once to lowercase label bodies (length bytes stay untouched). -/// Lowercasing has to happen post-escape-resolution because a decimal -/// escape like `\065` yields `'A'`, which canonical form must convert -/// to `'a'`. +/// Lowercasing happens *after* escape resolution because `\065` yields +/// `'A'`, which canonical form must convert to `'a'`. pub fn name_to_wire(name: &str) -> Vec { let mut buf = BytePacketBuffer::new(); buf.write_qname(name)