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

Add test to reproduce issue in impl Stream for Entries causing filename truncation #3

Open
wants to merge 1 commit into
base: edera
Choose a base branch
from

Conversation

RazerM
Copy link

@RazerM RazerM commented Nov 5, 2024

I tracked down this issue in astral-sh/uv#5450 (comment)

let val = ready_err!(Pin::new(&mut ef).poll_read_all(cx));

            if is_recognized_header && entry.header().entry_type().is_pax_local_extensions() {
                if self.pax_extensions.is_some() {
                    return Poll::Ready(Some(Err(other(
                        "two pax extensions entries describing \
                         the same member",
                    ))));
                }
                let mut ef = EntryFields::from(entry);
                let val = ready_err!(Pin::new(&mut ef).poll_read_all(cx));
                self.pax_extensions = Some(val);
                continue;
            }

if Pin::new(&mut ef).poll_read_all(cx) is Poll::Pending then ready_err! returns it, so the Pax extension is lost. The same would apply to a pending poll that occurs while a > longlink or longname is being prepared. When poll_next is called again the next entry header is parsed.

This PR demonstrates the issue by creating an AsyncRead impl which pends every second time it is polled.

Commenting out this line makes the test pass, because the reader doesn't enter a pending state in the "wrong" place.

It is probably also the cause of dignifiedquire#39

@RazerM RazerM changed the title Add test to reproduce issue in impl Stream for Entries Add test to reproduce issue in impl Stream for Entries causing filename truncation Nov 5, 2024
@azenla
Copy link
Member

azenla commented Dec 14, 2024

Interesting... I will try to poke at this during the weekend.

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

Successfully merging this pull request may close these issues.

2 participants