[pbs-devel] [PATCH pxar] decoder: fix incorrect accounting for header in payload input

[pbs-devel] [PATCH pxar] decoder: fix incorrect accounting for header in payload input

[Christian Ebner]

Payload entries are separated by headers of type PAYLOAD within the
payload stream of split pxar archives, used for consistency checks
when accessing the file contents via a reader instance.

Commit 5b8204d0 moved these consistency checks, so they only happen
when actually accessing the content, thereby drastically improving
performance when navigating contents via the metadata archive.

The commit however also incorrectly increased the `Decoder`s
`payload_consumed` field by the size of the header, in case the file
payload has not been accessed by the `content_reader`.

As this filed is used to account for consumed bytes while sequentially
reading to possibly skip over entries, this leads to incorrectly
skipping of bytes in the stream (less than required). The main
manifestation being that a pxar extract with provided match pattern
failed.

Therefore, drop the incorrect accounting of the payload header.

Fixes: 5b8204d0 ("decoder: move payload header check for split input")

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Stumbled accorss this one while working on issue 2996.
Without this a
```
pxar extract archive.mpxar target --payload-input archive.ppxar
--pattern <some-pattern>
```
will fail the payload header checks.

 src/decoder/mod.rs | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs
index 6191627..613ec12 100644
--- a/src/decoder/mod.rs
+++ b/src/decoder/mod.rs
@@ -299,14 +299,9 @@ impl<I: SeqRead> DecoderImpl<I> {
                 }
                 State::InPayload {
                     offset,
-                    header_checked,
                     ..
                 } => {
                     if self.input.payload().is_some() {
-                        if !header_checked {
-                            // header is only checked if payload has been accessed
-                            self.payload_consumed += size_of::<Header>() as u64;
-                        }
                         // Update consumed payload as given by the offset referenced by the content reader
                         self.payload_consumed += offset;
                     } else {
-- 
2.39.2



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

[pbs-devel] applied: [PATCH pxar] decoder: fix incorrect accounting for header in payload input

[Fabian Grünbichler]

thanks (and also thanks for the reminder!)

On September 18, 2024 5:00 pm, Christian Ebner wrote:
> Payload entries are separated by headers of type PAYLOAD within the
> payload stream of split pxar archives, used for consistency checks
> when accessing the file contents via a reader instance.
> 
> Commit 5b8204d0 moved these consistency checks, so they only happen
> when actually accessing the content, thereby drastically improving
> performance when navigating contents via the metadata archive.
> 
> The commit however also incorrectly increased the `Decoder`s
> `payload_consumed` field by the size of the header, in case the file
> payload has not been accessed by the `content_reader`.
> 
> As this filed is used to account for consumed bytes while sequentially
> reading to possibly skip over entries, this leads to incorrectly
> skipping of bytes in the stream (less than required). The main
> manifestation being that a pxar extract with provided match pattern
> failed.
> 
> Therefore, drop the incorrect accounting of the payload header.
> 
> Fixes: 5b8204d0 ("decoder: move payload header check for split input")
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Stumbled accorss this one while working on issue 2996.
> Without this a
> ```
> pxar extract archive.mpxar target --payload-input archive.ppxar
> --pattern <some-pattern>
> ```
> will fail the payload header checks.
> 
>  src/decoder/mod.rs | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs
> index 6191627..613ec12 100644
> --- a/src/decoder/mod.rs
> +++ b/src/decoder/mod.rs
> @@ -299,14 +299,9 @@ impl<I: SeqRead> DecoderImpl<I> {
>                  }
>                  State::InPayload {
>                      offset,
> -                    header_checked,
>                      ..
>                  } => {
>                      if self.input.payload().is_some() {
> -                        if !header_checked {
> -                            // header is only checked if payload has been accessed
> -                            self.payload_consumed += size_of::<Header>() as u64;
> -                        }
>                          // Update consumed payload as given by the offset referenced by the content reader
>                          self.payload_consumed += offset;
>                      } else {
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


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