diff --git a/engine/packages/universaldb/src/versionstamp.rs b/engine/packages/universaldb/src/versionstamp.rs index 92773d9087..1b84f8a2b4 100644 --- a/engine/packages/universaldb/src/versionstamp.rs +++ b/engine/packages/universaldb/src/versionstamp.rs @@ -62,13 +62,14 @@ pub fn substitute_versionstamp( versionstamp: Versionstamp, ) -> Result<(), String> { const VERSIONSTAMP_MARKER: u8 = 0x33; - const VERSIONSTAMP_SIZE: usize = 12; + const VERSIONSTAMP_SIZE: usize = 10; if packed_data.len() < 4 { return Err("Packed data too short to contain versionstamp offset".to_string()); } - let offset_bytes = packed_data.split_off(packed_data.len() - 4); + let data_len = packed_data.len() - 4; + let offset_bytes = &packed_data[data_len..]; let offset = u32::from_le_bytes([ offset_bytes[0], offset_bytes[1], @@ -76,11 +77,11 @@ pub fn substitute_versionstamp( offset_bytes[3], ]) as usize; - if offset >= packed_data.len() { + if offset >= data_len { return Err(format!( "Invalid versionstamp offset: {} exceeds data length {}", offset, - packed_data.len() + data_len )); } @@ -99,18 +100,19 @@ pub fn substitute_versionstamp( let versionstamp_end = versionstamp_start + VERSIONSTAMP_SIZE; - if versionstamp_end > packed_data.len() { + if versionstamp_end > data_len { return Err("Versionstamp extends beyond data bounds".to_string()); } let existing_bytes = &packed_data[versionstamp_start..versionstamp_end]; if existing_bytes[0..10] != [0xff; 10] { - // Versionstamp is already complete, nothing to do + packed_data.truncate(data_len); return Ok(()); } let versionstamp_bytes = versionstamp.as_bytes(); - packed_data[versionstamp_start..versionstamp_end].copy_from_slice(versionstamp_bytes); + packed_data[versionstamp_start..versionstamp_end].copy_from_slice(&versionstamp_bytes[..10]); + packed_data.truncate(data_len); Ok(()) } @@ -123,7 +125,8 @@ pub fn substitute_raw_versionstamp( return Err("Packed data too short to contain versionstamp offset".to_string()); } - let offset_bytes = data.split_off(data.len() - 4); + let data_len = data.len() - 4; + let offset_bytes = &data[data_len..]; let offset = u32::from_le_bytes([ offset_bytes[0], offset_bytes[1], @@ -135,15 +138,16 @@ pub fn substitute_raw_versionstamp( .checked_add(versionstamp_len) .ok_or_else(|| "Versionstamp offset overflowed".to_string())?; - if versionstamp_end > data.len() { + if versionstamp_end > data_len { return Err(format!( "Invalid versionstamp offset: {} exceeds data length {}", offset, - data.len() + data_len )); } data[offset..versionstamp_end].copy_from_slice(&versionstamp.as_bytes()[..versionstamp_len]); + data.truncate(data_len); Ok(data) } diff --git a/engine/packages/universaldb/tests/versionstamp.rs b/engine/packages/universaldb/tests/versionstamp.rs index 106560c364..dbc22403fd 100644 --- a/engine/packages/universaldb/tests/versionstamp.rs +++ b/engine/packages/universaldb/tests/versionstamp.rs @@ -18,7 +18,7 @@ fn test_generate_versionstamp() { #[test] fn test_substitute_versionstamp_success() { - let incomplete = Versionstamp::from([0xff; 12]); + let incomplete = Versionstamp::incomplete(100); let tuple = vec![ Element::String("mykey".into()), Element::Versionstamp(incomplete), @@ -99,9 +99,83 @@ fn test_substitute_versionstamp_already_complete() { assert!(result.is_ok()); } +#[test] +fn test_substitute_raw_versionstamp_trims_explicit_operand_offset() { + let versionstamp = generate_versionstamp(100); + let mut param = b"prefix".to_vec(); + let offset = param.len() as u32; + param.extend_from_slice(&[0xff; 10]); + param.extend_from_slice(b"suffix"); + param.extend_from_slice(&offset.to_le_bytes()); + + let substituted = substitute_raw_versionstamp(param.clone(), &versionstamp).unwrap(); + + assert_eq!(substituted.len(), param.len() - 4); + assert_eq!(&substituted[..offset as usize], b"prefix"); + assert_eq!( + &substituted[offset as usize..offset as usize + 10], + &versionstamp.as_bytes()[..10] + ); + assert_eq!(&substituted[offset as usize + 10..], b"suffix"); +} + +#[test] +fn test_substitute_raw_versionstamp_matches_fdb_metadata_value_operand() { + let versionstamp = generate_versionstamp(100); + let mut param = vec![0; 14]; + param[10..].copy_from_slice(&0u32.to_le_bytes()); + + let substituted = substitute_raw_versionstamp(param, &versionstamp).unwrap(); + + assert_eq!(substituted, versionstamp.as_bytes()[..10]); +} + +#[test] +fn test_substitute_raw_versionstamp_preserves_depot_suffix_bytes() { + let versionstamp = generate_versionstamp(100); + let mut param = vec![0xff; 10]; + param.extend_from_slice(&[0; 6]); + param.extend_from_slice(&0u32.to_le_bytes()); + + let substituted = substitute_raw_versionstamp(param, &versionstamp).unwrap(); + + assert_eq!(substituted.len(), 16); + assert_eq!(&substituted[..10], &versionstamp.as_bytes()[..10]); + assert_eq!(&substituted[10..], &[0; 6]); +} + +#[test] +fn test_substitute_versionstamp_matches_official_tuple_operand_layout() { + let tuple = ( + "prefix", + Versionstamp::incomplete(12345), + "suffix", + ); + let mut ours = pack_with_versionstamp(&tuple); + let official = ours.clone(); + let versionstamp = generate_versionstamp(54321); + + substitute_versionstamp(&mut ours, versionstamp.clone()).unwrap(); + + let offset_start = official.len() - 4; + let offset = u32::from_le_bytes( + official[offset_start..] + .try_into() + .expect("official tuple offset should be four bytes"), + ) as usize; + let mut expected = official[..offset_start].to_vec(); + expected[offset..offset + 10].copy_from_slice(&versionstamp.as_bytes()[..10]); + + assert_eq!(ours, expected); + + let unpacked: (String, Versionstamp, String) = unpack(&ours).unwrap(); + assert!(unpacked.1.is_complete()); + assert_eq!(unpacked.1.user_version(), 12345); +} + #[test] fn test_pack_and_substitute_versionstamp() { - let incomplete = Versionstamp::from([0xff; 12]); + let incomplete = Versionstamp::incomplete(100); let tuple = vec![ Element::String("mykey".into()), Element::Versionstamp(incomplete), @@ -155,8 +229,6 @@ fn test_versionstamp_incomplete_preserves_user_version() { "User version bytes should match" ); - // Test with substitute_versionstamp - the user version from the incomplete versionstamp - // should be ignored and replaced with the one from the generated versionstamp let tuple = vec![ Element::String("test".into()), Element::Versionstamp(incomplete), @@ -172,11 +244,10 @@ fn test_versionstamp_incomplete_preserves_user_version() { match &unpacked[1] { Element::Versionstamp(v) => { assert!(v.is_complete()); - // The user version should be from the generated versionstamp, not the original incomplete one assert_eq!( v.user_version(), - new_user_version, - "Substituted versionstamp should have the new user version" + user_version, + "Substituted versionstamp should preserve the tuple user version" ); } _ => panic!("Expected versionstamp"),