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 ESMTPS id B2EAC60ABD for ; Tue, 15 Dec 2020 13:18:39 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A342C11931 for ; Tue, 15 Dec 2020 13:18:09 +0100 (CET) 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 ESMTPS id 2C1F611925 for ; Tue, 15 Dec 2020 13:18:08 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E34AD4515E for ; Tue, 15 Dec 2020 13:18:07 +0100 (CET) Date: Tue, 15 Dec 2020 13:18:06 +0100 From: Wolfgang Bumiller To: Dominik Csapak Cc: Proxmox Backup Server development discussion Message-ID: <20201215121806.5xkruwznsuxt47a7@wobu-vie.proxmox.com> References: <20201215103737.9957-1-w.bumiller@proxmox.com> <17a50cdc-1fd8-cf2f-0061-0c99542b660b@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <17a50cdc-1fd8-cf2f-0061-0c99542b660b@proxmox.com> User-Agent: NeoMutt/20180716 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.018 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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. [mod.rs, main.rs] Subject: [pbs-devel] applied: [PATCH pxar] fix `decode_entry` on special files 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: Tue, 15 Dec 2020 12:18:39 -0000 thanks for testing, applied On Tue, Dec 15, 2020 at 12:08:53PM +0100, Dominik Csapak wrote: > looks good AFAICT > fixes the problem with fifo/sockets > > Tested-By: Dominik Csapak > > On 12/15/20 11:37 AM, Wolfgang Bumiller wrote: > > When using the random accessor to access FIFOs or sockets, > > the ranged reader limits the data to only that entry, and > > the `decode_entry` will never see a `PAYLOAD` or > > `GOODBYE_TABLE` item to finish the entry. > > Instead, it'll reach EOF and we need to handle this. > > The accessor now tells the decoder to expect EOF as a valid > > condition for ending the entry. > > > > Signed-off-by: Wolfgang Bumiller > > --- > > > > The included testcase previously failed with > > "failed to decode entry for fifo0" > > > > src/accessor/mod.rs | 2 +- > > src/decoder/mod.rs | 52 +++++++++++++++++++++++++++++++++++--------- > > tests/simple/main.rs | 46 +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 89 insertions(+), 11 deletions(-) > > > > diff --git a/src/accessor/mod.rs b/src/accessor/mod.rs > > index 0ab03a6..d02dc13 100644 > > --- a/src/accessor/mod.rs > > +++ b/src/accessor/mod.rs > > @@ -229,7 +229,7 @@ async fn get_decoder( > > entry_range: Range, > > path: PathBuf, > > ) -> io::Result>> { > > - Ok(DecoderImpl::new_full(SeqReadAtAdapter::new(input, entry_range), path).await?) > > + Ok(DecoderImpl::new_full(SeqReadAtAdapter::new(input, entry_range), path, true).await?) > > } > > // NOTE: This performs the Decoder::read_next_item() behavior! Keep in mind when changing! > > diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs > > index fcc2dd9..2a5e79a 100644 > > --- a/src/decoder/mod.rs > > +++ b/src/decoder/mod.rs > > @@ -155,6 +155,10 @@ pub(crate) struct DecoderImpl { > > path_lengths: Vec, > > state: State, > > with_goodbye_tables: bool, > > + > > + /// The random access code uses decoders for sub-ranges which may not end in a `PAYLOAD` for > > + /// entries like FIFOs or sockets, so there we explicitly allow an item to terminate with EOF. > > + eof_after_entry: bool, > > } > > enum State { > > @@ -191,14 +195,18 @@ pub(crate) enum ItemResult { > > impl DecoderImpl { > > pub async fn new(input: I) -> io::Result { > > - Self::new_full(input, "/".into()).await > > + Self::new_full(input, "/".into(), false).await > > } > > pub(crate) fn input(&self) -> &I { > > &self.input > > } > > - pub(crate) async fn new_full(input: I, path: PathBuf) -> io::Result { > > + pub(crate) async fn new_full( > > + input: I, > > + path: PathBuf, > > + eof_after_entry: bool, > > + ) -> io::Result { > > let this = DecoderImpl { > > input, > > current_header: unsafe { mem::zeroed() }, > > @@ -210,6 +218,7 @@ impl DecoderImpl { > > path_lengths: Vec::new(), > > state: State::Begin, > > with_goodbye_tables: false, > > + eof_after_entry, > > }; > > // this.read_next_entry().await?; > > @@ -383,7 +392,14 @@ impl DecoderImpl { > > self.current_header = unsafe { mem::zeroed() }; > > - while self.read_next_item().await? != ItemResult::Entry {} > > + loop { > > + match self.read_next_item_or_eof().await? { > > + Some(ItemResult::Entry) => break, > > + Some(ItemResult::Attribute) => continue, > > + None if self.eof_after_entry => break, > > + None => io_bail!("unexpected EOF in entry"), > > + } > > + } > > if self.entry.is_dir() { > > self.path_lengths > > @@ -402,24 +418,40 @@ impl DecoderImpl { > > .ok_or_else(|| io_format_err!("unexpected EOF")) > > } > > + async fn read_next_item(&mut self) -> io::Result { > > + match self.read_next_item_or_eof().await? { > > + Some(item) => Ok(item), > > + None => io_bail!("unexpected EOF"), > > + } > > + } > > + > > + // NOTE: The random accessor will decode FIFOs and Sockets in a decoder instance with a ranged > > + // reader so there is no PAYLOAD or GOODBYE TABLE to "end" an entry. > > + // > > // NOTE: This behavior method is also recreated in the accessor's `get_decoder_at_filename` > > // function! Keep in mind when changing! > > - async fn read_next_item(&mut self) -> io::Result { > > - self.read_next_header().await?; > > - self.read_current_item().await > > + async fn read_next_item_or_eof(&mut self) -> io::Result> { > > + match self.read_next_header_or_eof().await? { > > + Some(()) => self.read_current_item().await.map(Some), > > + None => Ok(None), > > + } > > } > > - async fn read_next_header(&mut self) -> io::Result<()> { > > + async fn read_next_header_or_eof(&mut self) -> io::Result> { > > let dest = unsafe { > > std::slice::from_raw_parts_mut( > > &mut self.current_header as *mut Header as *mut u8, > > size_of_val(&self.current_header), > > ) > > }; > > - seq_read_exact(&mut self.input, dest).await?; > > - self.current_header.check_header_size()?; > > - Ok(()) > > + match seq_read_exact_or_eof(&mut self.input, dest).await? { > > + Some(()) => { > > + self.current_header.check_header_size()?; > > + Ok(Some(())) > > + } > > + None => Ok(None), > > + } > > } > > /// Read the next item, the header is already loaded. > > diff --git a/tests/simple/main.rs b/tests/simple/main.rs > > index c73ca10..f15a0f5 100644 > > --- a/tests/simple/main.rs > > +++ b/tests/simple/main.rs > > @@ -42,6 +42,9 @@ fn test1() { > > assert!(!file.is_empty(), "encoder did not write any data"); > > + // may be useful for testing... > > + // std::fs::write("myarchive.pxar", &file).expect("failed to write out test archive"); > > + > > let mut input = &file[..]; > > let mut decoder = decoder::Decoder::from_std(&mut input).expect("failed to create decoder"); > > let decoded_fs = > > @@ -53,6 +56,7 @@ fn test1() { > > .expect("failed to create random access reader for encoded archive"); > > check_bunzip2(&accessor); > > + check_run_special_files(&accessor); > > } > > fn check_bunzip2(accessor: &accessor::Accessor<&[u8]>) { > > @@ -85,3 +89,45 @@ fn check_bunzip2(accessor: &accessor::Accessor<&[u8]>) { > > assert_eq!(content, "This is the bzip2 executable"); > > } > > + > > +fn check_run_special_files(accessor: &accessor::Accessor<&[u8]>) { > > + let rundir = accessor > > + .open_root() > > + .expect("failed to open root of encoded archive") > > + .lookup("run") > > + .expect("failed to open /run in encoded archive") > > + .expect("missing /run in encoded archive") > > + .enter_directory() > > + .expect("expected /run to be a directory in the test archive"); > > + > > + assert_eq!(rundir.entry_count(), 2, "expected 2 entries in /run"); > > + > > + let mut rd = rundir.read_dir(); > > + let fifo0 = rd > > + .next() > > + .expect("expected 'fifo0' entry in rundir") > > + .expect("failed to get first (fifo0) entry in test archive /run directory"); > > + assert_eq!( > > + fifo0.file_name(), > > + Path::new("fifo0"), > > + "expected first file in /run to be fifo0" > > + ); > > + > > + let _entry = fifo0 > > + .decode_entry() > > + .expect("failed to decode entry for fifo0"); > > + > > + let sock0 = rd > > + .next() > > + .expect("expected 'sock0' entry in rundir") > > + .expect("failed to get second (sock0) entry in test archive /run directory"); > > + assert_eq!( > > + sock0.file_name(), > > + Path::new("sock0"), > > + "expected second file in /run to be sock0" > > + ); > > + > > + let _entry = sock0 > > + .decode_entry() > > + .expect("failed to decode entry for sock0"); > > +} > >