From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id D4C381FF15C for ; Wed, 13 Nov 2024 20:59:26 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8DCCF1C80E; Wed, 13 Nov 2024 20:59:26 +0100 (CET) Date: Wed, 13 Nov 2024 20:59:20 +0100 From: Stoiko Ivanov To: Proxmox VE development discussion Message-ID: References: <20241113163249.76434-1-d.herzig@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20241113163249.76434-1-d.herzig@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.072 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH storage] dirplugin: fix #3986: check for trailing slashes X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Thanks for tackling this long-open issue! On Wed, Nov 13, 2024 at 05:32:49PM +0100, Daniel Herzig wrote: > This patch removes trailing slashes from manually entered > paths by adding an additional if clause in the sub > check_config. For me the context of the change, and why something in the commit is done the way it is, is more helpful than explaining what the code does (this should be clear from the code itself). I wanted to drop a small suggestion for a shortening of the perl-code (see comment inline below) - but then wondered why an adaptation of the path happens in check_config of a storage plugin - as it's been a while since I dealt with the storage-config. The change might be ok - as PVE::SectionConfig calls check_config of the plugin when parsing the configuration file - at least to me that's not directly obvious. Also mentioning how you tested a change can also be helpful to reviewers. > > Signed-off-by: Daniel Herzig > --- > src/PVE/Storage/DirPlugin.pm | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm > index 2efa8d5..9dcdf4a 100644 > --- a/src/PVE/Storage/DirPlugin.pm > +++ b/src/PVE/Storage/DirPlugin.pm > @@ -244,6 +244,9 @@ sub check_config { > if ($opts->{path} !~ m|^/[-/a-zA-Z0-9_.@]+$|) { > die "illegal path for directory storage: $opts->{path}\n"; > } > + if ($opts->{path} =~ /.*\/$/) { > + $opts->{path} = substr($opts->{path}, 0 , -1); > + } without explicit testing - this could be put as: $opts->{path} =~ s/(.*)\/$/$1/; (you could also add + to strip multiple trailing slashes) > return $opts; > } > > -- > 2.39.5 > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel