From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with UTF8SMTPS id BA617617B8 for ; Wed, 21 Oct 2020 12:39:39 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with UTF8SMTP id AB7341741B for ; Wed, 21 Oct 2020 12:39:39 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with UTF8SMTPS id 4C31B1740C for ; Wed, 21 Oct 2020 12:39:38 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id 0DEA945EA1 for ; Wed, 21 Oct 2020 12:39:38 +0200 (CEST) To: pbs-devel@lists.proxmox.com References: <20201021100501.24286-1-d.csapak@proxmox.com> From: Dominik Csapak Message-ID: <83145525-f6d4-df02-a8b6-f31e8e0be3de@proxmox.com> Date: Wed, 21 Oct 2020 12:39:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:82.0) Gecko/20100101 Thunderbird/82.0 MIME-Version: 1.0 In-Reply-To: <20201021100501.24286-1-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.232 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_NUMSUBJECT 0.5 Subject ends in numbers excluding current years NICE_REPLY_A -0.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [create.rs, catalog.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup] fix #2998: encode mtime as i64 instead of u64 X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Oct 2020 10:39:39 -0000 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 > --- > 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, start: u64, size: u64, mtime:u64) -> Self { > + fn new(etype: CatalogEntryType, name: Vec, 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 Result>( > + fn parse Result>( > 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 BackupCatalogWriter for CatalogWriter { > 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 CatalogReader { > } > } > > +/// 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(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(reader: &mut R) -> Result { > + > + 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 = >