Skip to content

Commit fab1a6c

Browse files
committed
Simplify and harden some libyaml consistency checks
1 parent 933708b commit fab1a6c

File tree

1 file changed

+6
-8
lines changed

1 file changed

+6
-8
lines changed

src/yaml/chunker/parser.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,24 +126,22 @@ where
126126
const READ_SUCCESS: i32 = 1;
127127
const READ_FAILURE: i32 = 0;
128128

129-
// Let's knock out this particular class of UB across the board.
129+
// These should never fail, but let's be extra safe. Ideally we would
130+
// panic in cases as degenerate as this, but I want to model a scenario
131+
// where we're using the true libyaml through FFI and can't unwind.
130132
if read_state.is_null() || buffer.is_null() || size_read.is_null() {
131133
return READ_FAILURE;
132134
}
135+
let Ok(buffer_size) = usize::try_from(buffer_size) else {
136+
return READ_FAILURE;
137+
};
133138

134139
// SAFETY: self.read_state_mut() is the only other dereference of the
135140
// read_state; see its comment explaining how it's mutually exclusive
136141
// with running the parser. The lifetime here lasts through the end of
137142
// this function, i.e. before the parser is finished.
138143
let read_state = unsafe { &mut *read_state.cast::<ReadState<R>>() };
139144

140-
// We assume libyaml is implemented correctly, and won't pass a buffer
141-
// size larger than can exist in memory. (Since unsafe_libyaml is
142-
// actually Rust we could use usize::try_from and unwrap, but panicking
143-
// wouldn't be okay in the true FFI scenario I want to model.)
144-
#[allow(clippy::cast_possible_truncation)]
145-
let buffer_size = buffer_size as usize;
146-
147145
// libyaml is not guaranteed to initialize its buffer prior to the first
148146
// read. It would be instant Undefined Behavior to slice that buffer,
149147
// and unsound to expose it to a safe Read impl, so we need to bounce

0 commit comments

Comments
 (0)