p2p: add consensus_encoding impls to CommandString
What the developer wrote
Authored by Nick Johnson
p2p: add consensus_encoding impls to CommandString
What changed, and why it matters
This commit adds new encoding and decoding logic for Bitcoin P2P network command strings (the 12-byte labels on messages like 'version' or 'ping'). The decoder trims trailing zero padding and checks that the remaining bytes are plain ASCII before converting them to a Rust string. The conversion uses an 'unchecked' UTF-8 function, but only after an ASCII check, so it is safe in itself. The change is a routine protocol implementation addition; there is no disclosed security bug or fix.
No immediate action required. Reviewers may want to confirm that the `CommandString` constructor enforces the 12-byte length invariant so the encoder's `debug_assert` cannot be bypassed in release builds, and verify that the new decoder's stricter ASCII requirement is acceptable for all intended P2P use cases.
Security signals we found
Use of `unsafe { String::from_utf8_unchecked(...) }` is present, but guarded by a prior `is_ascii()` check, making it sound.
New decoder rejects non-ASCII command strings, which is stricter than the legacy Bitcoin protocol (command strings are conventionally ASCII).
No bounds overflow: the fixed 12-byte array and `debug_assert!(strbytes.len() <= 12)` prevent encoder overruns in non-debug builds the slice copy is bounded by `strbytes.len()` into a 12-byte destination; caller must ensure `CommandString` invariant holds.
No memory safety issue, panic, or remote exploit path is evident from the diff.
Evidence from the diff
The patch introduces encoding::Encodable/Decodable implementations for CommandString in p2p/src/message.rs. The encoder copies at most 12 ASCII bytes into a fixed 12-byte array. The decoder uses a 12-byte array decoder, strips trailing NULs, validates the remainder is ASCII, then uses String::from_utf8_unchecked. Because ASCII is a subset of valid UTF-8, the unchecked conversion is sound. The commit does not modify existing consensus-critical decoding paths; it adds a parallel new-style encoding API. No vulnerability, CVE, or security fix is mentioned in the commit or supplied references.
Changed components
rust-bitcoin p2p message handlingp2p/src/message.rsCommandString typenew consensus_encoding APIInspect captured patch +76 / −0
diff --git a/p2p/src/message.rs b/p2p/src/message.rs
index 41c158f6..fca41136 100644
--- a/p2p/src/message.rs
+++ b/p2p/src/message.rs
@@ -134,6 +134,82 @@ impl Decodable for CommandString {
}
}
+impl encoding::Encodable for CommandString {
+ type Encoder<'e> = encoding::ArrayEncoder<12>;
+
+ fn encoder(&self) -> Self::Encoder<'_> {
+ let mut rawbytes = [0u8; 12];
+ let strbytes = self.0.as_bytes();
+ debug_assert!(strbytes.len() <= 12);
+ rawbytes[..strbytes.len()].copy_from_slice(strbytes);
+ encoding::ArrayEncoder::without_length_prefix(rawbytes)
+ }
+}
+
+impl encoding::Decodable for CommandString {
+ type Decoder = CommandStringDecoder;
+
+ fn decoder() -> Self::Decoder { CommandStringDecoder { inner: encoding::ArrayDecoder::new() } }
+}
+
+/// Decoder for [`CommandString`].
+pub struct CommandStringDecoder {
+ inner: encoding::ArrayDecoder<12>,
+}
+
+impl encoding::Decoder for CommandStringDecoder {
+ type Output = CommandString;
+ type Error = CommandStringDecodeError;
+
+ fn push_bytes(&mut self, bytes: &mut &[u8]) -> Result<bool, Self::Error> {
+ self.inner.push_bytes(bytes).map_err(CommandStringDecodeError::UnexpectedEof)
+ }
+
+ fn end(self) -> Result<Self::Output, Self::Error> {
+ let rawbytes = self.inner.end().map_err(CommandStringDecodeError::UnexpectedEof)?;
+ // Trim null padding from the end.
+ let trimmed =
+ rawbytes.iter().rposition(|&b| b != 0).map_or(&rawbytes[..0], |i| &rawbytes[..=i]);
+
+ if !trimmed.is_ascii() {
+ return Err(CommandStringDecodeError::NotAscii);
+ }
+
+ Ok(CommandString(Cow::Owned(unsafe { String::from_utf8_unchecked(trimmed.to_vec()) })))
+ }
+
+ fn read_limit(&self) -> usize { self.inner.read_limit() }
+}
+
+/// Error decoding a [`CommandString`].
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+pub enum CommandStringDecodeError {
+ /// Unexpected end of data.
+ UnexpectedEof(encoding::UnexpectedEofError),
+ /// Command string contains non-ASCII characters.
+ NotAscii,
+}
+
+impl fmt::Display for CommandStringDecodeError {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ match self {
+ Self::UnexpectedEof(e) => write!(f, "unexpected end of data: {}", e),
+ Self::NotAscii => write!(f, "command string must be ASCII"),
+ }
+ }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for CommandStringDecodeError {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+ match self {
+ Self::UnexpectedEof(e) => Some(e),
+ Self::NotAscii => None,
+ }
+ }
+}
+
/// Error returned when a command string is invalid.
///
/// This is currently returned for command strings longer than 12.
Why this scored 17/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.