public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup] fix #2998: encode mtime as i64 instead of u64
@ 2020-10-21 10:05 Dominik Csapak
  2020-10-21 10:39 ` Dominik Csapak
  0 siblings, 1 reply; 2+ messages in thread
From: Dominik Csapak @ 2020-10-21 10:05 UTC (permalink / raw)
  To: pbs-devel

saves files mtime as i64 instead of u64 which enables backup of
files with negative mtime

the catalog_decode_i64 is compatible to encoded u64 values
but not reverse, so all "old" catalogs can be read with the new
decoder, but catalogs that contain negative mtimes will decode wrongly
on older clients

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/backup/catalog.rs | 76 +++++++++++++++++++++++++++++++++++++++----
 src/pxar/catalog.rs   |  2 +-
 src/pxar/create.rs    |  2 +-
 3 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/src/backup/catalog.rs b/src/backup/catalog.rs
index d1f519e5..dd892a88 100644
--- a/src/backup/catalog.rs
+++ b/src/backup/catalog.rs
@@ -78,7 +78,7 @@ pub struct DirEntry {
 #[derive(Clone, Debug, PartialEq)]
 pub enum DirEntryAttribute {
     Directory { start: u64 },
-    File { size: u64, mtime: u64 },
+    File { size: u64, mtime: i64 },
     Symlink,
     Hardlink,
     BlockDevice,
@@ -89,7 +89,7 @@ pub enum DirEntryAttribute {
 
 impl DirEntry {
 
-    fn new(etype: CatalogEntryType, name: Vec<u8>, start: u64, size: u64, mtime:u64) -> Self {
+    fn new(etype: CatalogEntryType, name: Vec<u8>, start: u64, size: u64, mtime: i64) -> Self {
         match etype {
             CatalogEntryType::Directory => {
                 DirEntry { name, attr: DirEntryAttribute::Directory { start } }
@@ -184,7 +184,7 @@ impl DirInfo {
                 catalog_encode_u64(writer, name.len() as u64)?;
                 writer.write_all(name)?;
                 catalog_encode_u64(writer, *size)?;
-                catalog_encode_u64(writer, *mtime)?;
+                catalog_encode_i64(writer, *mtime)?;
             }
             DirEntry { name, attr: DirEntryAttribute::Symlink } => {
                 writer.write_all(&[CatalogEntryType::Symlink as u8])?;
@@ -234,7 +234,7 @@ impl DirInfo {
         Ok((self.name, data))
     }
 
-    fn parse<C: FnMut(CatalogEntryType, &[u8], u64, u64, u64) -> Result<bool, Error>>(
+    fn parse<C: FnMut(CatalogEntryType, &[u8], u64, u64, i64) -> Result<bool, Error>>(
         data: &[u8],
         mut callback: C,
     ) -> Result<(), Error> {
@@ -265,7 +265,7 @@ impl DirInfo {
                 }
                 CatalogEntryType::File => {
                     let size = catalog_decode_u64(&mut cursor)?;
-                    let mtime = catalog_decode_u64(&mut cursor)?;
+                    let mtime = catalog_decode_i64(&mut cursor)?;
                     callback(etype, name, 0, size, mtime)?
                 }
                 _ => {
@@ -362,7 +362,7 @@ impl <W: Write> BackupCatalogWriter for CatalogWriter<W> {
         Ok(())
     }
 
-    fn add_file(&mut self, name: &CStr, size: u64, mtime: u64) -> Result<(), Error> {
+    fn add_file(&mut self, name: &CStr, size: u64, mtime: i64) -> Result<(), Error> {
         let dir = self.dirstack.last_mut().ok_or_else(|| format_err!("outside root"))?;
         let name = name.to_bytes().to_vec();
         dir.entries.push(DirEntry { name, attr: DirEntryAttribute::File { size, mtime } });
@@ -587,6 +587,70 @@ impl <R: Read + Seek> CatalogReader<R> {
     }
 }
 
+/// Serialize i64 as short, variable length byte sequence
+///
+/// Stores 7 bits per byte, Bit 8 indicates the end of the sequence (when not set).
+/// We limit values to a maximum of 2^63.
+/// If the first byte is 128 (all zero but the end marker), the number is negative
+pub fn catalog_encode_i64<W: Write>(writer: &mut W, v: i64) -> Result<(), Error> {
+    let mut enc = Vec::new();
+
+    let mut d = if v < 0 {
+        (-1 * v) as u64
+    } else {
+        v as u64
+    };
+
+    loop {
+        if d < 128 {
+            if v < 0 {
+                enc.push(128 | d as u8);
+                enc.push(0u8);
+            } else {
+                enc.push(d as u8);
+            }
+            break;
+        }
+        enc.push((128 | (d & 127)) as u8);
+        d = d >> 7;
+    }
+    writer.write_all(&enc)?;
+
+    Ok(())
+}
+
+/// Deserialize i64 from variable length byte sequence
+///
+/// We currently read maximal 10 bytes, which give a maximum of 63 bits + sign.
+/// this method is compatible with catalog_encode_u64, meaning it can
+/// properly decode values that are encoded with catalog_encode_u64
+/// (which are <2^63)
+pub fn catalog_decode_i64<R: Read>(reader: &mut R) -> Result<i64, Error> {
+
+    let mut v: u64 = 0;
+    let mut buf = [0u8];
+
+    for i in 0..10 { // only allow 10 bytes (63 bits + sign marker)
+        if buf.is_empty() {
+            bail!("decode_i64 failed - unexpected EOB");
+        }
+        reader.read_exact(&mut buf)?;
+
+        let t = buf[0];
+
+        if t == 0 {
+            return Ok(v as i64 * -1);
+        } else if t < 128 {
+            v |= (t as u64) << (i*7);
+            return Ok(v as i64);
+        } else {
+            v |= ((t & 127) as u64) << (i*7);
+        }
+    }
+
+    bail!("decode_i64 failed - missing end marker");
+}
+
 /// Serialize u64 as short, variable length byte sequence
 ///
 /// Stores 7 bits per byte, Bit 8 indicates the end of the sequence (when not set).
diff --git a/src/pxar/catalog.rs b/src/pxar/catalog.rs
index 96b42427..02b57dfa 100644
--- a/src/pxar/catalog.rs
+++ b/src/pxar/catalog.rs
@@ -9,7 +9,7 @@ use std::ffi::CStr;
 pub trait BackupCatalogWriter {
     fn start_directory(&mut self, name: &CStr) -> Result<(), Error>;
     fn end_directory(&mut self) -> Result<(), Error>;
-    fn add_file(&mut self, name: &CStr, size: u64, mtime: u64) -> Result<(), Error>;
+    fn add_file(&mut self, name: &CStr, size: u64, mtime: i64) -> Result<(), Error>;
     fn add_symlink(&mut self, name: &CStr) -> Result<(), Error>;
     fn add_hardlink(&mut self, name: &CStr) -> Result<(), Error>;
     fn add_block_device(&mut self, name: &CStr) -> Result<(), Error>;
diff --git a/src/pxar/create.rs b/src/pxar/create.rs
index 74c49050..6ba6f15c 100644
--- a/src/pxar/create.rs
+++ b/src/pxar/create.rs
@@ -535,7 +535,7 @@ impl<'a, 'b> Archiver<'a, 'b> {
 
                 let file_size = stat.st_size as u64;
                 if let Some(ref mut catalog) = self.catalog {
-                    catalog.add_file(c_file_name, file_size, stat.st_mtime as u64)?;
+                    catalog.add_file(c_file_name, file_size, stat.st_mtime)?;
                 }
 
                 let offset: LinkOffset =
-- 
2.20.1





^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup] fix #2998: encode mtime as i64 instead of u64
  2020-10-21 10:05 [pbs-devel] [PATCH proxmox-backup] fix #2998: encode mtime as i64 instead of u64 Dominik Csapak
@ 2020-10-21 10:39 ` Dominik Csapak
  0 siblings, 0 replies; 2+ messages in thread
From: Dominik Csapak @ 2020-10-21 10:39 UTC (permalink / raw)
  To: pbs-devel

On 10/21/20 12:05 PM, Dominik Csapak wrote:
> saves files mtime as i64 instead of u64 which enables backup of
> files with negative mtime
> 
> the catalog_decode_i64 is compatible to encoded u64 values
> but not reverse, so all "old" catalogs can be read with the new
> decoder, but catalogs that contain negative mtimes will decode wrongly
> on older clients
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>   src/backup/catalog.rs | 76 +++++++++++++++++++++++++++++++++++++++----
>   src/pxar/catalog.rs   |  2 +-
>   src/pxar/create.rs    |  2 +-
>   3 files changed, 72 insertions(+), 8 deletions(-)
> 
> diff --git a/src/backup/catalog.rs b/src/backup/catalog.rs
> index d1f519e5..dd892a88 100644
> --- a/src/backup/catalog.rs
> +++ b/src/backup/catalog.rs
> @@ -78,7 +78,7 @@ pub struct DirEntry {
>   #[derive(Clone, Debug, PartialEq)]
>   pub enum DirEntryAttribute {
>       Directory { start: u64 },
> -    File { size: u64, mtime: u64 },
> +    File { size: u64, mtime: i64 },
>       Symlink,
>       Hardlink,
>       BlockDevice,
> @@ -89,7 +89,7 @@ pub enum DirEntryAttribute {
>   
>   impl DirEntry {
>   
> -    fn new(etype: CatalogEntryType, name: Vec<u8>, start: u64, size: u64, mtime:u64) -> Self {
> +    fn new(etype: CatalogEntryType, name: Vec<u8>, start: u64, size: u64, mtime: i64) -> Self {
>           match etype {
>               CatalogEntryType::Directory => {
>                   DirEntry { name, attr: DirEntryAttribute::Directory { start } }
> @@ -184,7 +184,7 @@ impl DirInfo {
>                   catalog_encode_u64(writer, name.len() as u64)?;
>                   writer.write_all(name)?;
>                   catalog_encode_u64(writer, *size)?;
> -                catalog_encode_u64(writer, *mtime)?;
> +                catalog_encode_i64(writer, *mtime)?;
>               }
>               DirEntry { name, attr: DirEntryAttribute::Symlink } => {
>                   writer.write_all(&[CatalogEntryType::Symlink as u8])?;
> @@ -234,7 +234,7 @@ impl DirInfo {
>           Ok((self.name, data))
>       }
>   
> -    fn parse<C: FnMut(CatalogEntryType, &[u8], u64, u64, u64) -> Result<bool, Error>>(
> +    fn parse<C: FnMut(CatalogEntryType, &[u8], u64, u64, i64) -> Result<bool, Error>>(
>           data: &[u8],
>           mut callback: C,
>       ) -> Result<(), Error> {
> @@ -265,7 +265,7 @@ impl DirInfo {
>                   }
>                   CatalogEntryType::File => {
>                       let size = catalog_decode_u64(&mut cursor)?;
> -                    let mtime = catalog_decode_u64(&mut cursor)?;
> +                    let mtime = catalog_decode_i64(&mut cursor)?;
>                       callback(etype, name, 0, size, mtime)?
>                   }
>                   _ => {
> @@ -362,7 +362,7 @@ impl <W: Write> BackupCatalogWriter for CatalogWriter<W> {
>           Ok(())
>       }
>   
> -    fn add_file(&mut self, name: &CStr, size: u64, mtime: u64) -> Result<(), Error> {
> +    fn add_file(&mut self, name: &CStr, size: u64, mtime: i64) -> Result<(), Error> {
>           let dir = self.dirstack.last_mut().ok_or_else(|| format_err!("outside root"))?;
>           let name = name.to_bytes().to_vec();
>           dir.entries.push(DirEntry { name, attr: DirEntryAttribute::File { size, mtime } });
> @@ -587,6 +587,70 @@ impl <R: Read + Seek> CatalogReader<R> {
>       }
>   }
>   
> +/// Serialize i64 as short, variable length byte sequence
> +///
> +/// Stores 7 bits per byte, Bit 8 indicates the end of the sequence (when not set).
> +/// We limit values to a maximum of 2^63.
> +/// If the first byte is 128 (all zero but the end marker), the number is negative

typo here: s/first/last/ and 0x0 instead of 128
i can send a v2 or a follow-up

> +pub fn catalog_encode_i64<W: Write>(writer: &mut W, v: i64) -> Result<(), Error> {
> +    let mut enc = Vec::new();
> +
> +    let mut d = if v < 0 {
> +        (-1 * v) as u64
> +    } else {
> +        v as u64
> +    };
> +
> +    loop {
> +        if d < 128 {
> +            if v < 0 {
> +                enc.push(128 | d as u8);
> +                enc.push(0u8);
> +            } else {
> +                enc.push(d as u8);
> +            }
> +            break;
> +        }
> +        enc.push((128 | (d & 127)) as u8);
> +        d = d >> 7;
> +    }
> +    writer.write_all(&enc)?;
> +
> +    Ok(())
> +}
> +
> +/// Deserialize i64 from variable length byte sequence
> +///
> +/// We currently read maximal 10 bytes, which give a maximum of 63 bits + sign.
> +/// this method is compatible with catalog_encode_u64, meaning it can
> +/// properly decode values that are encoded with catalog_encode_u64
> +/// (which are <2^63)
> +pub fn catalog_decode_i64<R: Read>(reader: &mut R) -> Result<i64, Error> {
> +
> +    let mut v: u64 = 0;
> +    let mut buf = [0u8];
> +
> +    for i in 0..10 { // only allow 10 bytes (63 bits + sign marker)
> +        if buf.is_empty() {
> +            bail!("decode_i64 failed - unexpected EOB");
> +        }
> +        reader.read_exact(&mut buf)?;
> +
> +        let t = buf[0];
> +
> +        if t == 0 {
> +            return Ok(v as i64 * -1);
> +        } else if t < 128 {
> +            v |= (t as u64) << (i*7);
> +            return Ok(v as i64);
> +        } else {
> +            v |= ((t & 127) as u64) << (i*7);
> +        }
> +    }
> +
> +    bail!("decode_i64 failed - missing end marker");
> +}
> +
>   /// Serialize u64 as short, variable length byte sequence
>   ///
>   /// Stores 7 bits per byte, Bit 8 indicates the end of the sequence (when not set).
> diff --git a/src/pxar/catalog.rs b/src/pxar/catalog.rs
> index 96b42427..02b57dfa 100644
> --- a/src/pxar/catalog.rs
> +++ b/src/pxar/catalog.rs
> @@ -9,7 +9,7 @@ use std::ffi::CStr;
>   pub trait BackupCatalogWriter {
>       fn start_directory(&mut self, name: &CStr) -> Result<(), Error>;
>       fn end_directory(&mut self) -> Result<(), Error>;
> -    fn add_file(&mut self, name: &CStr, size: u64, mtime: u64) -> Result<(), Error>;
> +    fn add_file(&mut self, name: &CStr, size: u64, mtime: i64) -> Result<(), Error>;
>       fn add_symlink(&mut self, name: &CStr) -> Result<(), Error>;
>       fn add_hardlink(&mut self, name: &CStr) -> Result<(), Error>;
>       fn add_block_device(&mut self, name: &CStr) -> Result<(), Error>;
> diff --git a/src/pxar/create.rs b/src/pxar/create.rs
> index 74c49050..6ba6f15c 100644
> --- a/src/pxar/create.rs
> +++ b/src/pxar/create.rs
> @@ -535,7 +535,7 @@ impl<'a, 'b> Archiver<'a, 'b> {
>   
>                   let file_size = stat.st_size as u64;
>                   if let Some(ref mut catalog) = self.catalog {
> -                    catalog.add_file(c_file_name, file_size, stat.st_mtime as u64)?;
> +                    catalog.add_file(c_file_name, file_size, stat.st_mtime)?;
>                   }
>   
>                   let offset: LinkOffset =
> 





^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-10-21 10:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21 10:05 [pbs-devel] [PATCH proxmox-backup] fix #2998: encode mtime as i64 instead of u64 Dominik Csapak
2020-10-21 10:39 ` Dominik Csapak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal