From 0e0829c887728edfc2ac42d50a4a7e701699e760 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Wed, 13 Sep 2023 13:34:04 -0700 Subject: [PATCH] Move out-slice len adjustment from symbol_copy_to_slice to its caller. 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 0bc40291ae38f63a7e8c5521297e42ce77fd6c85 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). --- soroban-env-common/src/symbol.rs | 14 +++++++++++--- soroban-env-host/src/host.rs | 3 +-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/soroban-env-common/src/symbol.rs b/soroban-env-common/src/symbol.rs index 07ab6d418..cb2d9e3a5 100644 --- a/soroban-env-common/src/symbol.rs +++ b/soroban-env-common/src/symbol.rs @@ -319,9 +319,17 @@ impl TryFromVal 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, + )) + } } } } diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index f6a9dafa8..cf185fcb7 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -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::(s, b_pos, &mut slice[..len]) + self.memobj_copy_to_slice::(s, b_pos, slice) } fn bytes_new_from_slice(&self, mem: &[u8]) -> Result {