Skip to content

Commit

Permalink
Move out-slice len adjustment from symbol_copy_to_slice to its caller.
Browse files Browse the repository at this point in the history
In general this is not a correct adjustment to make in
symbol_copy_to_slice because that function takes an arbitrary input pos
in the symbol it's copying from (like all the buffer-copy host
functions). It is correct only in the case where pos == 0 and even then
it represents a bit of a footgun by papering over real errors in the
caller (passing too long a slice, which the caller might incorrectly
assume is entirely filled by the call if the call silently succeeded).

This adjustment was added in 0bc4029
presumably in order to "make tests pass". Removing it does break tests
but only because of one call path that uses this host function, where a
host SymbolObject is converted to a fixed-length guest SymbolStr. This is
a caller in which a zero pos is already being passed and the adjustment
is appropriate, so in this commit we change that call site to do the
adjustment itself (which causes all tests to pass again).
  • Loading branch information
graydon committed Sep 13, 2023
1 parent f19ef13 commit 409f263
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 5 deletions.
14 changes: 11 additions & 3 deletions soroban-env-common/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,17 @@ impl<E: Env> TryFromVal<E, Symbol> for SymbolStr {
} else {
let obj: SymbolObject = unsafe { SymbolObject::unchecked_from_val(v.0) };
let mut arr = [0u8; SCSYMBOL_LIMIT as usize];
env.symbol_copy_to_slice(obj, Val::U32_ZERO, &mut arr)
.map_err(Into::into)?;
Ok(SymbolStr(arr))
let len: u32 = env.symbol_len(obj).map_err(Into::into)?.into();
if let Some(slice) = arr.get_mut(..len as usize) {
env.symbol_copy_to_slice(obj, Val::U32_ZERO, slice)
.map_err(Into::into)?;
Ok(SymbolStr(arr))
} else {
Err(crate::Error::from_type_and_code(
crate::xdr::ScErrorType::Value,
crate::xdr::ScErrorCode::InternalError,
))
}
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions soroban-env-host/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1123,8 +1123,7 @@ impl EnvBase for Host {
b_pos: U32Val,
slice: &mut [u8],
) -> Result<(), HostError> {
let len = self.visit_obj(s, |sym: &ScSymbol| Ok(sym.len()))?;
self.memobj_copy_to_slice::<ScSymbol>(s, b_pos, &mut slice[..len])
self.memobj_copy_to_slice::<ScSymbol>(s, b_pos, slice)
}

fn bytes_new_from_slice(&self, mem: &[u8]) -> Result<BytesObject, HostError> {
Expand Down

0 comments on commit 409f263

Please sign in to comment.