[pbs-devel] [PATCH v2 proxmox-backup] client: catalog shell: avoid navigating below archive root

[pbs-devel] [PATCH v2 proxmox-backup] client: catalog shell: avoid navigating below archive root

[Christian Ebner]

Avoid to underflow the catalogs shell position stack by navigating
below the archives root directory into the catalog root. Otherwise
the shell will panic, as the root entry is always expected to be
present.

This threats the archive root directory as being it's own parent
directory, mimicking the behaviour of most common shells.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Encountered while implementing the catalog shell for the split pxar
archive case.

Without this additional check, a `cd ..` in the pxar archive root of
the catalog shell will panic.

changes since version 1:
- use `is_empty` to check if vector is empty
- extend commit message and comment to clarify that archive root acts as
  its own parent directory

 pbs-client/src/catalog_shell.rs | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/pbs-client/src/catalog_shell.rs b/pbs-client/src/catalog_shell.rs
index 349bb7cbc..1c50cbf96 100644
--- a/pbs-client/src/catalog_shell.rs
+++ b/pbs-client/src/catalog_shell.rs
@@ -720,6 +720,13 @@ impl Shell {
                     &mut None,
                 )
                 .await?;
+
+                if new_position.is_empty() {
+                    // Avoid moving below archive root into catalog root, thereby treating
+                    // the archive root as its own parent directory.
+                    return Ok(());
+                }
+
                 if !new_position.last().unwrap().catalog.is_directory() {
                     bail!("not a directory");
                 }
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup] client: catalog shell: avoid navigating below archive root

[Wolfgang Bumiller]

On Thu, Aug 08, 2024 at 01:42:03PM GMT, Christian Ebner wrote:
> Avoid to underflow the catalogs shell position stack by navigating
> below the archives root directory into the catalog root. Otherwise
> the shell will panic, as the root entry is always expected to be
> present.
> 
> This threats the archive root directory as being it's own parent
> directory, mimicking the behaviour of most common shells.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Encountered while implementing the catalog shell for the split pxar
> archive case.
> 
> Without this additional check, a `cd ..` in the pxar archive root of
> the catalog shell will panic.


What about `cd ../../..` when *not* at a root, would we now ignore this
without any error messages?

> 
> changes since version 1:
> - use `is_empty` to check if vector is empty
> - extend commit message and comment to clarify that archive root acts as
>   its own parent directory
> 
>  pbs-client/src/catalog_shell.rs | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/pbs-client/src/catalog_shell.rs b/pbs-client/src/catalog_shell.rs
> index 349bb7cbc..1c50cbf96 100644
> --- a/pbs-client/src/catalog_shell.rs
> +++ b/pbs-client/src/catalog_shell.rs
> @@ -720,6 +720,13 @@ impl Shell {
>                      &mut None,
>                  )
>                  .await?;
> +
> +                if new_position.is_empty() {
> +                    // Avoid moving below archive root into catalog root, thereby treating
> +                    // the archive root as its own parent directory.
> +                    return Ok(());
> +                }
> +
>                  if !new_position.last().unwrap().catalog.is_directory() {
>                      bail!("not a directory");
>                  }
> -- 
> 2.39.2


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup] client: catalog shell: avoid navigating below archive root

[Christian Ebner]

On 8/30/24 10:00, Wolfgang Bumiller wrote:
> On Thu, Aug 08, 2024 at 01:42:03PM GMT, Christian Ebner wrote:
>> Avoid to underflow the catalogs shell position stack by navigating
>> below the archives root directory into the catalog root. Otherwise
>> the shell will panic, as the root entry is always expected to be
>> present.
>>
>> This threats the archive root directory as being it's own parent
>> directory, mimicking the behaviour of most common shells.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> Encountered while implementing the catalog shell for the split pxar
>> archive case.
>>
>> Without this additional check, a `cd ..` in the pxar archive root of
>> the catalog shell will panic.
> 
> 
> What about `cd ../../..` when *not* at a root, would we now ignore this
> without any error messages?

True, the case where the current working directory is not the root 
directory is not handled correctly as is. Will send a new version of the 
patch fixing that.

Thanks a lot!



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup] client: catalog shell: avoid navigating below archive root

[Christian Ebner]

superseded-by version 3:
https://lists.proxmox.com/pipermail/pbs-devel/2024-September/010758.html


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel