From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
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 7E0B79A94C
 for <pbs-devel@lists.proxmox.com>; Wed, 10 May 2023 15:23:30 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 5905728E30
 for <pbs-devel@lists.proxmox.com>; Wed, 10 May 2023 15:23:00 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pbs-devel@lists.proxmox.com>; Wed, 10 May 2023 15:22:59 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id F2FAC47FB2
 for <pbs-devel@lists.proxmox.com>; Wed, 10 May 2023 15:22:58 +0200 (CEST)
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Wed, 10 May 2023 15:22:58 +0200
Message-Id: <20230510132258.3350401-1-d.csapak@proxmox.com>
X-Mailer: git-send-email 2.30.2
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.015 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 T_SCC_BODY_TEXT_LINE    -0.01 -
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [zip.rs]
Subject: [pbs-devel] [PATCH proxmox] compression: zip_directory: improve
 error handling
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 10 May 2023 13:23:30 -0000

when zipping a directory, our intention was to skip over files that
cannot be zipped (e.g. the file can't be read/vanished/etc.), so we
ignored errors and simply logged it.

but when 'add_entry' fails, we will never actually restore, since every
error there is fatal to the point that the zip cannot be finished thats
because we take the 'target' sink out of self, and only insert it again
after all writes succeeded. so if an error occurs in between 'target' is
not put into self again (and never will be) and the zip cannot be
finished (even if we would catch all those intermediate errors and
restore 'target', we don't know in which state the output was, so we're
unable to finish a valid zip)

to fix that, split the actual 'add_entry' part there out of the async
move block and treat its errors always as fatal

without this, we generate heaps of log lines even after an error
occurred, and can never recover

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
note that i know we use 'eprintln' instead of the log macro, but it's
not part of the fix, and i don't know if we want to add the log crate as
dep here. we only use this function currently in the restore daemon for
downloading a directory

 proxmox-compression/src/zip.rs | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/proxmox-compression/src/zip.rs b/proxmox-compression/src/zip.rs
index c9ed80e..6d457fb 100644
--- a/proxmox-compression/src/zip.rs
+++ b/proxmox-compression/src/zip.rs
@@ -643,7 +643,7 @@ where
         let entry_path = entry.path().to_owned();
         let encoder = &mut encoder;
 
-        if let Err(err) = async move {
+        match async move {
             let entry_path_no_base = entry.path().strip_prefix(base_path)?;
             let metadata = entry.metadata()?;
             let mtime = match metadata
@@ -659,22 +659,27 @@ where
             if entry.file_type().is_file() {
                 let file = tokio::fs::File::open(entry.path()).await?;
                 let ze = ZipEntry::new(entry_path_no_base, mtime, mode, true);
-                encoder.add_entry(ze, Some(file)).await?;
+                Ok(Some((ze, Some(file))))
             } else if entry.file_type().is_dir() {
                 let ze = ZipEntry::new(entry_path_no_base, mtime, mode, false);
                 let content: Option<tokio::fs::File> = None;
-                encoder.add_entry(ze, content).await?;
+                Ok(Some((ze, content)))
+            } else {
+                // ignore other file types
+                Ok::<_, Error>(None)
             }
-            // ignore other file types
-            Ok::<(), Error>(())
         }
         .await
         {
-            eprintln!(
-                "zip: error encoding file or directory '{}': {}",
-                entry_path.display(),
-                err
-            );
+            Ok(Some((ze, content))) => encoder.add_entry(ze, content).await?,
+            Ok(None) => {}
+            Err(err) => {
+                eprintln!(
+                    "zip: error encoding file or directory '{}': {}",
+                    entry_path.display(),
+                    err
+                );
+            }
         }
     }
 
-- 
2.30.2