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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 6ABF49A04 for ; Fri, 4 Aug 2023 10:52:16 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 48A8AC287 for ; Fri, 4 Aug 2023 10:52:16 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 4 Aug 2023 10:52:14 +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 7C6FF42BEE for ; Fri, 4 Aug 2023 10:52:14 +0200 (CEST) Message-ID: Date: Fri, 4 Aug 2023 10:52:13 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.1 Content-Language: en-US To: Proxmox Backup Server development discussion , =?UTF-8?Q?Fabian_Gr=c3=bcnbichler?= , Gabriel Goller References: <20230803152238.124625-1-g.goller@proxmox.com> <836aff2c-4143-577c-5abe-f9601f77293f@proxmox.com> <1691137144.zzi13nxud4.astroid@yuna.none> From: Fiona Ebner In-Reply-To: <1691137144.zzi13nxud4.astroid@yuna.none> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.001 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 NICE_REPLY_A -0.091 Looks like a legit reply (A) 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. [create.rs] Subject: Re: [pbs-devel] [PATH proxmox-backup] fix #4380: stat() is run when file is executed 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: Fri, 04 Aug 2023 08:52:16 -0000 Am 04.08.23 um 10:21 schrieb Fabian Grünbichler: > On August 4, 2023 9:42 am, Fiona Ebner wrote: >> Am 03.08.23 um 17:22 schrieb Gabriel Goller: >>> diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs >>> index 2577cf98..c573c2a3 100644 >>> --- a/pbs-client/src/pxar/create.rs >>> +++ b/pbs-client/src/pxar/create.rs >>> @@ -434,6 +434,15 @@ impl Archiver { >>> assert_single_path_component(os_file_name)?; >>> let full_path = self.path.join(os_file_name); >>> >>> + let match_path = PathBuf::from("/").join(full_path.clone()); >>> + if self >>> + .patterns >>> + .matches(match_path.as_os_str().as_bytes(), None) >> >> Is it fine to call matches() without the file mode in all cases? Can't >> it make a difference for directory matching? If it's okay, please >> explain why in the commit message. > > good catch, thanks. > > I guess we need something like this if we want to support it - the > second hunk is only needed in case we ever differentiate between the > different types other than directories ('/' at the end of the pattern) > and regular files. More is required if we ever need that, because (continued below) > > in the end, it might make more sense to try the other approach I > indicated as follow-up in my first reply? we already have the stat info > of each dir we encounter, so we can decide if a dir is a "weird > unreadable one" and treat that specially, moving the pattern match here > back below the stat, and just never go down that code path for affected > dirs? Can't there be cases where stat() for some non-directory could also fail? If we ever require to differentiate between different non-directory types, it becomes a real chicken-and-egg problem I think. Seems like if we can't stat(), we can choose between: 1. exclude anyways, even if we can't be sure whether it's special or regular 2. failing (thus WONTFIXing the bug for this edge case) But such patterns are currently not used, so.. :P > > diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs > index c573c2a3..eaa84c76 100644 > --- a/pbs-client/src/pxar/create.rs > +++ b/pbs-client/src/pxar/create.rs > @@ -435,9 +435,15 @@ impl Archiver { > let full_path = self.path.join(os_file_name); > > let match_path = PathBuf::from("/").join(full_path.clone()); > + let entry_type = if file.file_type() == Some(nix::dir::Type::Directory) { > + Some(libc::S_IFDIR) > + } else { > + Some(libc::S_IFREG) (continued) this here wouldn't work for patterns that want to skip only regular files, but not other kinds of files. By claiming that it's a regular file here we'd skip regardless of what it actually is. > + }; > + > if self > .patterns > - .matches(match_path.as_os_str().as_bytes(), None) > + .matches(match_path.as_os_str().as_bytes(), entry_type) > == Some(MatchType::Exclude) > { > continue;