Always get the first Data Run when creating/rewinding attribute readers.

Ensures that the `data_position` function returns a proper value even when an attribute reader has never been seeked. This is a breaking change with respect to the signature of NtfsAttribute::value, hence I'm also updating the minor version. Fixes #10 Thanks to @leofidus and @vthib for reporting and debugging!
ColinFinck · Mar 13, 2022 · 93ce1f4 · 93ce1f4
1 parent a74dc2b
commit 93ce1f4
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 39 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "ntfs"
-version = "0.1.0"
+version = "0.2.0"
 authors = ["Colin Finck <colin@reactos.org>"]
 description = "A low-level NTFS filesystem library"
 homepage = "https://github.com/ColinFinck/ntfs"

diff --git a/examples/ntfs-shell/main.rs b/examples/ntfs-shell/main.rs
@@ -1,4 +1,4 @@
-// Copyright 2021 Colin Finck <colin@reactos.org>
+// Copyright 2021-2022 Colin Finck <colin@reactos.org>
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 mod sector_reader;
@@ -120,6 +120,7 @@ where
         let ty = attribute.ty()?;
 
         attr_print_attribute(
+            info,
             with_runs,
             &attribute,
             file.file_record_number(),
@@ -143,6 +144,7 @@ where
                 let entry_attribute = entry.to_attribute(&entry_file)?;
 
                 attr_print_attribute(
+                    info,
                     with_runs,
                     &entry_attribute,
                     entry_record_number,
@@ -156,13 +158,17 @@ where
     Ok(())
 }
 
-fn attr_print_attribute<'n>(
+fn attr_print_attribute<'n, T>(
+    info: &mut CommandInfo<T>,
     with_runs: bool,
     attribute: &NtfsAttribute<'n, '_>,
     record_number: u64,
     attribute_prefix: &str,
     data_run_prefix: &str,
-) -> Result<()> {
+) -> Result<()>
+where
+    T: Read + Seek,
+{
     let instance = format!("{}{}", attribute_prefix, attribute.instance());
     let ty = attribute.ty()?;
     let resident = attribute.is_resident();
@@ -176,7 +182,9 @@ fn attr_print_attribute<'n>(
     );
 
     if with_runs {
-        if let NtfsAttributeValue::NonResident(non_resident_value) = attribute.value()? {
+        let value = attribute.value(&mut info.fs)?;
+
+        if let NtfsAttributeValue::NonResident(non_resident_value) = value {
             for (i, data_run) in non_resident_value.data_runs().enumerate() {
                 let data_run = data_run?;
                 let instance = format!("{}{}", data_run_prefix, i);
@@ -498,7 +506,7 @@ where
     };
     let data_item = data_item?;
     let data_attribute = data_item.to_attribute();
-    let mut data_value = data_attribute.value()?;
+    let mut data_value = data_attribute.value(&mut info.fs)?;
 
     println!(
         "Saving {} bytes of data in \"{}\"...",

diff --git a/src/attribute.rs b/src/attribute.rs
@@ -366,7 +366,8 @@ impl<'n, 'f> NtfsAttribute<'n, 'f> {
         S: NtfsStructuredValue<'n, 'f>,
     {
         self.ensure_ty(S::TY)?;
-        S::from_attribute_value(fs, self.value()?)
+        let value = self.value(fs)?;
+        S::from_attribute_value(fs, value)
     }
 
     /// Returns the type of this NTFS Attribute, or [`NtfsError::UnsupportedAttributeType`]
@@ -428,7 +429,10 @@ impl<'n, 'f> NtfsAttribute<'n, 'f> {
     }
 
     /// Returns an [`NtfsAttributeValue`] structure to read the value of this NTFS Attribute.
-    pub fn value(&self) -> Result<NtfsAttributeValue<'n, 'f>> {
+    pub fn value<T>(&self, fs: &mut T) -> Result<NtfsAttributeValue<'n, 'f>>
+    where
+        T: Read + Seek,
+    {
         if let Some(list_entries) = self.list_entries {
             // The first attribute reports the entire data size for all connected attributes
             // (remaining ones are set to zero).
@@ -437,11 +441,12 @@ impl<'n, 'f> NtfsAttribute<'n, 'f> {
 
             let value = NtfsAttributeListNonResidentAttributeValue::new(
                 self.file.ntfs(),
+                fs,
                 list_entries.clone(),
                 self.instance(),
                 self.ty()?,
                 data_size,
-            );
+            )?;
             Ok(NtfsAttributeValue::AttributeListNonResident(value))
         } else if self.is_resident() {
             let value = self.resident_value()?;
@@ -726,7 +731,7 @@ mod tests {
         let data_attribute = data_attribute_item.to_attribute();
         assert_eq!(data_attribute.value_length(), 0);
 
-        let mut data_attribute_value = data_attribute.value().unwrap();
+        let mut data_attribute_value = data_attribute.value(&mut testfs1).unwrap();
         assert!(data_attribute_value.is_empty());
 
         let mut buf = [0u8; 5];

diff --git a/src/attribute_value/attribute_list_non_resident.rs b/src/attribute_value/attribute_list_non_resident.rs
@@ -37,24 +37,32 @@ pub struct NtfsAttributeListNonResidentAttributeValue<'n, 'f> {
 }
 
 impl<'n, 'f> NtfsAttributeListNonResidentAttributeValue<'n, 'f> {
-    pub(crate) fn new(
+    pub(crate) fn new<T>(
         ntfs: &'n Ntfs,
+        fs: &mut T,
         attribute_list_entries: NtfsAttributeListEntries<'n, 'f>,
         instance: u16,
         ty: NtfsAttributeType,
         data_size: u64,
-    ) -> Self {
+    ) -> Result<Self>
+    where
+        T: Read + Seek,
+    {
         let connected_entries =
             AttributeListConnectedEntries::new(attribute_list_entries.clone(), instance, ty);
+        let stream_state = StreamState::new(data_size);
 
-        Self {
+        let mut value = Self {
             ntfs,
             initial_attribute_list_entries: attribute_list_entries,
             connected_entries,
             data_size,
             attribute_state: None,
-            stream_state: StreamState::new(data_size),
-        }
+            stream_state,
+        };
+        value.next_attribute(fs)?;
+
+        Ok(value)
     }
 
     /// Returns the absolute current data seek position within the filesystem, in bytes.
@@ -165,6 +173,19 @@ impl<'n, 'f> NtfsAttributeListNonResidentAttributeValue<'n, 'f> {
     pub fn ntfs(&self) -> &'n Ntfs {
         self.ntfs
     }
+
+    /// Rewinds this value reader to the very beginning.
+    fn rewind<T>(&mut self, fs: &mut T) -> Result<()>
+    where
+        T: Read + Seek,
+    {
+        self.connected_entries.attribute_list_entries =
+            Some(self.initial_attribute_list_entries.clone());
+        self.stream_state = StreamState::new(self.len());
+        self.next_attribute(fs)?;
+
+        Ok(())
+    }
 }
 
 impl<'n, 'f> NtfsReadSeek for NtfsAttributeListNonResidentAttributeValue<'n, 'f> {
@@ -208,11 +229,7 @@ impl<'n, 'f> NtfsReadSeek for NtfsAttributeListNonResidentAttributeValue<'n, 'f>
 
         let mut bytes_left_to_seek = match pos {
             SeekFrom::Start(n) => {
-                // Rewind to the very beginning.
-                self.connected_entries.attribute_list_entries =
-                    Some(self.initial_attribute_list_entries.clone());
-                self.attribute_state = None;
-                self.stream_state = StreamState::new(self.len());
+                self.rewind(fs)?;
                 n
             }
             SeekFrom::Current(n) if n >= 0 => n as u64,

diff --git a/src/attribute_value/mod.rs b/src/attribute_value/mod.rs
@@ -44,6 +44,7 @@ impl<'n, 'f> NtfsAttributeValue<'n, 'f> {
     /// Returns the absolute current data seek position within the filesystem, in bytes.
     /// This may be `None` if:
     ///   * The current seek position is outside the valid range, or
+    ///   * The attribute does not have a Data Run, or
     ///   * The current Data Run is a "sparse" Data Run.
     pub fn data_position(&self) -> Option<u64> {
         match self {
@@ -119,6 +120,7 @@ where
     /// Returns the absolute current data seek position within the filesystem, in bytes.
     /// This may be `None` if:
     ///   * The current seek position is outside the valid range, or
+    ///   * The attribute does not have a Data Run, or
     ///   * The current Data Run is a "sparse" Data Run.
     pub fn data_position(&self) -> Option<u64> {
         self.value.data_position()

diff --git a/src/attribute_value/non_resident.rs b/src/attribute_value/non_resident.rs
@@ -42,22 +42,19 @@ impl<'n, 'f> NtfsNonResidentAttributeValue<'n, 'f> {
         position: u64,
         data_size: u64,
     ) -> Result<Self> {
-        let mut stream_data_runs = NtfsDataRuns::new(ntfs, data, position);
-        let mut stream_state = StreamState::new(data_size);
+        let stream_data_runs = NtfsDataRuns::new(ntfs, data, position);
+        let stream_state = StreamState::new(data_size);
 
-        // Get the first Data Run already here to let `data_position` return something meaningful.
-        if let Some(stream_data_run) = stream_data_runs.next() {
-            let stream_data_run = stream_data_run?;
-            stream_state.set_stream_data_run(stream_data_run);
-        }
-
-        Ok(Self {
+        let mut value = Self {
             ntfs,
             data,
             position,
             stream_data_runs,
             stream_state,
-        })
+        };
+        value.next_data_run()?;
+
+        Ok(value)
     }
 
     /// Returns a variant of this reader that implements [`Read`] and [`Seek`]
@@ -75,6 +72,7 @@ impl<'n, 'f> NtfsNonResidentAttributeValue<'n, 'f> {
     /// Returns the absolute current data seek position within the filesystem, in bytes.
     /// This may be `None` if:
     ///   * The current seek position is outside the valid range, or
+    ///   * The attribute does not have a Data Run, or
     ///   * The current Data Run is a "sparse" Data Run
     pub fn data_position(&self) -> Option<u64> {
         self.stream_state.data_position()
@@ -112,9 +110,13 @@ impl<'n, 'f> NtfsNonResidentAttributeValue<'n, 'f> {
         self.ntfs
     }
 
-    /// Returns the absolute position of the Data Run information within the filesystem, in bytes.
-    pub fn position(&self) -> u64 {
-        self.position
+    /// Rewinds this value reader to the very beginning.
+    fn rewind(&mut self) -> Result<()> {
+        self.stream_data_runs = self.data_runs();
+        self.stream_state = StreamState::new(self.len());
+        self.next_data_run()?;
+
+        Ok(())
     }
 }
 
@@ -153,9 +155,7 @@ impl<'n, 'f> NtfsReadSeek for NtfsNonResidentAttributeValue<'n, 'f> {
 
         let mut bytes_left_to_seek = match pos {
             SeekFrom::Start(n) => {
-                // Rewind to the very beginning.
-                self.stream_data_runs = self.data_runs();
-                self.stream_state = StreamState::new(self.len());
+                self.rewind()?;
                 n
             }
             SeekFrom::Current(n) if n >= 0 => n as u64,
@@ -217,6 +217,7 @@ where
     /// Returns the absolute current data seek position within the filesystem, in bytes.
     /// This may be `None` if:
     ///   * The current seek position is outside the valid range, or
+    ///   * The attribute does not have a Data Run, or
     ///   * The current Data Run is a "sparse" Data Run.
     pub fn data_position(&self) -> Option<u64> {
         self.value.data_position()
@@ -530,6 +531,7 @@ impl StreamState {
     /// Returns the absolute current data seek position within the filesystem, in bytes.
     /// This may be `None` if:
     ///   * The current seek position is outside the valid range, or
+    ///   * The attribute does not have a Data Run, or
     ///   * The current Data Run is a "sparse" Data Run
     pub(crate) fn data_position(&self) -> Option<u64> {
         let stream_data_run = self.stream_data_run.as_ref()?;

diff --git a/src/ntfs.rs b/src/ntfs.rs
@@ -103,7 +103,7 @@ impl Ntfs {
                 position: self.mft_position,
                 ty: NtfsAttributeType::Data,
             })?;
-        let mut mft_data_value = mft_data_attribute.value()?;
+        let mut mft_data_value = mft_data_attribute.value(fs)?;
 
         mft_data_value.seek(fs, SeekFrom::Start(offset))?;
         let position = mft_data_value

diff --git a/src/upcase_table.rs b/src/upcase_table.rs
@@ -1,4 +1,4 @@
-// Copyright 2021 Colin Finck <colin@reactos.org>
+// Copyright 2021-2022 Colin Finck <colin@reactos.org>
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use crate::attribute::NtfsAttributeType;
@@ -52,7 +52,7 @@ impl UpcaseTable {
         }
 
         // Read the entire raw data from the $DATA attribute.
-        let mut data_value = data_attribute.value()?;
+        let mut data_value = data_attribute.value(fs)?;
         let mut data = vec![0u8; UPCASE_TABLE_SIZE as usize];
         data_value.read_exact(fs, &mut data)?;