Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zstd fails with archives using skippable frames #271

Closed
cgwalters opened this issue Apr 28, 2024 · 2 comments
Closed

zstd fails with archives using skippable frames #271

cgwalters opened this issue Apr 28, 2024 · 2 comments

Comments

@cgwalters
Copy link

cgwalters commented Apr 28, 2024

xref ostreedev/ostree-rs-ext#616

The containers zstd:chunked format uses zstd's skippable frames.

It seems that async_compression's zstd support somehow fails with these.

zstd skippable frames don't seem very widely used, but the way I'm testing here is using e.g. skopeo copy --dest-compress-format=zstd:chunked docker://docker.io/library/busybox oci:busybox and you'll get a zstd:chunked tar archive in busybox/blobs/sha256.

I wrote up this quick test program:

use anyhow::Result;

use async_compression::tokio::bufread::ZstdDecoder;
use tokio::io::{stdin, BufReader};

async fn async_decompress() -> Result<()> {
    // Read zstd encoded data from stdin and decode
    let mut reader = ZstdDecoder::new(BufReader::new(stdin()));
    let mut stdout = tokio::io::stdout();
    tokio::io::copy(&mut reader, &mut stdout).await?;
    Ok(())
}

fn sync_decompress() -> Result<()> {
    zstd::stream::copy_decode(std::io::stdin(), std::io::stdout())?;
    Ok(())
}

#[tokio::main(flavor = "current_thread")]
async fn main() -> Result<()> {
    let args = std::env::args();
    let arg = args
        .skip(1)
        .next()
        .ok_or_else(|| anyhow::anyhow!("Missing arg"))?;
    match arg.as_str() {
        "async" => async_decompress().await?,
        "sync" => sync_decompress()?,
        o => anyhow::bail!("invalid {o}"),
    }

    Ok(())
}

And the result is (using nushell syntax on a test archive I have handy):

$ open -r blobs/sha256/bf6f77dfa3bbed41a513875363df862130cdc01bb64bfa2fcabd5b4a65faa1c2 | testcompress sync | tar tf -
<tar extracted ok>

vs

$ open -r blobs/sha256/bf6f77dfa3bbed41a513875363df862130cdc01bb64bfa2fcabd5b4a65faa1c2 | testcompress async | tar tf -
usr/
usr/src/
usr/src/wordpress/
usr/src/wordpress/.htaccess
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
$

Just trying to dig into this, I don't see many references to skippable frame support in the Rust zstd bindings - not clear to me even how one accesses them with the C library. Whereas I see a clear Skippable flag in the Go implementation.

cgwalters added a commit to cgwalters/ostree-rs-ext that referenced this issue Apr 29, 2024
This is basically just a workaround for Nullus157/async-compression#271

However, in practice I think we may as well just use
a native blocking tokio thread here.

There's a lot of shenanigans going on though because
we're wrapping sync I/O with async and then back to sync
because the tar code we're using is still sync.

What would be a lot better is to move the compression to be
inline with the sync tar parsing, but that would require some
API changes and more code motion.
@NobodyXu
Copy link
Collaborator

zsrd skippable format is not supported yet, it is tracked in #266 .

We are currently waiting for zstd to add it

@cgwalters
Copy link
Author

Closing as duplicate, thanks.

@cgwalters cgwalters closed this as not planned Won't fix, can't repro, duplicate, stale Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants