Prevent writing empty annotations sequences (#781)

* Prevent writing empty annotations sequences * Adds tests for explicit empty annotations seq for text --------- Co-authored-by: Zack Slayton <[email protected]>
amazon-ion · Jun 3, 2024 · 1c22df0 · 1c22df0
1 parent 80088b4
commit 1c22df0
Show file tree

Hide file tree

Showing 9 changed files with 127 additions and 9 deletions.
diff --git a/src/lazy/any_encoding.rs b/src/lazy/any_encoding.rs
@@ -724,6 +724,16 @@ impl<'top> LazyRawValue<'top, AnyEncoding> for LazyRawAnyValue<'top> {
         }
     }
 
+    fn has_annotations(&self) -> bool {
+        use LazyRawValueKind::*;
+        match &self.encoding {
+            Text_1_0(v) => v.has_annotations(),
+            Binary_1_0(v) => v.has_annotations(),
+            Text_1_1(v) => v.has_annotations(),
+            Binary_1_1(v) => v.has_annotations(),
+        }
+    }
+
     fn annotations(&self) -> RawAnyAnnotationsIterator<'top> {
         use LazyRawValueKind::*;
         match &self.encoding {

diff --git a/src/lazy/binary/raw/v1_1/value.rs b/src/lazy/binary/raw/v1_1/value.rs
@@ -110,6 +110,10 @@ impl<'top> LazyRawValue<'top, BinaryEncoding_1_1> for LazyRawBinaryValue_1_1<'to
         self.is_null()
     }
 
+    fn has_annotations(&self) -> bool {
+        self.encoded_value.has_annotations()
+    }
+
     fn annotations(&self) -> <BinaryEncoding_1_1 as Decoder>::AnnotationsIterator<'top> {
         self.annotations()
     }

diff --git a/src/lazy/binary/raw/value.rs b/src/lazy/binary/raw/value.rs
@@ -108,6 +108,10 @@ impl<'top> LazyRawValue<'top, BinaryEncoding_1_0> for LazyRawBinaryValue_1_0<'to
         self.is_null()
     }
 
+    fn has_annotations(&self) -> bool {
+        self.has_annotations()
+    }
+
     fn annotations(&self) -> RawBinaryAnnotationsIterator<'top> {
         self.annotations()
     }

diff --git a/src/lazy/decoder.rs b/src/lazy/decoder.rs
@@ -374,6 +374,7 @@ pub trait LazyRawValue<'top, D: Decoder>:
 {
     fn ion_type(&self) -> IonType;
     fn is_null(&self) -> bool;
+    fn has_annotations(&self) -> bool;
     fn annotations(&self) -> D::AnnotationsIterator<'top>;
     fn read(&self) -> IonResult<RawValueRef<'top, D>>;
 

diff --git a/src/lazy/encoder/binary/v1_0/value_writer.rs b/src/lazy/encoder/binary/v1_0/value_writer.rs
@@ -337,6 +337,11 @@ macro_rules! annotate_and_delegate_1_0 {
 
 impl<'value, 'top> BinaryAnnotatedValueWriter_1_0<'value, 'top> {
     pub(crate) fn annotate_encoded_value(&mut self, encoded_value: &[u8]) -> IonResult<()> {
+        if self.annotations.is_empty() {
+            self.output_buffer.extend_from_slice(encoded_value);
+            return Ok(());
+        }
+
         let mut encoded_annotations_sequence = BumpVec::new_in(self.allocator);
         self.encode_annotations_sequence(&mut encoded_annotations_sequence)?;
 
@@ -444,11 +449,11 @@ impl<'value, 'top> ValueWriter for BinaryAnnotatedValueWriter_1_0<'value, 'top>
 mod tests {
     use crate::lazy::encoder::annotate::Annotatable;
     use crate::lazy::encoder::binary::v1_0::writer::LazyRawBinaryWriter_1_0;
-    use crate::lazy::encoder::value_writer::SequenceWriter;
     use crate::lazy::encoder::value_writer::StructWriter;
+    use crate::lazy::encoder::value_writer::{AnnotatableWriter, SequenceWriter};
     use crate::lazy::encoder::write_as_ion::WriteAsSExp;
     use crate::raw_symbol_ref::AsRawSymbolRef;
-    use crate::{Element, IonData, IonResult, RawSymbolRef, Timestamp};
+    use crate::{Element, IonData, IonResult, RawSymbolRef, SymbolId, Timestamp, ValueWriter};
 
     fn writer_test(
         expected: &str,
@@ -598,6 +603,23 @@ mod tests {
         })
     }
 
+    #[test]
+    fn write_annotated_without_annotations() -> IonResult<()> {
+        // This test explicitly adds an empty annotations sequence to values and value writers
+        // to make sure they do not emit an annotations wrapper without annotations.
+        let expected = "1 name 2024T";
+        const EMPTY_ANNOTATIONS: [SymbolId; 0] = [];
+        writer_test(expected, |writer| {
+            writer.write(1.annotated_with(EMPTY_ANNOTATIONS))?;
+            writer.write(RawSymbolRef::SymbolId(4).annotated_with(EMPTY_ANNOTATIONS))?;
+            writer
+                .value_writer()
+                .with_annotations(EMPTY_ANNOTATIONS)?
+                .write(Timestamp::with_year(2024).build()?)?;
+            Ok(())
+        })
+    }
+
     #[test]
     fn write_annotated_scalars() -> IonResult<()> {
         let expected = r#"

diff --git a/src/lazy/encoder/text/v1_0/writer.rs b/src/lazy/encoder/text/v1_0/writer.rs
@@ -114,3 +114,40 @@ impl<W: Write> LazyRawWriter<W> for LazyRawTextWriter_1_0<W> {
         &mut self.output
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::lazy::encoder::text::v1_0::writer::LazyRawTextWriter_1_0;
+    use crate::{v1_1, Annotatable, ElementReader, IonData, IonResult, Reader, SequenceWriter};
+
+    #[test]
+    fn write_annotated_values() -> IonResult<()> {
+        const NO_ANNOTATIONS: [&str; 0] = [];
+        let mut writer = LazyRawTextWriter_1_0::new(vec![])?;
+        writer
+            .write(1)?
+            // Explicitly setting an empty annotations sequence
+            .write(2.annotated_with(NO_ANNOTATIONS))?
+            .write(3.annotated_with("foo"))?
+            .write(4.annotated_with(["foo", "bar", "baz"]))?;
+        let encoded_bytes = writer.close()?;
+        let encoded_text = String::from_utf8(encoded_bytes).unwrap();
+        println!("{encoded_text}");
+
+        let expected_ion = r#"
+            1
+            2 // An explicitly empty annotations sequence results in an unannotated value
+            foo::3
+            foo::bar::baz::4
+        "#;
+
+        let mut reader = Reader::new(v1_1::Text, encoded_text)?;
+        let actual = reader.read_all_elements()?;
+
+        let mut reader = Reader::new(v1_1::Text, expected_ion)?;
+        let expected = reader.read_all_elements()?;
+
+        assert!(IonData::eq(&expected, &actual));
+        Ok(())
+    }
+}
diff --git a/src/lazy/encoder/text/v1_1/writer.rs b/src/lazy/encoder/text/v1_1/writer.rs
@@ -103,8 +103,8 @@ mod tests {
     use crate::lazy::text::raw::v1_1::reader::{LazyRawTextReader_1_1, MacroIdRef};
     use crate::symbol_ref::AsSymbolRef;
     use crate::{
-        v1_1, Decimal, ElementReader, IonData, IonResult, IonType, Null, RawSymbolRef, Reader,
-        Timestamp,
+        v1_1, Annotatable, Decimal, ElementReader, IonData, IonResult, IonType, Null, RawSymbolRef,
+        Reader, Timestamp,
     };
 
     #[test]
@@ -315,4 +315,36 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn write_annotated_values() -> IonResult<()> {
+        const NO_ANNOTATIONS: [&str; 0] = [];
+        let mut writer = LazyRawTextWriter_1_1::new(vec![])?;
+        writer
+            .write(1)?
+            // Explicitly setting an empty annotations sequence
+            .write(2.annotated_with(NO_ANNOTATIONS))?
+            .write(3.annotated_with("foo"))?
+            .write(4.annotated_with(["foo", "bar", "baz"]))?;
+        let encoded_bytes = writer.close()?;
+        let encoded_text = String::from_utf8(encoded_bytes).unwrap();
+        println!("{encoded_text}");
+
+        let expected_ion = r#"
+            $ion_1_1
+            1
+            2 // An explicitly empty annotations sequence results in an unannotated value
+            foo::3
+            foo::bar::baz::4
+        "#;
+
+        let mut reader = Reader::new(v1_1::Text, encoded_text)?;
+        let actual = reader.read_all_elements()?;
+
+        let mut reader = Reader::new(v1_1::Text, expected_ion)?;
+        let expected = reader.read_all_elements()?;
+
+        assert!(IonData::eq(&expected, &actual));
+        Ok(())
+    }
 }
diff --git a/src/lazy/encoder/write_as_ion.rs b/src/lazy/encoder/write_as_ion.rs
@@ -273,12 +273,16 @@ impl WriteAsIon for Value {
 
 impl<'a, D: Decoder> WriteAsIon for LazyValue<'a, D> {
     fn write_as_ion<V: ValueWriter>(&self, writer: V) -> IonResult<()> {
-        let mut annotations = AnnotationsVec::new();
-        for annotation in self.annotations() {
-            annotations.push(annotation?.into());
+        if self.has_annotations() {
+            let mut annotations = AnnotationsVec::new();
+            for annotation in self.annotations() {
+                annotations.push(annotation?.into());
+            }
+            self.read()?
+                .write_as_ion(writer.with_annotations(annotations)?)
+        } else {
+            self.read()?.write_as_ion(writer)
         }
-        self.read()?
-            .write_as_ion(writer.with_annotations(annotations)?)
     }
 }
 

diff --git a/src/lazy/text/value.rs b/src/lazy/text/value.rs
@@ -181,6 +181,10 @@ impl<'top, E: TextEncoding<'top>> LazyRawValue<'top, E> for LazyRawTextValue<'to
         self.encoded_value.is_null()
     }
 
+    fn has_annotations(&self) -> bool {
+        self.has_annotations() // Inherent impl
+    }
+
     fn annotations(&self) -> <E as Decoder>::AnnotationsIterator<'top> {
         let range = self
             .encoded_value