Minor refactoring of pxars xattr encoder/decoder, mostly reformatting and renaming.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>

src/pxar/encoder.rs

@@ -230,25 +230,29 @@ impl <'a, W: Write> Encoder<'a, W> {
         (self.feature_flags & feature_flags) == feature_flags
     }
 
-    fn read_xattrs(&self, fd: RawFd, stat: &FileStat, entry: &CaFormatEntry) -> Result<(Vec<CaFormatXAttr>, Option<CaFormatFCaps>), Error> {
+    fn read_xattrs(&self, fd: RawFd, stat: &FileStat) -> Result<(Vec<CaFormatXAttr>, Option<CaFormatFCaps>), Error> {
         let mut xattrs = Vec::new();
         let mut fcaps = None;
 
         let flags = CA_FORMAT_WITH_XATTRS | CA_FORMAT_WITH_FCAPS;
-        if !self.has_features(flags) { return Ok((xattrs, fcaps)); }
+        if !self.has_features(flags) {
+            return Ok((xattrs, fcaps));
+        }
         // Should never be called on symlinks, just in case check anyway
-        if (stat.st_mode & libc::S_IFMT) == libc::S_IFLNK { return Ok((xattrs, fcaps)); }
+        if (stat.st_mode & libc::S_IFMT) == libc::S_IFLNK {
+            return Ok((xattrs, fcaps));
+        }
 
         let xattr_names = match xattr::flistxattr(fd) {
             Ok(names) => names,
-            Err(Errno::EOPNOTSUPP) => return Ok((xattrs, fcaps)),
-            Err(Errno::EBADF) => return Ok((xattrs, fcaps)),
             Err(err) => bail!("read_xattrs failed for {:?} - {}", self.full_path(), err),
         };
 
-        for name in xattr_names.split(|c| *c == '\0' as u8) {
+        for name in xattr_names.split(|c| *c == b'\0') {
             // Only extract the relevant extended attributes
-            if !xattr::name_store(&name) { continue; }
+            if !xattr::is_valid_xattr_name(&name) {
+                continue;
+            }
 
             let value = match xattr::fgetxattr(fd, name) {
                 Ok(value) => value,
@@ -257,7 +261,7 @@ impl <'a, W: Write> Encoder<'a, W> {
                 Err(err) => bail!("read_xattrs failed for {:?} - {}", self.full_path(), err),
             };
 
-            if xattr::security_capability(&name) {
+            if xattr::is_security_capability(&name) {
                 // fcaps are stored in own format within the archive
                 fcaps = Some(CaFormatFCaps {
                     data: value,
@@ -355,10 +359,12 @@ impl <'a, W: Write> Encoder<'a, W> {
 
         self.read_chattr(rawfd, &mut dir_entry)?;
         self.read_fat_attr(rawfd, magic, &mut dir_entry)?;
-        let (xattrs, fcaps) = self.read_xattrs(rawfd, &dir_stat, &dir_entry)?;
+        let (xattrs, fcaps) = self.read_xattrs(rawfd, &dir_stat)?;
 
         self.write_entry(dir_entry)?;
-        for xattr in xattrs { self.write_xattr(xattr)?; }
+        for xattr in xattrs {
+            self.write_xattr(xattr)?;
+        }
         self.write_fcaps(fcaps)?;
 
         let mut dir_count = 0;
@@ -545,10 +551,12 @@ impl <'a, W: Write> Encoder<'a, W> {
 
         self.read_chattr(filefd, &mut entry)?;
         self.read_fat_attr(filefd, magic, &mut entry)?;
-        let (xattrs, fcaps) = self.read_xattrs(filefd, &stat, &entry)?;
+        let (xattrs, fcaps) = self.read_xattrs(filefd, &stat)?;
 
         self.write_entry(entry)?;
-        for xattr in xattrs { self.write_xattr(xattr)?; }
+        for xattr in xattrs {
+            self.write_xattr(xattr)?;
+        }
         self.write_fcaps(fcaps)?;
 
         let include_payload;

src/pxar/sequential_decoder.rs

@@ -150,25 +150,20 @@ impl <'a, R: Read> SequentialDecoder<'a, R> {
     fn read_xattr(&mut self, size: usize) -> Result<CaFormatXAttr, Error> {
         let buffer = self.reader.read_exact_allocated(size)?;
 
-        match buffer.iter().position(|c| *c == '\0' as u8) {
+        let separator = buffer.iter().position(|c| *c == b'\0')
-            Some(pos) => {
+            .ok_or_else(|| format_err!("no value found in xattr"))?;
-                // pos needs to be within the first 256 bytes in order to
+
-                // terminate a valid xattr name
+        let (name, value) = buffer.split_at(separator);
-                if pos > 255 {
+        if !xattr::is_valid_xattr_name(name) ||
-                    bail!("Invalid zero termination for xattr name.");
+            xattr::is_security_capability(name)
-                }
+        {
-                if !xattr::name_store(&buffer[0..pos]) || xattr::security_capability(&buffer[0..pos]) {
+            bail!("incorrect xattr name - {}.", String::from_utf8_lossy(name));
-                    bail!("Invalid name for xattr - {}.", String::from_utf8_lossy(&buffer[0..pos]));
-                }
-                let name = buffer[0..pos].to_vec();
-                let value = buffer[pos + 1..size].to_vec();
-                return Ok(CaFormatXAttr {
-                    name: name,
-                    value: value,
-                });
-            },
-            _ => bail!("Incorrect zero termination in xattr."),
         }
+
+        Ok(CaFormatXAttr {
+            name: name.to_vec(),
+            value: value[1..].to_vec(),
+        })
     }
 
     fn read_fcaps(&mut self, size: usize) -> Result<CaFormatFCaps, Error> {

src/tools/xattr.rs

@@ -92,17 +92,20 @@ pub fn fsetxattr_fcaps(fd: RawFd, fcaps: CaFormatFCaps) -> Result<(), nix::errno
     Ok(())
 }
 
-pub fn security_capability(name: &[u8]) -> bool {
+pub fn is_security_capability(name: &[u8]) -> bool {
     name == b"security.capability"
 }
 
-pub fn name_store(name: &[u8]) -> bool {
+/// Check if the passed name buffer starts with a valid xattr namespace prefix
-    if name.is_empty() { return false; }
+/// and is within the length limit of 255 bytes
-    if name.starts_with(b"user.") { return true; }
+pub fn is_valid_xattr_name(name: &[u8]) -> bool {
-    if name.starts_with(b"trusted.") { return true; }
+    if name.is_empty() || name.len() > 255 {
-    if security_capability(name) { return true; }
+        return false;
-
+    }
-    false
+    if name.starts_with(b"user.") || name.starts_with(b"trusted.") {
+        return true;
+    }
+    is_security_capability(name)
 }
 
 #[cfg(test)]