Skip to content

Commit 3c29af7

Browse files
feat(proto)!: allow non-UTF-8 header values and align validation with the server
The NATS Server relays the header block of HMSG messages verbatim, without validating header names or values: values containing obs-text (0x80-0xFF), invalid UTF-8 or nothing at all pass through untouched, and nats.go is able to produce all of them. Watermelon instead decoded values into a UTF-8 `ByteString`, failing the decode of messages other clients handle fine, and the various validators enforced arbitrary length caps and Unicode whitespace rules the server knows nothing about. Store `HeaderValue` as raw `Bytes`, following the design of the `http` crate: `as_str` and the `str` conversion traits are replaced by a fallible `to_str` plus `as_bytes`, and the decoder now accepts non-UTF-8 and empty values, the latter previously panicking the header line parser. Validators now reject only what the wire cannot carry, dropping the arbitrary length limits: subjects and queue groups travel inside whitespace delimited control lines, so ` `, `\t`, `\r` and `\n` remain rejected; header values only reject `\r` and `\n`; header names additionally reject `:` and non-ASCII bytes, which Go's `textproto.ReadMIMEHeader` errors on when nats.go parses received headers. JetStream publishes now validate user-supplied `Nats-*` header values via the new `JetstreamError::HeaderValue` variant instead of trusting them, and the decoder's maximum head length is raised to 128 KiB: routed, leafnode and gateway connections may produce arg lines of up to 64 KiB, well past the previous 16 KiB limit.
1 parent 56c9ba9 commit 3c29af7

10 files changed

Lines changed: 348 additions & 183 deletions

File tree

watermelon-proto/src/headers/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ mod value;
88

99
pub mod error {
1010
pub use super::name::HeaderNameValidateError;
11-
pub use super::value::HeaderValueValidateError;
11+
pub use super::value::{HeaderValueToStrError, HeaderValueValidateError};
1212
}

watermelon-proto/src/headers/name.rs

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,13 @@ use bytestring::ByteString;
1414
/// contain a valid header name that meets the following requirements:
1515
///
1616
/// * The value is not empty
17-
/// * The value has a length less than or equal to 64 [^2]
18-
/// * The value does not contain any whitespace characters or `:`
17+
/// * The value does not contain bytes ≥ 0x80, `\r`, `\n`, or `:`
1918
///
2019
/// `HeaderName` can be constructed from [`HeaderName::from_static`]
2120
/// or any of the `TryFrom` implementations.
2221
///
2322
/// [^1]: Because [`HeaderName::from_dangerous_value`] is safe to call,
2423
/// unsafe code must not assume any of the above invariants.
25-
/// [^2]: Messages coming from the NATS server are allowed to violate this rule.
2624
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
2725
pub struct HeaderName(UniCase<ByteString>);
2826

@@ -170,11 +168,8 @@ pub enum HeaderNameValidateError {
170168
/// The value is empty
171169
#[error("HeaderName is empty")]
172170
Empty,
173-
/// The value has a length greater than 64
174-
#[error("HeaderName is too long")]
175-
TooLong,
176-
/// The value contains an Unicode whitespace character or `:`
177-
#[error("HeaderName contained an illegal whitespace character")]
171+
/// The value contains an illegal character
172+
#[error("HeaderName contained an illegal character")]
178173
IllegalCharacter,
179174
}
180175

@@ -183,15 +178,18 @@ fn validate_header_name(header_name: &str) -> Result<(), HeaderNameValidateError
183178
return Err(HeaderNameValidateError::Empty);
184179
}
185180

186-
if header_name.len() > 64 {
187-
// This is an arbitrary limit, but I guess the server must also have one
188-
return Err(HeaderNameValidateError::TooLong);
189-
}
190-
191-
if header_name.chars().any(|c| c.is_whitespace() || c == ':') {
192-
// The theoretical security limit is just ` `, `\t`, `\r`, `\n` and `:`.
193-
// Let's be more careful.
194-
return Err(HeaderNameValidateError::IllegalCharacter);
181+
for b in header_name.bytes() {
182+
match b {
183+
// The NATS server relays header names verbatim without validating
184+
// them, but Go's `textproto.ReadMIMEHeader` (used by `nats.go` to
185+
// parse received headers) errors out on bytes >= 0x80. Rejecting
186+
// them also keeps `HeaderName` ASCII, making the case-insensitive
187+
// comparison well defined.
188+
b if b >= 0x80 => return Err(HeaderNameValidateError::IllegalCharacter),
189+
// Wire protocol constraints: line terminators and name/value separator
190+
b'\r' | b'\n' | b':' => return Err(HeaderNameValidateError::IllegalCharacter),
191+
_ => {}
192+
}
195193
}
196194

197195
Ok(())
@@ -200,8 +198,11 @@ fn validate_header_name(header_name: &str) -> Result<(), HeaderNameValidateError
200198
#[cfg(test)]
201199
mod tests {
202200
use core::cmp::Ordering;
201+
use core::str::FromStr;
203202

204-
use super::HeaderName;
203+
use claims::assert_matches;
204+
205+
use super::{HeaderName, HeaderNameValidateError};
205206

206207
#[test]
207208
fn eq() {
@@ -210,4 +211,30 @@ mod tests {
210211
assert_eq!(cased, lowercase);
211212
assert_eq!(cased.cmp(&lowercase), Ordering::Equal);
212213
}
214+
215+
#[test]
216+
fn valid_header_names() {
217+
let names = ["Nats-Msg-Id", "a", "X-Custom-Header", "Header Name"];
218+
for name in names {
219+
let header_name = HeaderName::from_str(name).unwrap();
220+
assert_eq!(header_name.as_str(), name);
221+
}
222+
}
223+
224+
#[test]
225+
fn invalid_header_names() {
226+
assert_matches!(
227+
HeaderName::from_str(""),
228+
Err(HeaderNameValidateError::Empty)
229+
);
230+
231+
let names = [":", "name:", "na:me", "name\r", "na\nme", "nàme"];
232+
for name in names {
233+
assert_matches!(
234+
HeaderName::from_str(name),
235+
Err(HeaderNameValidateError::IllegalCharacter),
236+
"{name:?}"
237+
);
238+
}
239+
}
213240
}

0 commit comments

Comments
 (0)